Skip to content

Commit 7eca5af

Browse files
committed
Auto merge of #12137 - GuillaumeGomez:fix-unconditional_recursion-false-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
2 parents 88b5d51 + 1326672 commit 7eca5af

File tree

3 files changed

+51
-4
lines changed

3 files changed

+51
-4
lines changed

clippy_lints/src/unconditional_recursion.rs

+9-1
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,15 @@ fn check_partial_eq(cx: &LateContext<'_>, method_span: Span, method_def_id: Loca
167167
false
168168
}
169169
},
170-
ExprKind::MethodCall(segment, _receiver, &[_arg], _) if segment.ident.name == name.name => {
170+
ExprKind::MethodCall(segment, receiver, &[_arg], _) if segment.ident.name == name.name => {
171+
if let Some(ty) = cx.typeck_results().expr_ty_opt(receiver)
172+
&& let Some(ty_id) = get_ty_def_id(ty)
173+
&& self_arg != ty_id
174+
{
175+
// Since this called on a different type, the lint should not be
176+
// triggered here.
177+
return;
178+
}
171179
if let Some(fn_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
172180
&& let Some(trait_id) = cx.tcx.trait_of_item(fn_id)
173181
&& trait_id == trait_def_id

tests/ui/unconditional_recursion.rs

+24-2
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,28 @@ impl S13 {
264264
}
265265
}
266266

267-
fn main() {
268-
// test code goes here
267+
struct S14 {
268+
field: String,
269+
}
270+
271+
impl PartialEq for S14 {
272+
fn eq(&self, other: &Self) -> bool {
273+
// Should not warn!
274+
self.field.eq(&other.field)
275+
}
276+
}
277+
278+
struct S15<'a> {
279+
field: &'a S15<'a>,
269280
}
281+
282+
impl PartialEq for S15<'_> {
283+
fn eq(&self, other: &Self) -> bool {
284+
//~^ ERROR: function cannot return without recursing
285+
let mine = &self.field;
286+
let theirs = &other.field;
287+
mine.eq(theirs)
288+
}
289+
}
290+
291+
fn main() {}

tests/ui/unconditional_recursion.stderr

+18-1
Original file line numberDiff line numberDiff line change
@@ -340,5 +340,22 @@ note: recursive call site
340340
LL | Self::default()
341341
| ^^^^^^^^^^^^^^^
342342

343-
error: aborting due to 26 previous errors
343+
error: function cannot return without recursing
344+
--> $DIR/unconditional_recursion.rs:283:5
345+
|
346+
LL | / fn eq(&self, other: &Self) -> bool {
347+
LL | |
348+
LL | | let mine = &self.field;
349+
LL | | let theirs = &other.field;
350+
LL | | mine.eq(theirs)
351+
LL | | }
352+
| |_____^
353+
|
354+
note: recursive call site
355+
--> $DIR/unconditional_recursion.rs:287:9
356+
|
357+
LL | mine.eq(theirs)
358+
| ^^^^^^^^^^^^^^^
359+
360+
error: aborting due to 27 previous errors
344361

0 commit comments

Comments
 (0)