Skip to content

Commit a769633

Browse files
author
Linus Kirkwood
committed
Lint nested format_args! for uninlined args
Previously a `format_args!` call or a macro marked `clippy::format_args` would not be linted for uninlined arguments when one of the arguments was in turn a `format_args!` call.
1 parent 5800532 commit a769633

File tree

5 files changed

+101
-16
lines changed

5 files changed

+101
-16
lines changed

clippy_lints/src/format_args.rs

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs<'tcx> {
295295
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
296296
if let Some(macro_call) = root_macro_call_first_node(cx, expr)
297297
&& is_format_macro(cx, macro_call.def_id)
298-
&& let Some(format_args) = self.format_args.get(cx, expr, macro_call.expn)
298+
&& let Some(mut format_args) = self.format_args.get(cx, expr, macro_call.expn)
299299
{
300300
let mut linter = FormatArgsExpr {
301301
cx,
@@ -312,8 +312,34 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs<'tcx> {
312312
linter.check_trailing_comma();
313313
linter.check_templates();
314314

315-
if self.msrv.meets(cx, msrvs::FORMAT_ARGS_CAPTURE) {
316-
linter.check_uninlined_args();
315+
if !self.msrv.meets(cx, msrvs::FORMAT_ARGS_CAPTURE) {
316+
return;
317+
}
318+
319+
let mut uninlined_queue = vec![];
320+
loop {
321+
if linter.check_uninlined_args() {
322+
break;
323+
}
324+
325+
uninlined_queue.extend(self.format_args.get_nested(format_args));
326+
if let Some(inner_format_args) = uninlined_queue.pop() {
327+
format_args = inner_format_args;
328+
329+
linter = FormatArgsExpr {
330+
cx,
331+
expr,
332+
macro_call: &macro_call,
333+
format_args,
334+
ignore_mixed: self.ignore_mixed,
335+
msrv: &self.msrv,
336+
ty_msrv_map: &self.ty_msrv_map,
337+
has_derived_debug: &mut self.has_derived_debug,
338+
has_pointer_format: &mut self.has_pointer_format,
339+
};
340+
} else {
341+
break;
342+
}
317343
}
318344
}
319345
}
@@ -442,16 +468,16 @@ impl<'tcx> FormatArgsExpr<'_, 'tcx> {
442468
}
443469
}
444470

445-
fn check_uninlined_args(&self) {
471+
fn check_uninlined_args(&self) -> bool {
446472
if self.format_args.span.from_expansion() {
447-
return;
473+
return false;
448474
}
449475
if self.macro_call.span.edition() < Edition2021
450476
&& (is_panic(self.cx, self.macro_call.def_id) || is_assert_macro(self.cx, self.macro_call.def_id))
451477
{
452478
// panic!, assert!, and debug_assert! before 2021 edition considers a single string argument as
453479
// non-format
454-
return;
480+
return false;
455481
}
456482

457483
let mut fixes = Vec::new();
@@ -462,12 +488,12 @@ impl<'tcx> FormatArgsExpr<'_, 'tcx> {
462488
// Example of an un-inlinable format: print!("{}{1}", foo, 2)
463489
for (pos, usage) in self.format_arg_positions() {
464490
if !self.check_one_arg(pos, usage, &mut fixes) {
465-
return;
491+
return !fixes.is_empty();
466492
}
467493
}
468494

469495
if fixes.is_empty() {
470-
return;
496+
return false;
471497
}
472498

473499
// multiline span display suggestion is sometimes broken: https://github.com/rust-lang/rust/pull/102729#discussion_r988704308
@@ -494,6 +520,8 @@ impl<'tcx> FormatArgsExpr<'_, 'tcx> {
494520
);
495521
},
496522
);
523+
524+
true
497525
}
498526

