Auto merge of #12615 - Kobzol:fix-recursive-clone-from, r=blyxyas

Do not suggest `assigning_clones` in `Clone` impl

This PR modifies `assigning_clones` to detect situations where the `clone` call is inside a `Clone` impl, and avoids suggesting the lint in such situations.

r? `@blyxyas`

Fixes: https://github.com/rust-lang/rust-clippy/issues/12600

changelog: Do not invoke `assigning_clones` inside `Clone` impl

4 files changed, 49 insertions, 6 deletions

diff --git a/clippy_lints/src/assigning_clones.rs b/clippy_lints/src/assigning_clones.rs
index d7d2dbee433..e1e7d406cd0 100644
--- a/clippy_lints/src/assigning_clones.rs
+++ b/clippy_lints/src/assigning_clones.rs
@@ -181,6 +181,23 @@ fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallC
         return false;
     }
 
+    // If the call expression is inside an impl block that contains the method invoked by the
+    // call expression, we bail out to avoid suggesting something that could result in endless
+    // recursion.
+    if let Some(local_block_id) = impl_block.as_local()
+        && let Some(block) = cx.tcx.hir_node_by_def_id(local_block_id).as_owner()
+    {
+        let impl_block_owner = block.def_id();
+        if cx
+            .tcx
+            .hir()
+            .parent_id_iter(lhs.hir_id)
+            .any(|parent| parent.owner == impl_block_owner)
+        {
+            return false;
+        }
+    }
+
     // Find the function for which we want to check that it is implemented.
     let provided_fn = match call.target {
         TargetTrait::Clone => cx.tcx.get_diagnostic_item(sym::Clone).and_then(|clone| {
diff --git a/tests/ui/assigning_clones.fixed b/tests/ui/assigning_clones.fixed
index 160f3b94663..8387c7d6156 100644
--- a/tests/ui/assigning_clones.fixed
+++ b/tests/ui/assigning_clones.fixed
@@ -153,6 +153,19 @@ fn clone_inside_macro() {
     clone_inside!(a, b);
 }
 
+// Make sure that we don't suggest the lint when we call clone inside a Clone impl
+// https://github.com/rust-lang/rust-clippy/issues/12600
+pub struct AvoidRecursiveCloneFrom;
+
+impl Clone for AvoidRecursiveCloneFrom {
+    fn clone(&self) -> Self {
+        Self
+    }
+    fn clone_from(&mut self, source: &Self) {
+        *self = source.clone();
+    }
+}
+
 // ToOwned
 fn owned_method_mut_ref(mut_string: &mut String, ref_str: &str) {
     ref_str.clone_into(mut_string);
diff --git a/tests/ui/assigning_clones.rs b/tests/ui/assigning_clones.rs
index 14ba1d4db9a..6f4da9f652c 100644
--- a/tests/ui/assigning_clones.rs
+++ b/tests/ui/assigning_clones.rs
@@ -153,6 +153,19 @@ fn clone_inside_macro() {
     clone_inside!(a, b);
 }
 
+// Make sure that we don't suggest the lint when we call clone inside a Clone impl
+// https://github.com/rust-lang/rust-clippy/issues/12600
+pub struct AvoidRecursiveCloneFrom;
+
+impl Clone for AvoidRecursiveCloneFrom {
+    fn clone(&self) -> Self {
+        Self
+    }
+    fn clone_from(&mut self, source: &Self) {
+        *self = source.clone();
+    }
+}
+
 // ToOwned
 fn owned_method_mut_ref(mut_string: &mut String, ref_str: &str) {
     *mut_string = ref_str.to_owned();
diff --git a/tests/ui/assigning_clones.stderr b/tests/ui/assigning_clones.stderr
index ba59f067431..793927bd1cb 100644
--- a/tests/ui/assigning_clones.stderr
+++ b/tests/ui/assigning_clones.stderr
@@ -86,37 +86,37 @@ LL |     a = c.to_owned();
    |     ^^^^^^^^^^^^^^^^ help: use `clone_into()`: `c.clone_into(&mut a)`
 
 error: assigning the result of `ToOwned::to_owned()` may be inefficient
-  --> tests/ui/assigning_clones.rs:158:5
+  --> tests/ui/assigning_clones.rs:171: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:162:5
+  --> tests/ui/assigning_clones.rs:175: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:183:5
+  --> tests/ui/assigning_clones.rs:196: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:187:5
+  --> tests/ui/assigning_clones.rs:200: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:191:5
+  --> tests/ui/assigning_clones.rs:204: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:195:5
+  --> tests/ui/assigning_clones.rs:208:5
    |
 LL |     mut_thing = ToOwned::to_owned(ref_str);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(ref_str, &mut mut_thing)`