Skip to content

Commit

Permalink
Do not rewrite .as_ref().map(Arc::clone) and similar
Browse files Browse the repository at this point in the history
  • Loading branch information
samueltardieu committed Mar 22, 2024
1 parent f1bd9a4 commit bd1dd99
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 19 deletions.
4 changes: 3 additions & 1 deletion clippy_lints/src/methods/useless_asref.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::walk_ptrs_ty_depth;
use clippy_utils::ty::{should_call_clone_as_function, walk_ptrs_ty_depth};
use clippy_utils::{
get_parent_expr, is_diag_trait_item, match_def_path, path_to_local_id, paths, peel_blocks, strip_pat_refs,
};
Expand Down Expand Up @@ -93,6 +93,8 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, call_name: &str,
// And that it only has one argument.
&& let [arg] = args
&& is_calling_clone(cx, arg)
// And that we are not recommending recv.clone() over Arc::clone() or similar
&& !should_call_clone_as_function(cx, rcv_ty)
{
lint_as_ref_clone(cx, expr.span.with_hi(parent.span.hi()), recvr, call_name);
}
Expand Down
18 changes: 18 additions & 0 deletions tests/ui/useless_asref.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
)]

use std::fmt::Debug;
use std::rc::{Rc, Weak as RcWeak};
use std::sync::{Arc, Weak as ArcWeak};

struct FakeAsRef;

Expand Down Expand Up @@ -180,6 +182,22 @@ mod issue12135 {
}
}

fn issue_12528() {
struct Foo;

let opt = Some(Arc::new(Foo));
let _ = opt.as_ref().map(Arc::clone);

let opt = Some(Rc::new(Foo));
let _ = opt.as_ref().map(Rc::clone);

let opt = Some(Arc::downgrade(&Arc::new(Foo)));
let _ = opt.as_ref().map(ArcWeak::clone);

let opt = Some(Rc::downgrade(&Rc::new(Foo)));
let _ = opt.as_ref().map(RcWeak::clone);
}

fn main() {
not_ok();
ok();
Expand Down
18 changes: 18 additions & 0 deletions tests/ui/useless_asref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
)]

use std::fmt::Debug;
use std::rc::{Rc, Weak as RcWeak};
use std::sync::{Arc, Weak as ArcWeak};

struct FakeAsRef;

Expand Down Expand Up @@ -180,6 +182,22 @@ mod issue12135 {
}
}

fn issue_12528() {
struct Foo;

let opt = Some(Arc::new(Foo));
let _ = opt.as_ref().map(Arc::clone);

let opt = Some(Rc::new(Foo));
let _ = opt.as_ref().map(Rc::clone);

let opt = Some(Arc::downgrade(&Arc::new(Foo)));
let _ = opt.as_ref().map(ArcWeak::clone);

let opt = Some(Rc::downgrade(&Rc::new(Foo)));
let _ = opt.as_ref().map(RcWeak::clone);
}