499527
fn check_one_arg(&self, pos: &FormatArgPosition, usage: FormatParamUsage, fixes: &mut Vec<(Span, String)>) -> bool {

clippy_utils/src/macros.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,25 @@ impl FormatArgsStorage {
433433
self.0.get()?.get(&format_args_expr.span.with_parent(None))
434434
}
435435

436+
/// Returns AST [`FormatArgs`] nodes if there are any nested inside `parent_format_args`.
437+
pub fn get_nested(&self, parent_format_args: &FormatArgs) -> Vec<&FormatArgs> {
438+
let mut nested = Vec::new();
439+
let format_args_map = match self.0.get() {
440+
Some(map) => map,
441+
None => return nested,
442+
};
443+
444+
for arg in parent_format_args.arguments.all_args() {
445+
if matches!(arg.expr.kind, rustc_ast::ExprKind::FormatArgs(_))
446+
&& let Some(format_args) = format_args_map.get(&arg.expr.span.with_parent(None))
447+
{
448+
nested.push(format_args);
449+
}
450+
}
451+
452+
nested
453+
}
454+
436455
/// Should only be called by `FormatArgsCollector`
437456
pub fn set(&self, format_args: FxHashMap<Span, FormatArgs>) {
438457
self.0

tests/ui/uninlined_format_args.fixed

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -376,9 +376,10 @@ fn nested_format_args_user() {
376376
let local_i32 = 1;
377377
let local_f64 = 2.0;
378378

379-
// false negative: should warn but currently doesn't because the inner format_args
380-
// is not processed when it's used as an argument to another format_args
381-
nested_format_args!("{}", local_i32);
382-
nested_format_args!("val='{}'", local_i32);
383-
nested_format_args!("{:.1}", local_f64);
379+
nested_format_args!("{local_i32}");
380+
//~^ uninlined_format_args
381+
nested_format_args!("val='{local_i32}'");
382+
//~^ uninlined_format_args
383+
nested_format_args!("{local_f64:.1}");
384+
//~^ uninlined_format_args
384385
}

tests/ui/uninlined_format_args.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -381,9 +381,10 @@ fn nested_format_args_user() {
381381
let local_i32 = 1;
382382
let local_f64 = 2.0;
383383

384-
// false negative: should warn but currently doesn't because the inner format_args
385-
// is not processed when it's used as an argument to another format_args
386384
nested_format_args!("{}", local_i32);
385+
//~^ uninlined_format_args
387386
nested_format_args!("val='{}'", local_i32);
387+
//~^ uninlined_format_args
388388
nested_format_args!("{:.1}", local_f64);
389+
//~^ uninlined_format_args
389390
}

tests/ui/uninlined_format_args.stderr

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -895,5 +895,41 @@ LL - usr_println!(true, "{:.1}", local_f64);
895895
LL + usr_println!(true, "{local_f64:.1}");
896896
|
897897

898-
error: aborting due to 75 previous errors
898+
error: variables can be used directly in the `format!` string
899+
--> tests/ui/uninlined_format_args.rs:384:5
900+
|
901+
LL | nested_format_args!("{}", local_i32);
902+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
903+
|
904+
help: change this to
905+
|
906+
LL - nested_format_args!("{}", local_i32);
907+
LL + nested_format_args!("{local_i32}");
908+
|
909+
910+
error: variables can be used directly in the `format!` string
911+
--> tests/ui/uninlined_format_args.rs:386:5
912+
|
913+
LL | nested_format_args!("val='{}'", local_i32);
914+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
915+
|
916+
help: change this to
917+
|
918+
LL - nested_format_args!("val='{}'", local_i32);
919+
LL + nested_format_args!("val='{local_i32}'");
920+
|
921+
922+
error: variables can be used directly in the `format!` string
923+
--> tests/ui/uninlined_format_args.rs:388:5
924+
|
925+
LL | nested_format_args!("{:.1}", local_f64);
926+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
927+
|
928+
help: change this to
929+
|
930+
LL - nested_format_args!("{:.1}", local_f64);
931+
LL + nested_format_args!("{local_f64:.1}");
932+
|
933+
934+
error: aborting due to 78 previous errors
899935

0 commit comments

Comments
 (0)