about summary refs log tree commit diff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--clippy_lints/src/assigning_clones.rs29
-rw-r--r--tests/ui/assigning_clones.fixed68
-rw-r--r--tests/ui/assigning_clones.rs68
-rw-r--r--tests/ui/assigning_clones.stderr86
4 files changed, 222 insertions, 29 deletions
diff --git a/clippy_lints/src/assigning_clones.rs b/clippy_lints/src/assigning_clones.rs
index 03f777600f0..6e336efbb90 100644
--- a/clippy_lints/src/assigning_clones.rs
+++ b/clippy_lints/src/assigning_clones.rs
@@ -227,9 +227,22 @@ fn build_sugg<'tcx>(
             match call_kind {
                 CallKind::Method => {
                     let receiver_sugg = if let ExprKind::Unary(hir::UnOp::Deref, ref_expr) = lhs.kind {
-                        // `*lhs = self_expr.clone();` -> `lhs.clone_from(self_expr)`
-                        Sugg::hir_with_applicability(cx, ref_expr, "_", app)
+                        // If `ref_expr` is a reference, we can remove the dereference operator (`*`) to make
+                        // the generated code a bit simpler. In other cases, we don't do this special case, to avoid
+                        // having to deal with Deref (https://github.com/rust-lang/rust-clippy/issues/12437).
+
+                        let ty = cx.typeck_results().expr_ty(ref_expr);
+                        if ty.is_ref() {
+                            // Apply special case, remove `*`
+                            // `*lhs = self_expr.clone();` -> `lhs.clone_from(self_expr)`
+                            Sugg::hir_with_applicability(cx, ref_expr, "_", app)
+                        } else {
+                            // Keep the original lhs
+                            // `*lhs = self_expr.clone();` -> `(*lhs).clone_from(self_expr)`
+                            Sugg::hir_with_applicability(cx, lhs, "_", app)
+                        }
                     } else {
+                        // Keep the original lhs
                         // `lhs = self_expr.clone();` -> `lhs.clone_from(self_expr)`
                         Sugg::hir_with_applicability(cx, lhs, "_", app)
                     }
@@ -249,8 +262,16 @@ fn build_sugg<'tcx>(
                 },
                 CallKind::Ufcs => {
                     let self_sugg = if let ExprKind::Unary(hir::UnOp::Deref, ref_expr) = lhs.kind {
-                        // `*lhs = Clone::clone(self_expr);` -> `Clone::clone_from(lhs, self_expr)`
-                        Sugg::hir_with_applicability(cx, ref_expr, "_", app)
+                        // See special case of removing `*` in method handling above
+                        let ty = cx.typeck_results().expr_ty(ref_expr);
+                        if ty.is_ref() {
+                            // `*lhs = Clone::clone(self_expr);` -> `Clone::clone_from(lhs, self_expr)`
+                            Sugg::hir_with_applicability(cx, ref_expr, "_", app)
+                        } else {
+                            // `*lhs = Clone::clone(self_expr);` -> `Clone::clone_from(&mut *lhs, self_expr)`
+                            // mut_addr_deref is used to avoid unnecessary parentheses around `*lhs`
+                            Sugg::hir_with_applicability(cx, ref_expr, "_", app).mut_addr_deref()
+                        }
                     } else {
                         // `lhs = Clone::clone(self_expr);` -> `Clone::clone_from(&mut lhs, self_expr)`
                         Sugg::hir_with_applicability(cx, lhs, "_", app).mut_addr()
diff --git a/tests/ui/assigning_clones.fixed b/tests/ui/assigning_clones.fixed
index 60f6a385fc5..b376d55a402 100644
--- a/tests/ui/assigning_clones.fixed
+++ b/tests/ui/assigning_clones.fixed
@@ -3,6 +3,7 @@
 #![allow(clippy::ptr_arg)] // https://github.com/rust-lang/rust-clippy/issues/10612
 #![allow(clippy::needless_late_init)]
 #![allow(clippy::box_collection)]
+#![allow(clippy::boxed_local)]
 #![warn(clippy::assigning_clones)]
 
 use std::borrow::ToOwned;
@@ -182,6 +183,31 @@ impl Clone for AvoidRecursiveCloneFrom {
     }
 }
 
+// Deref handling
+fn clone_into_deref_method(mut a: DerefWrapper<HasCloneFrom>, b: HasCloneFrom) {
+    (*a).clone_from(&b);
+}
+
+fn clone_into_deref_with_clone_method(mut a: DerefWrapperWithClone<HasCloneFrom>, b: HasCloneFrom) {
+    (*a).clone_from(&b);
+}
+
+fn clone_into_box_method(mut a: Box<HasCloneFrom>, b: HasCloneFrom) {
+    (*a).clone_from(&b);
+}
+
+fn clone_into_self_deref_method(a: &mut DerefWrapperWithClone<HasCloneFrom>, b: DerefWrapperWithClone<HasCloneFrom>) {
+    a.clone_from(&b);
+}
+
+fn clone_into_deref_function(mut a: DerefWrapper<HasCloneFrom>, b: HasCloneFrom) {
+    Clone::clone_from(&mut *a, &b);
+}
+
+fn clone_into_box_function(mut a: Box<HasCloneFrom>, b: HasCloneFrom) {
+    Clone::clone_from(&mut *a, &b);
+}
+
 // ToOwned
 fn owned_method_mut_ref(mut_string: &mut String, ref_str: &str) {
     ref_str.clone_into(mut_string);
@@ -328,3 +354,45 @@ mod borrowck_conflicts {
         path = path.components().as_path().to_owned();
     }
 }
+
+struct DerefWrapper<T>(T);
+
+impl<T> Deref for DerefWrapper<T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl<T> DerefMut for DerefWrapper<T> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
+struct DerefWrapperWithClone<T>(T);
+
+impl<T> Deref for DerefWrapperWithClone<T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl<T> DerefMut for DerefWrapperWithClone<T> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
+impl<T: Clone> Clone for DerefWrapperWithClone<T> {
+    fn clone(&self) -> Self {
+        Self(self.0.clone())
+    }
+
+    fn clone_from(&mut self, source: &Self) {
+        *self = Self(source.0.clone());
+    }
+}
diff --git a/tests/ui/assigning_clones.rs b/tests/ui/assigning_clones.rs
index 6eb85be511a..11a5d4459c3 100644
--- a/tests/ui/assigning_clones.rs
+++ b/tests/ui/assigning_clones.rs
@@ -3,6 +3,7 @@
 #![allow(clippy::ptr_arg)] // https://github.com/rust-lang/rust-clippy/issues/10612
 #![allow(clippy::needless_late_init)]
 #![allow(clippy::box_collection)]
+#![allow(clippy::boxed_local)]
 #![warn(clippy::assigning_clones)]
 
 use std::borrow::ToOwned;
@@ -182,6 +183,31 @@ impl Clone for AvoidRecursiveCloneFrom {
     }
 }
 
+// Deref handling
+fn clone_into_deref_method(mut a: DerefWrapper<HasCloneFrom>, b: HasCloneFrom) {
+    *a = b.clone();
+}
+
+fn clone_into_deref_with_clone_method(mut a: DerefWrapperWithClone<HasCloneFrom>, b: HasCloneFrom) {
+    *a = b.clone();
+}
+
+fn clone_into_box_method(mut a: Box<HasCloneFrom>, b: HasCloneFrom) {
+    *a = b.clone();
+}
+
+fn clone_into_self_deref_method(a: &mut DerefWrapperWithClone<HasCloneFrom>, b: DerefWrapperWithClone<HasCloneFrom>) {
+    *a = b.clone();
+}
+
+fn clone_into_deref_function(mut a: DerefWrapper<HasCloneFrom>, b: HasCloneFrom) {
+    *a = Clone::clone(&b);
+}
+
+fn clone_into_box_function(mut a: Box<HasCloneFrom>, b: HasCloneFrom) {
+    *a = Clone::clone(&b);
+}
+
 // ToOwned
 fn owned_method_mut_ref(mut_string: &mut String, ref_str: &str) {
     *mut_string = ref_str.to_owned();
@@ -328,3 +354,45 @@ mod borrowck_conflicts {
         path = path.components().as_path().to_owned();
     }
 }
+
+struct DerefWrapper<T>(T);
+
+impl<T> Deref for DerefWrapper<T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl<T> DerefMut for DerefWrapper<T> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
+struct DerefWrapperWithClone<T>(T);
+
+impl<T> Deref for DerefWrapperWithClone<T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl<T> DerefMut for DerefWrapperWithClone<T> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
+impl<T: Clone> Clone for DerefWrapperWithClone<T> {
+    fn clone(&self) -> Self {
+        Self(self.0.clone())
+    }
+
+    fn clone_from(&mut self, source: &Self) {
+        *self = Self(source.0.clone());
+    }
+}
diff --git a/tests/ui/assigning_clones.stderr b/tests/ui/assigning_clones.stderr
index a68516376ab..19724a6d4ec 100644
--- a/tests/ui/assigning_clones.stderr
+++ b/tests/ui/assigning_clones.stderr
@@ -1,5 +1,5 @@
 error: assigning the result of `Clone::clone()` may be inefficient
-  --> tests/ui/assigning_clones.rs:24:5
+  --> tests/ui/assigning_clones.rs:25:5
    |
 LL |     *mut_thing = value_thing.clone();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `mut_thing.clone_from(&value_thing)`
@@ -8,142 +8,178 @@ LL |     *mut_thing = value_thing.clone();
    = help: to override `-D warnings` add `#[allow(clippy::assigning_clones)]`
 
 error: assigning the result of `Clone::clone()` may be inefficient
-  --> tests/ui/assigning_clones.rs:28:5
+  --> tests/ui/assigning_clones.rs:29:5
    |
 LL |     *mut_thing = ref_thing.clone();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `mut_thing.clone_from(ref_thing)`
 
 error: assigning the result of `Clone::clone()` may be inefficient
-  --> tests/ui/assigning_clones.rs:32:5
+  --> tests/ui/assigning_clones.rs:33:5
    |
 LL |     mut_thing = ref_thing.clone();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `mut_thing.clone_from(ref_thing)`
 
 error: assigning the result of `Clone::clone()` may be inefficient
-  --> tests/ui/assigning_clones.rs:36:5
+  --> tests/ui/assigning_clones.rs:37:5
    |
 LL |     *mut_thing = Clone::clone(ref_thing);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(mut_thing, ref_thing)`
 
 error: assigning the result of `Clone::clone()` may be inefficient
-  --> tests/ui/assigning_clones.rs:40:5
+  --> tests/ui/assigning_clones.rs:41:5
    |
 LL |     mut_thing = Clone::clone(ref_thing);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(&mut mut_thing, ref_thing)`
 
 error: assigning the result of `Clone::clone()` may be inefficient
-  --> tests/ui/assigning_clones.rs:44:5
+  --> tests/ui/assigning_clones.rs:45:5
    |
 LL |     *mut_thing = Clone::clone(ref_thing);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(mut_thing, ref_thing)`
 
 error: assigning the result of `Clone::clone()` may be inefficient
-  --> tests/ui/assigning_clones.rs:48:5
+  --> tests/ui/assigning_clones.rs:49:5
    |
 LL |     *mut_thing = HasCloneFrom::clone(ref_thing);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(mut_thing, ref_thing)`
 
 error: assigning the result of `Clone::clone()` may be inefficient
-  --> tests/ui/assigning_clones.rs:52:5
+  --> tests/ui/assigning_clones.rs:53:5
    |
 LL |     *mut_thing = <HasCloneFrom as Clone>::clone(ref_thing);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(mut_thing, ref_thing)`
 
 error: assigning the result of `Clone::clone()` may be inefficient
-  --> tests/ui/assigning_clones.rs:57:5
+  --> tests/ui/assigning_clones.rs:58:5
    |
 LL |     *(mut_thing + &mut HasCloneFrom) = ref_thing.clone();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `(mut_thing + &mut HasCloneFrom).clone_from(ref_thing)`
 
 error: assigning the result of `Clone::clone()` may be inefficient
-  --> tests/ui/assigning_clones.rs:62:5
+  --> tests/ui/assigning_clones.rs:63:5
    |
 LL |     *mut_thing = (ref_thing + ref_thing).clone();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `mut_thing.clone_from(ref_thing + ref_thing)`
 
 error: assigning the result of `Clone::clone()` may be inefficient
-  --> tests/ui/assigning_clones.rs:67:5
+  --> tests/ui/assigning_clones.rs:68:5
    |
 LL |     s = format!("{} {}", "hello", "world").clone();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `s.clone_from(&format!("{} {}", "hello", "world"))`
 
 error: assigning the result of `Clone::clone()` may be inefficient
-  --> tests/ui/assigning_clones.rs:72:5
+  --> tests/ui/assigning_clones.rs:73:5
    |
 LL |     s = Clone::clone(&format!("{} {}", "hello", "world"));
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(&mut s, &format!("{} {}", "hello", "world"))`
 
 error: assigning the result of `Clone::clone()` may be inefficient
-  --> tests/ui/assigning_clones.rs:78:9
+  --> tests/ui/assigning_clones.rs:79:9
    |
 LL |         a = b.clone();
    |         ^^^^^^^^^^^^^ help: use `clone_from()`: `a.clone_from(&b)`
 
 error: assigning the result of `Clone::clone()` may be inefficient
-  --> tests/ui/assigning_clones.rs:149:5
+  --> tests/ui/assigning_clones.rs:150:5
    |
 LL |     a = b.clone();
    |     ^^^^^^^^^^^^^ help: use `clone_from()`: `a.clone_from(&b)`
 
 error: assigning the result of `Clone::clone()` may be inefficient
-  --> tests/ui/assigning_clones.rs:156:5
+  --> tests/ui/assigning_clones.rs:157:5
    |
 LL |     a = b.clone();
    |     ^^^^^^^^^^^^^ help: use `clone_from()`: `a.clone_from(&b)`
 
 error: assigning the result of `ToOwned::to_owned()` may be inefficient
-  --> tests/ui/assigning_clones.rs:157:5
+  --> tests/ui/assigning_clones.rs:158:5
    |
 LL |     a = c.to_owned();
    |     ^^^^^^^^^^^^^^^^ help: use `clone_into()`: `c.clone_into(&mut a)`
 
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> tests/ui/assigning_clones.rs:188:5
+   |
+LL |     *a = b.clone();
+   |     ^^^^^^^^^^^^^^ help: use `clone_from()`: `(*a).clone_from(&b)`
+
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> tests/ui/assigning_clones.rs:192:5
+   |
+LL |     *a = b.clone();
+   |     ^^^^^^^^^^^^^^ help: use `clone_from()`: `(*a).clone_from(&b)`
+
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> tests/ui/assigning_clones.rs:196:5
+   |
+LL |     *a = b.clone();
+   |     ^^^^^^^^^^^^^^ help: use `clone_from()`: `(*a).clone_from(&b)`
+
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> tests/ui/assigning_clones.rs:200:5
+   |
+LL |     *a = b.clone();
+   |     ^^^^^^^^^^^^^^ help: use `clone_from()`: `a.clone_from(&b)`
+
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> tests/ui/assigning_clones.rs:204:5
+   |
+LL |     *a = Clone::clone(&b);
+   |     ^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(&mut *a, &b)`
+
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> tests/ui/assigning_clones.rs:208:5
+   |
+LL |     *a = Clone::clone(&b);
+   |     ^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(&mut *a, &b)`
+
 error: assigning the result of `ToOwned::to_owned()` may be inefficient
-  --> tests/ui/assigning_clones.rs:187:5
+  --> tests/ui/assigning_clones.rs:213:5
    |
 LL |     *mut_string = ref_str.to_owned();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(mut_string)`
 
 error: assigning the result of `ToOwned::to_owned()` may be inefficient
-  --> tests/ui/assigning_clones.rs:191:5
+  --> tests/ui/assigning_clones.rs:217:5
    |
 LL |     mut_string = ref_str.to_owned();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut mut_string)`
 
 error: assigning the result of `ToOwned::to_owned()` may be inefficient
-  --> tests/ui/assigning_clones.rs:212:5
+  --> tests/ui/assigning_clones.rs:238:5
    |
 LL |     **mut_box_string = ref_str.to_owned();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut (*mut_box_string))`
 
 error: assigning the result of `ToOwned::to_owned()` may be inefficient
-  --> tests/ui/assigning_clones.rs:216:5
+  --> tests/ui/assigning_clones.rs:242:5
    |
 LL |     **mut_box_string = ref_str.to_owned();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut (*mut_box_string))`
 
 error: assigning the result of `ToOwned::to_owned()` may be inefficient