fn main() {
not_ok();
ok();
Expand Down
36 changes: 18 additions & 18 deletions tests/ui/useless_asref.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: this call to `as_ref` does nothing
--> tests/ui/useless_asref.rs:48:18
--> tests/ui/useless_asref.rs:50:18
|
LL | foo_rstr(rstr.as_ref());
| ^^^^^^^^^^^^^ help: try: `rstr`
Expand All @@ -11,103 +11,103 @@ LL | #![deny(clippy::useless_asref)]
| ^^^^^^^^^^^^^^^^^^^^^

error: this call to `as_ref` does nothing
--> tests/ui/useless_asref.rs:50:20
--> tests/ui/useless_asref.rs:52:20
|
LL | foo_rslice(rslice.as_ref());
| ^^^^^^^^^^^^^^^ help: try: `rslice`

error: this call to `as_mut` does nothing
--> tests/ui/useless_asref.rs:54:21
--> tests/ui/useless_asref.rs:56:21
|
LL | foo_mrslice(mrslice.as_mut());
| ^^^^^^^^^^^^^^^^ help: try: `mrslice`

error: this call to `as_ref` does nothing
--> tests/ui/useless_asref.rs:56:20
--> tests/ui/useless_asref.rs:58:20
|
LL | foo_rslice(mrslice.as_ref());
| ^^^^^^^^^^^^^^^^ help: try: `mrslice`

error: this call to `as_ref` does nothing
--> tests/ui/useless_asref.rs:63:20
--> tests/ui/useless_asref.rs:65:20
|
LL | foo_rslice(rrrrrslice.as_ref());
| ^^^^^^^^^^^^^^^^^^^ help: try: `rrrrrslice`

error: this call to `as_ref` does nothing
--> tests/ui/useless_asref.rs:65:18
--> tests/ui/useless_asref.rs:67:18
|
LL | foo_rstr(rrrrrstr.as_ref());
| ^^^^^^^^^^^^^^^^^ help: try: `rrrrrstr`

error: this call to `as_mut` does nothing
--> tests/ui/useless_asref.rs:70:21
--> tests/ui/useless_asref.rs:72:21
|
LL | foo_mrslice(mrrrrrslice.as_mut());
| ^^^^^^^^^^^^^^^^^^^^ help: try: `mrrrrrslice`

error: this call to `as_ref` does nothing
--> tests/ui/useless_asref.rs:72:20
--> tests/ui/useless_asref.rs:74:20
|
LL | foo_rslice(mrrrrrslice.as_ref());
| ^^^^^^^^^^^^^^^^^^^^ help: try: `mrrrrrslice`

error: this call to `as_ref` does nothing
--> tests/ui/useless_asref.rs:76:16
--> tests/ui/useless_asref.rs:78:16
|
LL | foo_rrrrmr((&&&&MoreRef).as_ref());
| ^^^^^^^^^^^^^^^^^^^^^^ help: try: `(&&&&MoreRef)`

error: this call to `as_mut` does nothing
--> tests/ui/useless_asref.rs:126:13
--> tests/ui/useless_asref.rs:128:13
|
LL | foo_mrt(mrt.as_mut());
| ^^^^^^^^^^^^ help: try: `mrt`

error: this call to `as_ref` does nothing
--> tests/ui/useless_asref.rs:128:12
--> tests/ui/useless_asref.rs:130:12
|
LL | foo_rt(mrt.as_ref());
| ^^^^^^^^^^^^ help: try: `mrt`

error: this call to `as_ref.map(...)` does nothing
--> tests/ui/useless_asref.rs:139:13
--> tests/ui/useless_asref.rs:141:13
|
LL | let z = x.as_ref().map(String::clone);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.clone()`

error: this call to `as_ref.map(...)` does nothing
--> tests/ui/useless_asref.rs:141:13
--> tests/ui/useless_asref.rs:143:13
|
LL | let z = x.as_ref().map(|z| z.clone());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.clone()`

error: this call to `as_ref.map(...)` does nothing
--> tests/ui/useless_asref.rs:143:13
--> tests/ui/useless_asref.rs:145:13
|
LL | let z = x.as_ref().map(|z| String::clone(z));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.clone()`

error: this call to `as_ref.map(...)` does nothing
--> tests/ui/useless_asref.rs:167:9
--> tests/ui/useless_asref.rs:169:9
|
LL | x.field.as_ref().map(|v| v.clone());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.field.clone()`

error: this call to `as_ref.map(...)` does nothing
--> tests/ui/useless_asref.rs:169:9
--> tests/ui/useless_asref.rs:171:9
|
LL | x.field.as_ref().map(Clone::clone);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.field.clone()`

error: this call to `as_ref.map(...)` does nothing
--> tests/ui/useless_asref.rs:171:9
--> tests/ui/useless_asref.rs:173:9
|
LL | x.field.as_ref().map(|v| Clone::clone(v));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.field.clone()`

error: this call to `as_ref.map(...)` does nothing
--> tests/ui/useless_asref.rs:176:9
--> tests/ui/useless_asref.rs:178:9
|
LL | Some(1).as_ref().map(|&x| x.clone());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(1).clone()`
Expand Down

0 comments on commit bd1dd99

Please sign in to comment.