Skip to content

Commit d5488b3

Browse files
y21y21
authored
useless_asref: add a check for Clone before suggesting the use of .clone() (rust-lang#14174)
fixes rust-lang#12357 changelog: [`useless_asref`]: don't suggest to use `.clone()` if the target type doesn't implement the `Clone` trait
2 parents e479a9f + 91548d0 commit d5488b3

File tree

4 files changed

+82
-19
lines changed

4 files changed

+82
-19
lines changed

‎clippy_lints/src/methods/useless_asref.rs‎

Lines changed: 4 additions & 1 deletion
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::{should_call_clone_as_function, walk_ptrs_ty_depth};
3+
use clippy_utils::ty::{implements_trait, 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, peel_blocks, strip_pat_refs,
66
};
@@ -101,6 +101,9 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, call_name: &str,
101101
&& is_calling_clone(cx, arg)
102102
// And that we are not recommending recv.clone() over Arc::clone() or similar
103103
&& !should_call_clone_as_function(cx, rcv_ty)
104+
// https://github.com/rust-lang/rust-clippy/issues/12357
105+
&& let Some(clone_trait) = cx.tcx.lang_items().clone_trait()
106+
&& implements_trait(cx, cx.typeck_results().expr_ty(recvr), clone_trait, &[])
104107
{
105108
lint_as_ref_clone(cx, expr.span.with_hi(parent.span.hi()), recvr, call_name);
106109
}

‎tests/ui/useless_asref.fixed‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
)]
99

1010
use std::fmt::Debug;
11+
use std::ops::Deref;
1112
use std::rc::{Rc, Weak as RcWeak};
1213
use std::sync::{Arc, Weak as ArcWeak};
1314

@@ -218,6 +219,35 @@ fn issue_14088() {
218219
let _: Option<&str> = s.as_ref().map(|x| x.as_ref());
219220
}
220221

222+
pub struct Wrap<T> {
223+
inner: T,
224+
}
225+
226+
impl<T> Deref for Wrap<T> {
227+
type Target = T;
228+
229+
fn deref(&self) -> &Self::Target {
230+
&self.inner
231+
}
232+
}
233+
234+
struct NonCloneableError;
235+
236+
pub struct Issue12357 {
237+
current: Option<Wrap<Arc<u32>>>,
238+
}
239+
240+
impl Issue12357 {
241+
fn f(&self) -> Option<Arc<u32>> {
242+
self.current.as_ref().map(|p| Arc::clone(p))
243+
}
244+
245+
fn g(&self) {
246+
let result: Result<String, NonCloneableError> = Ok("Hello".to_string());
247+
let cloned = result.as_ref().map(|s| s.clone());
248+
}
249+
}
250+
221251
fn main() {
222252
not_ok();
223253
ok();

‎tests/ui/useless_asref.rs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
)]
99

1010
use std::fmt::Debug;
11+
use std::ops::Deref;
1112
use std::rc::{Rc, Weak as RcWeak};
1213
use std::sync::{Arc, Weak as ArcWeak};
1314

@@ -218,6 +219,35 @@ fn issue_14088() {
218219
let _: Option<&str> = s.as_ref().map(|x| x.as_ref());
219220
}
220221

222+
pub struct Wrap<T> {
223+
inner: T,
224+
}
225+
226+
impl<T> Deref for Wrap<T> {
227+
type Target = T;
228+
229+
fn deref(&self) -> &Self::Target {
230+
&self.inner
231+
}
232+
}
233+
234+
struct NonCloneableError;
235+
236+
pub struct Issue12357 {
237+
current: Option<Wrap<Arc<u32>>>,
238+
}
239+
240+
impl Issue12357 {
241+
fn f(&self) -> Option<Arc<u32>> {
242+
self.current.as_ref().map(|p| Arc::clone(p))
243+
}
244+
245+
fn g(&self) {
246+
let result: Result<String, NonCloneableError> = Ok("Hello".to_string());
247+
let cloned = result.as_ref().map(|s| s.clone());
248+
}
249+
}
250+
221251
fn main() {
222252
not_ok();
223253
ok();

‎tests/ui/useless_asref.stderr‎

Lines changed: 18 additions & 18 deletions
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:50:18
2+
--> tests/ui/useless_asref.rs:51: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:53:20
14+
--> tests/ui/useless_asref.rs:54: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:58:21
20+
--> tests/ui/useless_asref.rs:59: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:61:20
26+
--> tests/ui/useless_asref.rs:62: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:69:20
32+
--> tests/ui/useless_asref.rs:70: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:72:18
38+
--> tests/ui/useless_asref.rs:73: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:78:21
44+
--> tests/ui/useless_asref.rs:79: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:81:20
50+
--> tests/ui/useless_asref.rs:82: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:86:16
56+
--> tests/ui/useless_asref.rs:87: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:137:13
62+
--> tests/ui/useless_asref.rs:138: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:140:12
68+
--> tests/ui/useless_asref.rs:141: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:152:13
74+
--> tests/ui/useless_asref.rs:153: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:155:13
80+
--> tests/ui/useless_asref.rs:156: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:158:13
86+
--> tests/ui/useless_asref.rs:159: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:182:9
92+
--> tests/ui/useless_asref.rs:183: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:185:9
98+
--> tests/ui/useless_asref.rs:186: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:188:9
104+
--> tests/ui/useless_asref.rs:189: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:193:9
110+
--> tests/ui/useless_asref.rs:194:9
111111
|
112112
LL | Some(1).as_ref().map(|&x| x.clone());
113113
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(1).clone()`

0 commit comments

Comments
 (0)