From 183c4abb224d4108df4c65cdaca760888fbbb709 Mon Sep 17 00:00:00 2001
From: Rabi Guha <rabisg@google.com>
Date: Thu, 9 Apr 2020 19:20:09 +0530
Subject: [PATCH] Check for clone-on-copy in argument positions

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
---
 clippy_lints/src/methods/mod.rs   |  5 ++--
 tests/ui/unnecessary_clone.rs     |  9 ++++++++
 tests/ui/unnecessary_clone.stderr | 38 ++++++++++++++++++++-----------
 3 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index c539e0360..fb9b60ae8 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -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) => {
diff --git a/tests/ui/unnecessary_clone.rs b/tests/ui/unnecessary_clone.rs
index 6dff8d87b..7a1d031fa 100644
--- a/tests/ui/unnecessary_clone.rs
+++ b/tests/ui/unnecessary_clone.rs
@@ -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();
 
@@ -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() {
diff --git a/tests/ui/unnecessary_clone.stderr b/tests/ui/unnecessary_clone.stderr
index 69447e682..7b34ff9e3 100644
--- a/tests/ui/unnecessary_clone.stderr
+++ b/tests/ui/unnecessary_clone.stderr
@@ -1,5 +1,5 @@
 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`
@@ -7,19 +7,31 @@ LL |     42.clone();
    = 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)`
@@ -27,43 +39,43 @@ LL |     rc.clone();
    = 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();
    |                      ^^^^^^^^^
@@ -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