Skip to content

Commit fed2f28

Browse files
samueltardieusamueltardieu
committed
Do not rewrite .as_ref().map(Arc::clone) and similar
1 parent 02fc256 commit fed2f28

File tree

4 files changed

+57
-19
lines changed

4 files changed

+57
-19
lines changed

‎clippy_lints/src/methods/useless_asref.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::source::snippet_with_applicability;
3-
use clippy_utils::ty::walk_ptrs_ty_depth;
3+
use clippy_utils::ty::{should_call_clone_as_function, walk_ptrs_ty_depth};
44
use clippy_utils::{
55
get_parent_expr, is_diag_trait_item, match_def_path, path_to_local_id, paths, peel_blocks, strip_pat_refs,
66
};
@@ -93,6 +93,8 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, call_name: &str,
9393
// And that it only has one argument.
9494
&& let [arg] = args
9595
&& is_calling_clone(cx, arg)
96+
// And that we are not recommending recv.clone() over Arc::clone() or similar
97+
&& !should_call_clone_as_function(cx, rcv_ty)
9698
{
9799
lint_as_ref_clone(cx, expr.span.with_hi(parent.span.hi()), recvr, call_name);
98100
}

‎tests/ui/useless_asref.fixed

+18
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
)]
99

1010
use std::fmt::Debug;
11+
use std::rc::{Rc, Weak as RcWeak};
12+
use std::sync::{Arc, Weak as ArcWeak};
1113

1214
struct FakeAsRef;
1315

@@ -180,6 +182,22 @@ mod issue12135 {
180182
}
181183
}
182184

185+
fn issue_12528() {
186+
struct Foo;
187+
188+
let opt = Some(Arc::new(Foo));
189+
let _ = opt.as_ref().map(Arc::clone);
190+
191+
let opt = Some(Rc::new(Foo));
192+
let _ = opt.as_ref().map(Rc::clone);
193+
194+
let opt = Some(Arc::downgrade(&Arc::new(Foo)));
195+
let _ = opt.as_ref().map(ArcWeak::clone);
196+
197+
let opt = Some(Rc::downgrade(&Rc::new(Foo)));
198+
let _ = opt.as_ref().map(RcWeak::clone);
199+
}
200+
183201
fn main() {
184202
not_ok();
185203
ok();

‎tests/ui/useless_asref.rs

+18
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
)]
99

1010
use std::fmt::Debug;
11+
use std::rc::{Rc, Weak as RcWeak};
12+
use std::sync::{Arc, Weak as ArcWeak};
1113

1214
struct FakeAsRef;
1315

@@ -180,6 +182,22 @@ mod issue12135 {
180182
}
181183
}
182184

185+
fn issue_12528() {
186+
struct Foo;
187+
188+
let opt = Some(Arc::new(Foo));
189+
let _ = opt.as_ref().map(Arc::clone);
190+
191+
let opt = Some(Rc::new(Foo));
192+
let _ = opt.as_ref().map(Rc::clone);
193+
194+
let opt = Some(Arc::downgrade(&Arc::new(Foo)));
195+
let _ = opt.as_ref().map(ArcWeak::clone);
196+
197+
let opt = Some(Rc::downgrade(&Rc::new(Foo)));
198+
let _ = opt.as_ref().map(RcWeak::clone);
199+
}
200+
183201
fn main() {
184202
not_ok();
185203
ok();

‎tests/ui/useless_asref.stderr

+18-18
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this call to `as_ref` does nothing
2-
--> tests/ui/useless_asref.rs:48:18
2+
--> tests/ui/useless_asref.rs:50:18
33
|
44
LL | foo_rstr(rstr.as_ref());
55
| ^^^^^^^^^^^^^ help: try: `rstr`
@@ -11,103 +11,103 @@ LL | #![deny(clippy::useless_asref)]
1111
| ^^^^^^^^^^^^^^^^^^^^^
1212

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

0 commit comments

Comments
 (0)