Skip to content

Commit

Permalink
Auto merge of #12137 - GuillaumeGomez:fix-unconditional_recursion-fal…
Browse files Browse the repository at this point in the history
…se-positive, r=llogiq

Fix false positive in `PartialEq` check in `unconditional_recursion` lint

Fixes #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
  • Loading branch information
bors committed Jan 12, 2024
2 parents 88b5d51 + 1326672 commit 7eca5af
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 4 deletions.
10 changes: 9 additions & 1 deletion clippy_lints/src/unconditional_recursion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 24 additions & 2 deletions tests/ui/unconditional_recursion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
19 changes: 18 additions & 1 deletion tests/ui/unconditional_recursion.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 7eca5af

Please sign in to comment.