diff options
| author | bors <bors@rust-lang.org> | 2024-01-12 18:30:11 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2024-01-12 18:30:11 +0000 |
| commit | 7eca5afd34cb3ed089816456e534712a20b447ee (patch) | |
| tree | ae3ab63f16cd0eced74890aa9727e581e7afe805 | |
| parent | 88b5d519a10ff05d2127a3e146c929fab364a102 (diff) | |
| parent | 132667288a6b9de113b03ffb82962c9ee8ab576d (diff) | |
| download | rust-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.rs | 10 | ||||
| -rw-r--r-- | tests/ui/unconditional_recursion.rs | 26 | ||||
| -rw-r--r-- | tests/ui/unconditional_recursion.stderr | 19 |
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 |
