about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-01-12 18:30:11 +0000
committerbors <bors@rust-lang.org>2024-01-12 18:30:11 +0000
commit7eca5afd34cb3ed089816456e534712a20b447ee (patch)
treeae3ab63f16cd0eced74890aa9727e581e7afe805
parent88b5d519a10ff05d2127a3e146c929fab364a102 (diff)
parent132667288a6b9de113b03ffb82962c9ee8ab576d (diff)
downloadrust-7eca5afd34cb3ed089816456e534712a20b447ee.tar.gz
rust-7eca5afd34cb3ed089816456e534712a20b447ee.zip
Auto merge of #12137 - GuillaumeGomez:fix-unconditional_recursion-false-positive, r=llogiq
Fix false positive in `PartialEq` check in `unconditional_recursion` lint

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

We needed to check for the type of the previous element <del>in case it's a field</del>.

EDIT: After some extra thoughts, no need to check if it's a field, just if it's the same type as `Self`.

r? `@llogiq`

changelog: Fix false positive in `PartialEq` check in `unconditional_recursion` lint
-rw-r--r--clippy_lints/src/unconditional_recursion.rs10
-rw-r--r--tests/ui/unconditional_recursion.rs26
-rw-r--r--tests/ui/unconditional_recursion.stderr19
3 files changed, 51 insertions, 4 deletions
diff --git a/clippy_lints/src/unconditional_recursion.rs b/clippy_lints/src/unconditional_recursion.rs
index e90306ded61..2c283002087 100644
--- a/clippy_lints/src/unconditional_recursion.rs
+++ b/clippy_lints/src/unconditional_recursion.rs
@@ -167,7 +167,15 @@ fn check_partial_eq(cx: &LateContext<'_>, method_span: Span, method_def_id: Loca
                     false
                 }
             },
-            ExprKind::MethodCall(segment, _receiver, &[_arg], _) if segment.ident.name == name.name => {
+            ExprKind::MethodCall(segment, receiver, &[_arg], _) if segment.ident.name == name.name => {
+                if let Some(ty) = cx.typeck_results().expr_ty_opt(receiver)
+                    && let Some(ty_id) = get_ty_def_id(ty)
+                    && self_arg != ty_id
+                {
+                    // Since this called on a different type, the lint should not be
+                    // triggered here.
+                    return;
+                }
                 if let Some(fn_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
                     && let Some(trait_id) = cx.tcx.trait_of_item(fn_id)
                     && trait_id == trait_def_id
diff --git a/tests/ui/unconditional_recursion.rs b/tests/ui/unconditional_recursion.rs
index e1a2d6a90b8..7b898a6e0e7 100644
--- a/tests/ui/unconditional_recursion.rs
+++ b/tests/ui/unconditional_recursion.rs
@@ -264,6 +264,28 @@ impl S13 {
     }
 }
 
-fn main() {
-    // test code goes here
+struct S14 {
+    field: String,
+}
+
+impl PartialEq for S14 {
+    fn eq(&self, other: &Self) -> bool {
+        // Should not warn!
+        self.field.eq(&other.field)
+    }
+}
+
+struct S15<'a> {
+    field: &'a S15<'a>,
 }
+
+impl PartialEq for S15<'_> {
+    fn eq(&self, other: &Self) -> bool {
+        //~^ ERROR: function cannot return without recursing
+        let mine = &self.field;
+        let theirs = &other.field;
+        mine.eq(theirs)
+    }
+}
+
+fn main() {}
diff --git a/tests/ui/unconditional_recursion.stderr b/tests/ui/unconditional_recursion.stderr
index 5d82e2a9f31..094b80d4586 100644
--- a/tests/ui/unconditional_recursion.stderr
+++ b/tests/ui/unconditional_recursion.stderr
@@ -340,5 +340,22 @@ note: recursive call site
 LL |         Self::default()
    |         ^^^^^^^^^^^^^^^
 
-error: aborting due to 26 previous errors
+error: function cannot return without recursing
+  --> $DIR/unconditional_recursion.rs:283:5
+   |
+LL | /     fn eq(&self, other: &Self) -> bool {
+LL | |
+LL | |         let mine = &self.field;
+LL | |         let theirs = &other.field;
+LL | |         mine.eq(theirs)
+LL | |     }
+   | |_____^
+   |
+note: recursive call site
+  --> $DIR/unconditional_recursion.rs:287:9
+   |
+LL |         mine.eq(theirs)
+   |         ^^^^^^^^^^^^^^^
+
+error: aborting due to 27 previous errors