Skip to content

Commit e107ec2

Browse files
committed
Add extra check to format_impl lint for self.fmt()
As mentioned in this Reddit comment: https://www.reddit.com/r/rust/comments/tq602q/my_first_clippy_lint_detecting_use_of_self_as/i2fhdy1/ Previously use of self.fmt within the Format trait wasn't detected Note we don't check fully qualified paths atm, but that shouldn't happen by accident anyway.
1 parent 5344236 commit e107ec2

File tree

3 files changed

+71
-1
lines changed

3 files changed

+71
-1
lines changed

clippy_lints/src/format_impl.rs

+27
Original file line numberDiff line numberDiff line change
@@ -131,11 +131,38 @@ impl<'tcx> LateLintPass<'tcx> for FormatImpl {
131131
check_to_string_in_display(cx, expr);
132132
}
133133

134+
check_self_fmt_in_fmt(cx, expr, format_trait_impl);
134135
check_self_in_format_args(cx, expr, format_trait_impl);
135136
check_print_in_format_impl(cx, expr, format_trait_impl);
136137
}
137138
}
138139

140+
fn check_self_fmt_in_fmt(cx: &LateContext<'_>, expr: &Expr<'_>, format_trait: FormatTrait) {
141+
if_chain! {
142+
// Get the hir_id of the object we are calling the method on
143+
if let ExprKind::MethodCall(path, [ref self_arg, ..], _) = expr.kind;
144+
// Is the method to_string() ?
145+
if path.ident.name == sym!(fmt);
146+
// Is the method a part of the ToString trait? (i.e. not to_string() implemented
147+
// separately)
148+
if let FormatTrait { name, .. } = format_trait;
149+
if let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
150+
if is_diag_trait_item(cx, expr_def_id, name);
151+
// Is the method is called on self
152+
if let ExprKind::Path(QPath::Resolved(_, path)) = self_arg.kind;
153+
if let [segment] = path.segments;
154+
if segment.ident.name == kw::SelfLower;
155+
then {
156+
span_lint(
157+
cx,
158+
RECURSIVE_FORMAT_IMPL,
159+
expr.span,
160+
&format!("using `self.fmt` as `{name}` in `impl {name}` will cause infinite recursion"),
161+
);
162+
}
163+
}
164+
}
165+
139166
fn check_to_string_in_display(cx: &LateContext<'_>, expr: &Expr<'_>) {
140167
if_chain! {
141168
// Get the hir_id of the object we are calling the method on

tests/ui/recursive_format_impl.rs

+37
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,43 @@ impl std::fmt::Debug for Tree {
310310
}
311311
}
312312

313+
// Should also catch use of self.fmt within the Format trait
314+
315+
enum Foo {
316+
Foo(usize),
317+
Bar(usize),
318+
}
319+
320+
impl std::fmt::Display for Foo {
321+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
322+
match self {
323+
Self::Foo(_) => self.fmt(f),
324+
Self::Bar(x) => x.fmt(f),
325+
}
326+
}
327+
}
328+
329+
// Doesn't trigger on nested enum matching
330+
enum Tree2 {
331+
Leaf,
332+
Node(Vec<Tree>),
333+
}
334+
335+
impl std::fmt::Display for Tree2 {
336+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
337+
match self {
338+
Tree2::Leaf => write!(f, "*"),
339+
Tree2::Node(children) => {
340+
write!(f, "(")?;
341+
for child in children.iter() {
342+
child.fmt(f)?;
343+
}
344+
write!(f, ")")
345+
},
346+
}
347+
}
348+
}
349+
313350
fn main() {
314351
let a = A;
315352
a.to_string();

tests/ui/recursive_format_impl.stderr

+7-1
Original file line numberDiff line numberDiff line change
@@ -87,5 +87,11 @@ LL | write!(f, "{}", &&**&&*self)
8787
|
8888
= note: this error originates in the macro `write` (in Nightly builds, run with -Z macro-backtrace for more info)
8989

90-
error: aborting due to 11 previous errors
90+
error: using `self.fmt` as `Display` in `impl Display` will cause infinite recursion
91+
--> $DIR/recursive_format_impl.rs:323:29
92+
|
93+
LL | Self::Foo(_) => self.fmt(f),
94+
| ^^^^^^^^^^^
95+
96+
error: aborting due to 12 previous errors
9197

0 commit comments

Comments
 (0)