From ab3946d7e9d83e8d177073f27d8ec704b399d687 Mon Sep 17 00:00:00 2001
From: Rabi Guha <rabisg@google.com>
Date: Sun, 12 Apr 2020 15:25:47 +0530
Subject: [PATCH] Fixes #5405: redundant clone false positive with arrays

Check whether slice elements implement Copy before suggesting to drop
the clone method
---
 clippy_lints/src/redundant_clone.rs |  7 ++++++-
 tests/ui/redundant_clone.fixed      | 10 ++++++++++
 tests/ui/redundant_clone.rs         | 10 ++++++++++
 tests/ui/redundant_clone.stderr     | 20 ++++++++++----------
 4 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs
index c315b575ef59..aedbafd408b0 100644
--- a/clippy_lints/src/redundant_clone.rs
+++ b/clippy_lints/src/redundant_clone.rs
@@ -341,6 +341,9 @@ fn base_local_and_movability<'tcx>(
     let mut deref = false;
     // Accessing a field of an ADT that has `Drop`. Moving the field out will cause E0509.
     let mut field = false;
+    // If projection is a slice index then clone can be removed only if the
+    // underlying type implements Copy
+    let mut slice = false;
 
     let PlaceRef { local, mut projection } = place.as_ref();
     while let [base @ .., elem] = projection {
@@ -348,9 +351,11 @@ fn base_local_and_movability<'tcx>(
         deref |= matches!(elem, mir::ProjectionElem::Deref);
         field |= matches!(elem, mir::ProjectionElem::Field(..))
             && has_drop(cx, mir::Place::ty_from(local, projection, &mir.local_decls, cx.tcx).ty);
+        slice |= matches!(elem, mir::ProjectionElem::Index(..))
+            && !is_copy(cx, mir::Place::ty_from(local, projection, &mir.local_decls, cx.tcx).ty);
     }
 
-    Some((local, deref || field))
+    Some((local, deref || field || slice))
 }
 
 struct LocalUseVisitor {
diff --git a/tests/ui/redundant_clone.fixed b/tests/ui/redundant_clone.fixed
index 54815603c6de..764c10a6d398 100644
--- a/tests/ui/redundant_clone.fixed
+++ b/tests/ui/redundant_clone.fixed
@@ -51,6 +51,7 @@ fn main() {
     cannot_move_from_type_with_drop();
     borrower_propagation();
     not_consumed();
+    issue_5405();
 }
 
 #[derive(Clone)]
@@ -160,3 +161,12 @@ fn not_consumed() {
         println!("{}", x);
     }
 }
+
+#[allow(clippy::clone_on_copy)]
+fn issue_5405() {
+    let a: [String; 1] = [String::from("foo")];
+    let _b: String = a[0].clone();
+
+    let c: [usize; 2] = [2, 3];
+    let _d: usize = c[1].clone();
+}
diff --git a/tests/ui/redundant_clone.rs b/tests/ui/redundant_clone.rs
index a9b31183adc8..839747b131d7 100644
--- a/tests/ui/redundant_clone.rs
+++ b/tests/ui/redundant_clone.rs
@@ -51,6 +51,7 @@ fn main() {
     cannot_move_from_type_with_drop();
     borrower_propagation();
     not_consumed();
+    issue_5405();
 }
 
 #[derive(Clone)]
@@ -160,3 +161,12 @@ fn not_consumed() {
         println!("{}", x);
     }
 }
+
+#[allow(clippy::clone_on_copy)]
+fn issue_5405() {
+    let a: [String; 1] = [String::from("foo")];
+    let _b: String = a[0].clone();
+
+    let c: [usize; 2] = [2, 3];
+    let _d: usize = c[1].clone();
+}
diff --git a/tests/ui/redundant_clone.stderr b/tests/ui/redundant_clone.stderr
index 9c27812b9cdc..eced198283ce 100644
--- a/tests/ui/redundant_clone.stderr
+++ b/tests/ui/redundant_clone.stderr
@@ -108,61 +108,61 @@ LL |     let _t = tup.0.clone();
    |              ^^^^^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:60:22
+  --> $DIR/redundant_clone.rs:61:22
    |
 LL |         (a.clone(), a.clone())
    |                      ^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:60:21
+  --> $DIR/redundant_clone.rs:61:21
    |
 LL |         (a.clone(), a.clone())
    |                     ^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:120:15
+  --> $DIR/redundant_clone.rs:121:15
    |
 LL |     let _s = s.clone();
    |               ^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:120:14
+  --> $DIR/redundant_clone.rs:121:14
    |
 LL |     let _s = s.clone();
    |              ^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:121:15
+  --> $DIR/redundant_clone.rs:122:15
    |
 LL |     let _t = t.clone();
    |               ^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:121:14
+  --> $DIR/redundant_clone.rs:122:14
    |
 LL |     let _t = t.clone();
    |              ^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:131:19
+  --> $DIR/redundant_clone.rs:132:19
    |
 LL |         let _f = f.clone();
    |                   ^^^^^^^^ help: remove this
    |
 note: this value is dropped without further use
-  --> $DIR/redundant_clone.rs:131:18
+  --> $DIR/redundant_clone.rs:132:18
    |
 LL |         let _f = f.clone();
    |                  ^
 
 error: redundant clone
-  --> $DIR/redundant_clone.rs:143:14
+  --> $DIR/redundant_clone.rs:144:14
    |
 LL |     let y = x.clone().join("matthias");
    |              ^^^^^^^^ help: remove this
    |
 note: cloned value is neither consumed nor mutated
-  --> $DIR/redundant_clone.rs:143:13
+  --> $DIR/redundant_clone.rs:144:13
    |
 LL |     let y = x.clone().join("matthias");
    |             ^^^^^^^^^