-  --> tests/ui/assigning_clones.rs:220:5
+  --> tests/ui/assigning_clones.rs:246:5
    |
 LL |     *mut_thing = ToOwned::to_owned(ref_str);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(ref_str, mut_thing)`
 
 error: assigning the result of `ToOwned::to_owned()` may be inefficient
-  --> tests/ui/assigning_clones.rs:224:5
+  --> tests/ui/assigning_clones.rs:250:5
    |
 LL |     mut_thing = ToOwned::to_owned(ref_str);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(ref_str, &mut mut_thing)`
 
 error: assigning the result of `ToOwned::to_owned()` may be inefficient
-  --> tests/ui/assigning_clones.rs:229:5
+  --> tests/ui/assigning_clones.rs:255:5
    |
 LL |     s = format!("{} {}", "hello", "world").to_owned();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `format!("{} {}", "hello", "world").clone_into(&mut s)`
 
 error: assigning the result of `ToOwned::to_owned()` may be inefficient
-  --> tests/ui/assigning_clones.rs:234:5
+  --> tests/ui/assigning_clones.rs:260:5
    |
 LL |     s = ToOwned::to_owned(&format!("{} {}", "hello", "world"));
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(&format!("{} {}", "hello", "world"), &mut s)`
 
-error: aborting due to 24 previous errors
+error: aborting due to 30 previous errors
generated by cgit-pink 1.4.1-3-g733a5 (git 2.37.1) at 2026-04-24 02:19:14 +0000