Skip to content

Commit 26602dd

Browse files
phanschphansch
authored
Merge pull request #3336 from HMPerson1/clone_on_copy_deref
Fix `clone_on_copy` not detecting derefs sometimes
2 parents 4884c2f + 2a9dec6 commit 26602dd

File tree

3 files changed

+87
-27
lines changed

3 files changed

+87
-27
lines changed

‎clippy_lints/src/methods/mod.rs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1332,7 +1332,8 @@ fn lint_clone_on_copy(cx: &LateContext<'_, '_>, expr: &hir::Expr, arg: &hir::Exp
13321332
if is_copy(cx, ty) {
13331333
let snip;
13341334
if let Some(snippet) = sugg::Sugg::hir_opt(cx, arg) {
1335-
if let ty::Ref(..) = cx.tables.expr_ty(arg).sty {
1335+
// x.clone() might have dereferenced x, possibly through Deref impls
1336+
if cx.tables.expr_ty(arg) != ty {
13361337
let parent = cx.tcx.hir.get_parent_node(expr.id);
13371338
match cx.tcx.hir.get(parent) {
13381339
hir::Node::Expr(parent) => match parent.node {
@@ -1354,7 +1355,18 @@ fn lint_clone_on_copy(cx: &LateContext<'_, '_>, expr: &hir::Expr, arg: &hir::Exp
13541355
},
13551356
_ => {},
13561357
}
1357-
snip = Some(("try dereferencing it", format!("{}", snippet.deref())));
1358+
1359+
let deref_count = cx.tables.expr_adjustments(arg).iter()
1360+
.filter(|adj| {
1361+
if let ty::adjustment::Adjust::Deref(_) = adj.kind {
1362+
true
1363+
} else {
1364+
false
1365+
}
1366+
})
1367+
.count();
1368+
let derefs: String = iter::repeat('*').take(deref_count).collect();
1369+
snip = Some(("try dereferencing it", format!("{}{}", derefs, snippet)));
13581370
} else {
13591371
snip = Some(("try removing the `clone` call", format!("{}", snippet)));
13601372
}

‎tests/ui/unnecessary_clone.rs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#![warn(clippy::clone_on_ref_ptr)]
1414
#![allow(unused)]
1515

16+
use std::cell::RefCell;
1617
use std::collections::HashSet;
1718
use std::collections::VecDeque;
1819
use std::rc::{self, Rc};
@@ -30,6 +31,9 @@ fn clone_on_copy() {
3031
vec![1].clone(); // ok, not a Copy type
3132
Some(vec![1]).clone(); // ok, not a Copy type
3233
(&42).clone();
34+
35+
let rc = RefCell::new(0);
36+
rc.borrow().clone();
3337
}
3438

3539
fn clone_on_ref_ptr() {
@@ -75,3 +79,35 @@ fn iter_clone_collect() {
7579
let v3 : HashSet<isize> = v.iter().cloned().collect();
7680
let v4 : VecDeque<isize> = v.iter().cloned().collect();
7781
}
82+
83+
mod many_derefs {
84+
struct A;
85+
struct B;
86+
struct C;
87+
struct D;
88+
#[derive(Copy, Clone)]
89+
struct E;
90+
91+
macro_rules! impl_deref {
92+
($src:ident, $dst:ident) => {
93+
impl std::ops::Deref for $src {
94+
type Target = $dst;
95+
fn deref(&self) -> &Self::Target { &$dst }
96+
}
97+
}
98+
}
99+
100+
impl_deref!(A, B);
101+
impl_deref!(B, C);
102+
impl_deref!(C, D);
103+
impl std::ops::Deref for D {
104+
type Target = &'static E;
105+
fn deref(&self) -> &Self::Target { &&E }
106+
}
107+
108+
fn go1() {
109+
let a = A;
110+
let _: E = a.clone();
111+
let _: E = *****a;
112+
}
113+
}

‎tests/ui/unnecessary_clone.stderr‎

Lines changed: 37 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,84 +1,96 @@
11
error: using `clone` on a `Copy` type
2-
--> $DIR/unnecessary_clone.rs:28:5
2+
--> $DIR/unnecessary_clone.rs:29:5
33
|
4-
28 | 42.clone();
4+
29 | 42.clone();
55
| ^^^^^^^^^^ help: try removing the `clone` call: `42`
66
|
77
= note: `-D clippy::clone-on-copy` implied by `-D warnings`
88

99
error: using `clone` on a `Copy` type
10-
--> $DIR/unnecessary_clone.rs:32:5
10+
--> $DIR/unnecessary_clone.rs:33:5
1111
|
12-
32 | (&42).clone();
12+
33 | (&42).clone();
1313
| ^^^^^^^^^^^^^ help: try dereferencing it: `*(&42)`
1414

15+
error: using `clone` on a `Copy` type
16+
--> $DIR/unnecessary_clone.rs:36:5
17+
|
18+
36 | rc.borrow().clone();
19+
| ^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `*rc.borrow()`
20+
1521
error: using '.clone()' on a ref-counted pointer
16-
--> $DIR/unnecessary_clone.rs:42:5
22+
--> $DIR/unnecessary_clone.rs:46:5
1723
|
18-
42 | rc.clone();
24+
46 | rc.clone();
1925
| ^^^^^^^^^^ help: try this: `Rc::<bool>::clone(&rc)`
2026
|
2127
= note: `-D clippy::clone-on-ref-ptr` implied by `-D warnings`
2228

2329
error: using '.clone()' on a ref-counted pointer
24-
--> $DIR/unnecessary_clone.rs:45:5
30+
--> $DIR/unnecessary_clone.rs:49:5
2531
|
26-
45 | arc.clone();
32+
49 | arc.clone();
2733
| ^^^^^^^^^^^ help: try this: `Arc::<bool>::clone(&arc)`
2834

2935
error: using '.clone()' on a ref-counted pointer
30-
--> $DIR/unnecessary_clone.rs:48:5
36+
--> $DIR/unnecessary_clone.rs:52:5
3137
|
32-
48 | rcweak.clone();
38+
52 | rcweak.clone();
3339
| ^^^^^^^^^^^^^^ help: try this: `Weak::<bool>::clone(&rcweak)`
3440

3541
error: using '.clone()' on a ref-counted pointer
36-
--> $DIR/unnecessary_clone.rs:51:5
42+
--> $DIR/unnecessary_clone.rs:55:5
3743
|
38-
51 | arc_weak.clone();
44+
55 | arc_weak.clone();
3945
| ^^^^^^^^^^^^^^^^ help: try this: `Weak::<bool>::clone(&arc_weak)`
4046

4147
error: using '.clone()' on a ref-counted pointer
42-
--> $DIR/unnecessary_clone.rs:55:29
48+
--> $DIR/unnecessary_clone.rs:59:29
4349
|
44-
55 | let _: Arc<SomeTrait> = x.clone();
50+
59 | let _: Arc<SomeTrait> = x.clone();
4551
| ^^^^^^^^^ help: try this: `Arc::<SomeImpl>::clone(&x)`
4652

4753
error: using `clone` on a `Copy` type
48-
--> $DIR/unnecessary_clone.rs:59:5
54+
--> $DIR/unnecessary_clone.rs:63:5
4955
|
50-
59 | t.clone();
56+
63 | t.clone();
5157
| ^^^^^^^^^ help: try removing the `clone` call: `t`
5258

5359
error: using `clone` on a `Copy` type
54-
--> $DIR/unnecessary_clone.rs:61:5
60+
--> $DIR/unnecessary_clone.rs:65:5
5561
|
56-
61 | Some(t).clone();
62+
65 | Some(t).clone();
5763
| ^^^^^^^^^^^^^^^ help: try removing the `clone` call: `Some(t)`
5864

5965
error: using `clone` on a double-reference; this will copy the reference instead of cloning the inner type
60-
--> $DIR/unnecessary_clone.rs:67:22
66+
--> $DIR/unnecessary_clone.rs:71:22
6167
|
62-
67 | let z: &Vec<_> = y.clone();
68+
71 | let z: &Vec<_> = y.clone();
6369
| ^^^^^^^^^
6470
|
6571
= note: #[deny(clippy::clone_double_ref)] on by default
6672
help: try dereferencing it
6773
|
68-
67 | let z: &Vec<_> = &(*y).clone();
74+
71 | let z: &Vec<_> = &(*y).clone();
6975
| ^^^^^^^^^^^^^
7076
help: or try being explicit about what type to clone
7177
|
72-
67 | let z: &Vec<_> = &std::vec::Vec<i32>::clone(y);
78+
71 | let z: &Vec<_> = &std::vec::Vec<i32>::clone(y);
7379
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
7480

7581
error: called `cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and more readable
76-
--> $DIR/unnecessary_clone.rs:74:27
82+
--> $DIR/unnecessary_clone.rs:78:27
7783
|
78-
74 | let v2 : Vec<isize> = v.iter().cloned().collect();
84+
78 | let v2 : Vec<isize> = v.iter().cloned().collect();
7985
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
8086
|
8187
= note: `-D clippy::iter-cloned-collect` implied by `-D warnings`
8288

83-
error: aborting due to 11 previous errors
89+
error: using `clone` on a `Copy` type
90+
--> $DIR/unnecessary_clone.rs:110:20
91+
|
92+
110 | let _: E = a.clone();
93+
| ^^^^^^^^^ help: try dereferencing it: `*****a`
94+
95+
error: aborting due to 13 previous errors
8496

0 commit comments

Comments
 (0)