Skip to content

Commit 1c431f4

Browse files
GuillaumeGomezGuillaumeGomez
committed
Move check for PartialEq in UNCONDITIONAL_RECURSION lint into its own function
1 parent 7e650b7 commit 1c431f4

File tree

1 file changed

+75
-62
lines changed

1 file changed

+75
-62
lines changed

‎clippy_lints/src/unconditional_recursion.rs

Lines changed: 75 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use rustc_hir::{Body, Expr, ExprKind, FnDecl, Item, ItemKind, Node};
88
use rustc_lint::{LateContext, LateLintPass};
99
use rustc_middle::ty::{self, Ty};
1010
use rustc_session::declare_lint_pass;
11+
use rustc_span::symbol::Ident;
1112
use rustc_span::{sym, Span};
1213

1314
declare_clippy_lint! {
@@ -55,6 +56,76 @@ fn is_local(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
5556
matches!(path_res(cx, expr), Res::Local(_))
5657
}
5758

59+
fn check_partial_eq(
60+
cx: &LateContext<'_>,
61+
body: &Body<'_>,
62+
method_span: Span,
63+
method_def_id: LocalDefId,
64+
name: Ident,
65+
) {
66+
let args = cx
67+
.tcx
68+
.instantiate_bound_regions_with_erased(cx.tcx.fn_sig(method_def_id).skip_binder())
69+
.inputs();
70+
// That has two arguments.
71+
if let [self_arg, other_arg] = args
72+
&& let Some(self_arg) = get_ty_def_id(*self_arg)
73+
&& let Some(other_arg) = get_ty_def_id(*other_arg)
74+
// The two arguments are of the same type.
75+
&& self_arg == other_arg
76+
&& let hir_id = cx.tcx.local_def_id_to_hir_id(method_def_id)
77+
&& let Some((
78+
_,
79+
Node::Item(Item {
80+
kind: ItemKind::Impl(impl_),
81+
owner_id,
82+
..
83+
}),
84+
)) = cx.tcx.hir().parent_iter(hir_id).next()
85+
// We exclude `impl` blocks generated from rustc's proc macros.
86+
&& !cx.tcx.has_attr(*owner_id, sym::automatically_derived)
87+
// It is a implementation of a trait.
88+
&& let Some(trait_) = impl_.of_trait
89+
&& let Some(trait_def_id) = trait_.trait_def_id()
90+
// The trait is `PartialEq`.
91+
&& Some(trait_def_id) == get_trait_def_id(cx, &["core", "cmp", "PartialEq"])
92+
{
93+
let to_check_op = if name.name == sym::eq {
94+
BinOpKind::Eq
95+
} else {
96+
BinOpKind::Ne
97+
};
98+
let expr = body.value.peel_blocks();
99+
let is_bad = match expr.kind {
100+
ExprKind::Binary(op, left, right) if op.node == to_check_op => is_local(cx, left) && is_local(cx, right),
101+
ExprKind::MethodCall(segment, receiver, &[arg], _) if segment.ident.name == name.name => {
102+
if is_local(cx, receiver)
103+
&& is_local(cx, &arg)
104+
&& let Some(fn_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
105+
&& let Some(trait_id) = cx.tcx.trait_of_item(fn_id)
106+
&& trait_id == trait_def_id
107+
{
108+
true
109+
} else {
110+
false
111+
}
112+
},
113+
_ => false,
114+
};
115+
if is_bad {
116+
span_lint_and_then(
117+
cx,
118+
UNCONDITIONAL_RECURSION,
119+
method_span,
120+
"function cannot return without recursing",
121+
|diag| {
122+
diag.span_note(expr.span, "recursive call site");
123+
},
124+
);
125+
}
126+
}
127+
}
128+
58129
impl<'tcx> LateLintPass<'tcx> for UnconditionalRecursion {
59130
#[allow(clippy::unnecessary_def_path)]
60131
fn check_fn(
@@ -64,70 +135,12 @@ impl<'tcx> LateLintPass<'tcx> for UnconditionalRecursion {
64135
_decl: &'tcx FnDecl<'tcx>,
65136
body: &'tcx Body<'tcx>,
66137
method_span: Span,
67-
def_id: LocalDefId,
138+
method_def_id: LocalDefId,
68139
) {
69140
// If the function is a method...
70-
if let FnKind::Method(name, _) = kind
71-
// That has two arguments.
72-
&& let [self_arg, other_arg] = cx
73-
.tcx
74-
.instantiate_bound_regions_with_erased(cx.tcx.fn_sig(def_id).skip_binder())
75-
.inputs()
76-
&& let Some(self_arg) = get_ty_def_id(*self_arg)
77-
&& let Some(other_arg) = get_ty_def_id(*other_arg)
78-
// The two arguments are of the same type.
79-
&& self_arg == other_arg
80-
&& let hir_id = cx.tcx.local_def_id_to_hir_id(def_id)
81-
&& let Some((
82-
_,
83-
Node::Item(Item {
84-
kind: ItemKind::Impl(impl_),
85-
owner_id,
86-
..
87-
}),
88-
)) = cx.tcx.hir().parent_iter(hir_id).next()
89-
// We exclude `impl` blocks generated from rustc's proc macros.
90-
&& !cx.tcx.has_attr(*owner_id, sym::automatically_derived)
91-
// It is a implementation of a trait.
92-
&& let Some(trait_) = impl_.of_trait
93-
&& let Some(trait_def_id) = trait_.trait_def_id()
94-
// The trait is `PartialEq`.
95-
&& Some(trait_def_id) == get_trait_def_id(cx, &["core", "cmp", "PartialEq"])
96-
{
97-
let to_check_op = if name.name == sym::eq {
98-
BinOpKind::Eq
99-
} else {
100-
BinOpKind::Ne
101-
};
102-
let expr = body.value.peel_blocks();
103-
let is_bad = match expr.kind {
104-
ExprKind::Binary(op, left, right) if op.node == to_check_op => {
105-
is_local(cx, left) && is_local(cx, right)
106-
},
107-
ExprKind::MethodCall(segment, receiver, &[arg], _) if segment.ident.name == name.name => {
108-
if is_local(cx, receiver)
109-
&& is_local(cx, &arg)
110-
&& let Some(fn_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
111-
&& let Some(trait_id) = cx.tcx.trait_of_item(fn_id)
112-
&& trait_id == trait_def_id
113-
{
114-
true
115-
} else {
116-
false
117-
}
118-
},
119-
_ => false,
120-
};
121-
if is_bad {
122-
span_lint_and_then(
123-
cx,
124-
UNCONDITIONAL_RECURSION,
125-
method_span,
126-
"function cannot return without recursing",
127-
|diag| {
128-
diag.span_note(expr.span, "recursive call site");
129-
},
130-
);
141+
if let FnKind::Method(name, _) = kind {
142+
if name.name == sym::eq || name.name == sym::ne {
143+
check_partial_eq(cx, body, method_span, method_def_id, name);
131144
}
132145
}
133146
}

0 commit comments

Comments
 (0)