Auto merge of #4411 - mikerite:fix-4384, r=flip1995

Fix `clone_on_copy` false positives

Closes #4384

changelog: Fix `clone_on_copy` false positives

Author: bors

clippy_lints/src/methods/mod.rs

@@ -1549,30 +1549,30 @@ fn lint_clone_on_copy(cx: &LateContext<'_, '_>, expr: &hir::Expr, arg: &hir::Exp
     if is_copy(cx, ty) {
         let snip;
         if let Some(snippet) = sugg::Sugg::hir_opt(cx, arg) {
+            let parent = cx.tcx.hir().get_parent_node(expr.hir_id);
+            match &cx.tcx.hir().get(parent) {
+                hir::Node::Expr(parent) => match parent.node {
+                    // &*x is a nop, &x.clone() is not
+                    hir::ExprKind::AddrOf(..) |
+                    // (*x).func() is useless, x.clone().func() can work in case func borrows mutably
+                    hir::ExprKind::MethodCall(..) => return,
+                    _ => {},
+                },
+                hir::Node::Stmt(stmt) => {
+                    if let hir::StmtKind::Local(ref loc) = stmt.node {
+                        if let hir::PatKind::Ref(..) = loc.pat.node {
+                            // let ref y = *x borrows x, let ref y = x.clone() does not
+                            return;
+                        }
+                    }
+                },
+                _ => {},
+            }
+
             // x.clone() might have dereferenced x, possibly through Deref impls
             if cx.tables.expr_ty(arg) == ty {
                 snip = Some(("try removing the `clone` call", format!("{}", snippet)));
             } else {
-                let parent = cx.tcx.hir().get_parent_node(expr.hir_id);
-                match cx.tcx.hir().get(parent) {
-                    hir::Node::Expr(parent) => match parent.node {
-                        // &*x is a nop, &x.clone() is not
-                        hir::ExprKind::AddrOf(..) |
-                        // (*x).func() is useless, x.clone().func() can work in case func borrows mutably
-                        hir::ExprKind::MethodCall(..) => return,
-                        _ => {},
-                    },
-                    hir::Node::Stmt(stmt) => {
-                        if let hir::StmtKind::Local(ref loc) = stmt.node {
-                            if let hir::PatKind::Ref(..) = loc.pat.node {
-                                // let ref y = *x borrows x, let ref y = x.clone() does not
-                                return;
-                            }
-                        }
-                    },
-                    _ => {},
-                }
-
                 let deref_count = cx
                     .tables
                     .expr_adjustments(arg)

tests/ui/unnecessary_clone.rs

@@ -22,6 +22,11 @@ fn clone_on_copy() {
 
     let rc = RefCell::new(0);
     rc.borrow().clone();
+
+    // Issue #4348
+    let mut x = 43;
+    let _ = &x.clone(); // ok, getting a ref
+    'a'.clone().make_ascii_uppercase(); // ok, clone and then mutate
 }
 
 fn clone_on_ref_ptr() {

tests/ui/unnecessary_clone.stderr

@@ -19,51 +19,51 @@ LL |     rc.borrow().clone();
    |     ^^^^^^^^^^^^^^^^^^^ help: try dereferencing it: `*rc.borrow()`
 
 error: using '.clone()' on a ref-counted pointer
-  --> $DIR/unnecessary_clone.rs:34:5
+  --> $DIR/unnecessary_clone.rs:39: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:37:5
+  --> $DIR/unnecessary_clone.rs:42:5
    |
 LL |     arc.clone();
    |     ^^^^^^^^^^^ help: try this: `Arc::<bool>::clone(&arc)`
 
 error: using '.clone()' on a ref-counted pointer
-  --> $DIR/unnecessary_clone.rs:40:5
+  --> $DIR/unnecessary_clone.rs:45:5
    |
 LL |     rcweak.clone();
    |     ^^^^^^^^^^^^^^ help: try this: `Weak::<bool>::clone(&rcweak)`
 
 error: using '.clone()' on a ref-counted pointer
-  --> $DIR/unnecessary_clone.rs:43:5
+  --> $DIR/unnecessary_clone.rs:48: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:47:33
+  --> $DIR/unnecessary_clone.rs:52: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:51:5
+  --> $DIR/unnecessary_clone.rs:56:5
    |
 LL |     t.clone();
    |     ^^^^^^^^^ help: try removing the `clone` call: `t`
 
 error: using `clone` on a `Copy` type
-  --> $DIR/unnecessary_clone.rs:53:5
+  --> $DIR/unnecessary_clone.rs:58: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:59:22
+  --> $DIR/unnecessary_clone.rs:64:22
    |
 LL |     let z: &Vec<_> = y.clone();
    |                      ^^^^^^^^^
@@ -79,21 +79,21 @@ LL |     let z: &Vec<_> = &std::vec::Vec<i32>::clone(y);
    |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and more readable
-  --> $DIR/unnecessary_clone.rs:66:27
+  --> $DIR/unnecessary_clone.rs:71:27
    |
 LL |     let v2: Vec<isize> = v.iter().cloned().collect();
    |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.to_vec()`
    |
    = note: `-D clippy::iter-cloned-collect` implied by `-D warnings`
 
 error: called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and more readable
-  --> $DIR/unnecessary_clone.rs:71:38
+  --> $DIR/unnecessary_clone.rs:76:38
    |
 LL |     let _: Vec<isize> = vec![1, 2, 3].iter().cloned().collect();
    |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.to_vec()`
 
 error: called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and more readable
-  --> $DIR/unnecessary_clone.rs:76:24
+  --> $DIR/unnecessary_clone.rs:81:24
    |
 LL |               .to_bytes()
    |  ________________________^
@@ -103,7 +103,7 @@ LL | |             .collect();
    | |______________________^ help: try: `.to_vec()`
 
 error: using `clone` on a `Copy` type
-  --> $DIR/unnecessary_clone.rs:114:20
+  --> $DIR/unnecessary_clone.rs:119:20
    |
 LL |         let _: E = a.clone();
    |                    ^^^^^^^^^ help: try dereferencing it: `*****a`