Skip to content

Commit

Permalink
Check for clone-on-copy in argument positions
Browse files Browse the repository at this point in the history
Earlier if arguments to method calls matched the above pattern they were
not reported. This patch ensures such arguments are checked as well.

Fixes #5436
  • Loading branch information
rabisg0 committed Apr 9, 2020
1 parent c25f26d commit 183c4ab
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 15 deletions.
5 changes: 3 additions & 2 deletions clippy_lints/src/methods/mod.rs
Expand Up @@ -1947,9 +1947,10 @@ fn lint_clone_on_copy(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, arg: &hir:
match &cx.tcx.hir().get(parent) {
hir::Node::Expr(parent) => match parent.kind {
// &*x is a nop, &x.clone() is not
hir::ExprKind::AddrOf(..) |
hir::ExprKind::AddrOf(..) => return,
// (*x).func() is useless, x.clone().func() can work in case func borrows mutably
hir::ExprKind::MethodCall(..) => return,
hir::ExprKind::MethodCall(_, _, parent_args) if expr.hir_id == parent_args[0].hir_id => return,

_ => {},
},
hir::Node::Stmt(stmt) => {
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/unnecessary_clone.rs
Expand Up @@ -13,6 +13,10 @@ impl SomeTrait for SomeImpl {}

fn main() {}

fn is_ascii(ch: char) -> bool {
ch.is_ascii()
}

fn clone_on_copy() {
42.clone();

Expand All @@ -27,6 +31,11 @@ fn clone_on_copy() {
let mut x = 43;
let _ = &x.clone(); // ok, getting a ref
'a'.clone().make_ascii_uppercase(); // ok, clone and then mutate
is_ascii('z'.clone());

// Issue #5436
let mut vec = Vec::new();
vec.push(42.clone());
}

fn clone_on_ref_ptr() {
Expand Down
38 changes: 25 additions & 13 deletions tests/ui/unnecessary_clone.stderr
@@ -1,69 +1,81 @@
error: using `clone` on a `Copy` type
--> $DIR/unnecessary_clone.rs:17:5
--> $DIR/unnecessary_clone.rs:21:5
|
LL | 42.clone();
| ^^^^^^^^^^ help: try removing the `clone` call: `42`
|
= note: `-D clippy::clone-on-copy` implied by `-D warnings`

error: using `clone` on a `Copy` type
--> $DIR/unnecessary_clone.rs:21:5
--> $DIR/unnecessary_clone.rs:25:5
|
LL | (&42).clone();
| ^^^^^^^^^^^^^ help: try dereferencing it: `*(&42)`

error: using `clone` on a `Copy` type
--> $DIR/unnecessary_clone.rs:24:5
--> $DIR/unnecessary_clone.rs:28:5
|
LL | rc.borrow().clone();
| ^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `*rc.borrow()`

error: using `clone` on a `Copy` type
--> $DIR/unnecessary_clone.rs:34:14
|
LL | is_ascii('z'.clone());
| ^^^^^^^^^^^ help: try removing the `clone` call: `'z'`

error: using `clone` on a `Copy` type
--> $DIR/unnecessary_clone.rs:38:14
|
LL | vec.push(42.clone());
| ^^^^^^^^^^ help: try removing the `clone` call: `42`

error: using `.clone()` on a ref-counted pointer
--> $DIR/unnecessary_clone.rs:39:5
--> $DIR/unnecessary_clone.rs:48:5
|
LL | rc.clone();
| ^^^^^^^^^^ help: try this: `Rc::<bool>::clone(&rc)`
|
= note: `-D clippy::clone-on-ref-ptr` implied by `-D warnings`

error: using `.clone()` on a ref-counted pointer
--> $DIR/unnecessary_clone.rs:42:5
--> $DIR/unnecessary_clone.rs:51:5
|
LL | arc.clone();
| ^^^^^^^^^^^ help: try this: `Arc::<bool>::clone(&arc)`

error: using `.clone()` on a ref-counted pointer
--> $DIR/unnecessary_clone.rs:45:5
--> $DIR/unnecessary_clone.rs:54:5
|
LL | rcweak.clone();
| ^^^^^^^^^^^^^^ help: try this: `Weak::<bool>::clone(&rcweak)`

error: using `.clone()` on a ref-counted pointer
--> $DIR/unnecessary_clone.rs:48:5
--> $DIR/unnecessary_clone.rs:57:5
|
LL | arc_weak.clone();
| ^^^^^^^^^^^^^^^^ help: try this: `Weak::<bool>::clone(&arc_weak)`

error: using `.clone()` on a ref-counted pointer
--> $DIR/unnecessary_clone.rs:52:33
--> $DIR/unnecessary_clone.rs:61:33
|
LL | let _: Arc<dyn SomeTrait> = x.clone();
| ^^^^^^^^^ help: try this: `Arc::<SomeImpl>::clone(&x)`

error: using `clone` on a `Copy` type
--> $DIR/unnecessary_clone.rs:56:5
--> $DIR/unnecessary_clone.rs:65:5
|
LL | t.clone();
| ^^^^^^^^^ help: try removing the `clone` call: `t`

error: using `clone` on a `Copy` type
--> $DIR/unnecessary_clone.rs:58:5
--> $DIR/unnecessary_clone.rs:67:5
|
LL | Some(t).clone();
| ^^^^^^^^^^^^^^^ help: try removing the `clone` call: `Some(t)`

error: using `clone` on a double-reference; this will copy the reference instead of cloning the inner type
--> $DIR/unnecessary_clone.rs:64:22
--> $DIR/unnecessary_clone.rs:73:22
|
LL | let z: &Vec<_> = y.clone();
| ^^^^^^^^^
Expand All @@ -79,10 +91,10 @@ LL | let z: &Vec<_> = &std::vec::Vec<i32>::clone(y);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: using `clone` on a `Copy` type
--> $DIR/unnecessary_clone.rs:100:20
--> $DIR/unnecessary_clone.rs:109:20
|
LL | let _: E = a.clone();
| ^^^^^^^^^ help: try dereferencing it: `*****a`

error: aborting due to 12 previous errors
error: aborting due to 14 previous errors

0 comments on commit 183c4ab

Please sign in to comment.