From 571118f4b018aad8b15e8f80aa905f0209a27aba Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= <berykubik@gmail.com>
Date: Tue, 2 Apr 2024 10:41:58 +0200
Subject: [PATCH] Do not suggest `assigning_clones` in `Clone` impl

---
 clippy_lints/src/assigning_clones.rs | 17 +++++++++++++++++
 tests/ui/assigning_clones.fixed      | 13 +++++++++++++
 tests/ui/assigning_clones.rs         | 13 +++++++++++++
 tests/ui/assigning_clones.stderr     | 12 ++++++------
 4 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/clippy_lints/src/assigning_clones.rs b/clippy_lints/src/assigning_clones.rs
index d7d2dbee4..e1e7d406c 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 160f3b946..8387c7d61 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 14ba1d4db..6f4da9f65 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 ba59f0674..793927bd1 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)`