Skip to content

Commit ab2d1c2

Browse files
Extend UNCONDITIONAL_RECURSION to check for ToString implementations
1 parent 1c431f4 commit ab2d1c2

File tree

1 file changed

+60
-9
lines changed

1 file changed

+60
-9
lines changed

clippy_lints/src/unconditional_recursion.rs

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
2-
use clippy_utils::{get_trait_def_id, path_res};
2+
use clippy_utils::{expr_or_init, get_trait_def_id, path_res};
33
use rustc_ast::BinOpKind;
44
use rustc_hir::def::Res;
55
use rustc_hir::def_id::{DefId, LocalDefId};
@@ -56,13 +56,8 @@ fn is_local(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
5656
matches!(path_res(cx, expr), Res::Local(_))
5757
}
5858

59-
fn check_partial_eq(
60-
cx: &LateContext<'_>,
61-
body: &Body<'_>,
62-
method_span: Span,
63-
method_def_id: LocalDefId,
64-
name: Ident,
65-
) {
59+
#[allow(clippy::unnecessary_def_path)]
60+
fn check_partial_eq(cx: &LateContext<'_>, body: &Body<'_>, method_span: Span, method_def_id: LocalDefId, name: Ident) {
6661
let args = cx
6762
.tcx
6863
.instantiate_bound_regions_with_erased(cx.tcx.fn_sig(method_def_id).skip_binder())
@@ -126,8 +121,62 @@ fn check_partial_eq(
126121
}
127122
}
128123

124+
#[allow(clippy::unnecessary_def_path)]
125+
fn check_to_string(cx: &LateContext<'_>, body: &Body<'_>, method_span: Span, method_def_id: LocalDefId, name: Ident) {
126+
let args = cx
127+
.tcx
128+
.instantiate_bound_regions_with_erased(cx.tcx.fn_sig(method_def_id).skip_binder())
129+
.inputs();
130+
// That has one argument.
131+
if let [_self_arg] = args
132+
&& let hir_id = cx.tcx.local_def_id_to_hir_id(method_def_id)
133+
&& let Some((
134+
_,
135+
Node::Item(Item {
136+
kind: ItemKind::Impl(impl_),
137+
owner_id,
138+
..
139+
}),
140+
)) = cx.tcx.hir().parent_iter(hir_id).next()
141+
// We exclude `impl` blocks generated from rustc's proc macros.
142+
&& !cx.tcx.has_attr(*owner_id, sym::automatically_derived)
143+
// It is a implementation of a trait.
144+
&& let Some(trait_) = impl_.of_trait
145+
&& let Some(trait_def_id) = trait_.trait_def_id()
146+
// The trait is `ToString`.
147+
&& Some(trait_def_id) == get_trait_def_id(cx, &["alloc", "string", "ToString"])
148+
{
149+
let expr = expr_or_init(cx, body.value.peel_blocks());
150+
let is_bad = match expr.kind {
151+
ExprKind::MethodCall(segment, receiver, &[arg], _) if segment.ident.name == name.name => {
152+
if is_local(cx, receiver)
153+
&& is_local(cx, &arg)
154+
&& let Some(fn_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
155+
&& let Some(trait_id) = cx.tcx.trait_of_item(fn_id)
156+
&& trait_id == trait_def_id
157+
{
158+
true
159+
} else {
160+
false
161+
}
162+
},
163+
_ => false,
164+
};
165+
if is_bad {
166+
span_lint_and_then(
167+
cx,
168+
UNCONDITIONAL_RECURSION,
169+
method_span,
170+
"function cannot return without recursing",
171+
|diag| {
172+
diag.span_note(expr.span, "recursive call site");
173+
},
174+
);
175+
}
176+
}
177+
}
178+
129179
impl<'tcx> LateLintPass<'tcx> for UnconditionalRecursion {
130-
#[allow(clippy::unnecessary_def_path)]
131180
fn check_fn(
132181
&mut self,
133182
cx: &LateContext<'tcx>,
@@ -141,6 +190,8 @@ impl<'tcx> LateLintPass<'tcx> for UnconditionalRecursion {
141190
if let FnKind::Method(name, _) = kind {
142191
if name.name == sym::eq || name.name == sym::ne {
143192
check_partial_eq(cx, body, method_span, method_def_id, name);
193+
} else if name.name == sym::to_string {
194+
check_to_string(cx, body, method_span, method_def_id, name);
144195
}
145196
}
146197
}

0 commit comments

Comments
 (0)