Skip to content

Commit 12ff6f2

Browse files
francorbachofrancorbacho
committed
Suggest removing redundant arguments in format!()
1 parent c587fd4 commit 12ff6f2

File tree

3 files changed

+180
-4
lines changed

3 files changed

+180
-4
lines changed

‎compiler/rustc_builtin_macros/src/format.rs

+80-4
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use parse::Position::ArgumentNamed;
12
use rustc_ast::ptr::P;
23
use rustc_ast::tokenstream::TokenStream;
34
use rustc_ast::{token, StmtKind};
@@ -6,7 +7,7 @@ use rustc_ast::{
67
FormatArgsPiece, FormatArgument, FormatArgumentKind, FormatArguments, FormatCount,
78
FormatDebugHex, FormatOptions, FormatPlaceholder, FormatSign, FormatTrait,
89
};
9-
use rustc_data_structures::fx::FxHashSet;
10+
use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
1011
use rustc_errors::{Applicability, MultiSpan, PResult, SingleLabelManySpans};
1112
use rustc_expand::base::{self, *};
1213
use rustc_parse_format as parse;
@@ -348,8 +349,8 @@ fn make_format_args(
348349
let mut unfinished_literal = String::new();
349350
let mut placeholder_index = 0;
350351

351-
for piece in pieces {
352-
match piece {
352+
for piece in &pieces {
353+
match *piece {
353354
parse::Piece::String(s) => {
354355
unfinished_literal.push_str(s);
355356
}
@@ -497,7 +498,17 @@ fn make_format_args(
497498
// If there's a lot of unused arguments,
498499
// let's check if this format arguments looks like another syntax (printf / shell).
499500
let detect_foreign_fmt = unused.len() > args.explicit_args().len() / 2;
500-
report_missing_placeholders(ecx, unused, detect_foreign_fmt, str_style, fmt_str, fmt_span);
501+
report_missing_placeholders(
502+
ecx,
503+
unused,
504+
&used,
505+
&args,
506+
&pieces,
507+
detect_foreign_fmt,
508+
str_style,
509+
fmt_str,
510+
fmt_span,
511+
);
501512
}
502513

503514
// Only check for unused named argument names if there are no other errors to avoid causing
@@ -564,6 +575,9 @@ fn invalid_placeholder_type_error(
564575
fn report_missing_placeholders(
565576
ecx: &mut ExtCtxt<'_>,
566577
unused: Vec<(Span, bool)>,
578+
used: &[bool],
579+
args: &FormatArguments,
580+
pieces: &[parse::Piece<'_>],
567581
detect_foreign_fmt: bool,
568582
str_style: Option<usize>,
569583
fmt_str: &str,
@@ -582,6 +596,68 @@ fn report_missing_placeholders(
582596
})
583597
};
584598

599+
let placeholders = pieces
600+
.iter()
601+
.filter_map(|piece| {
602+
if let parse::Piece::NextArgument(argument) = piece && let ArgumentNamed(binding) = argument.position {
603+
let span = fmt_span.from_inner(InnerSpan::new(argument.position_span.start, argument.position_span.end));
604+
Some((span, binding))
605+
} else { None }
606+
})
607+
.collect::<Vec<_>>();
608+
609+
let mut args_spans = vec![];
610+
let mut fmt_spans = FxIndexSet::default();
611+
612+
for (i, unnamed_arg) in args.unnamed_args().iter().enumerate().rev() {
613+
let Some(ty) = unnamed_arg.expr.to_ty() else { continue };
614+
let Some(argument_binding) = ty.kind.is_simple_path() else { continue };
615+
let argument_binding = argument_binding.as_str();
616+
617+
if used[i] {
618+
continue;
619+
}
620+
621+
let matching_placeholders = placeholders
622+
.iter()
623+
.filter(|(_, inline_binding)| argument_binding == *inline_binding)
624+
.collect::<Vec<_>>();
625+
626+
if !matching_placeholders.is_empty() {
627+
args_spans.push(unnamed_arg.expr.span);
628+
for placeholder in &matching_placeholders {
629+
fmt_spans.insert(*placeholder);
630+
}
631+
}
632+
}
633+
634+
if !args_spans.is_empty() {
635+
let mut multispan = MultiSpan::from(args_spans.clone());
636+
637+
let msg = if fmt_spans.len() > 1 {
638+
"the formatting strings already captures the bindings \
639+
directly, they don't need to be included in the argument list"
640+
} else {
641+
"the formatting string already captures the binding \
642+
directly, it doesn't need to be included in the argument list"
643+
};
644+
645+
for (span, binding) in fmt_spans {
646+
multispan.push_span_label(
647+
*span,
648+
format!("this formatting specifier is referencing the `{binding}` binding"),
649+
);
650+
}
651+
652+
for span in &args_spans {
653+
multispan.push_span_label(*span, "this can be removed");
654+
}
655+
656+
diag.span_help(multispan, msg);
657+
diag.emit();
658+
return;
659+
}
660+
585661
// Used to ensure we only report translations for *one* kind of foreign format.
586662
let mut found_foreign = false;
587663

‎tests/ui/did_you_mean/issue-105225.rs

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
fn main() {
2+
let x = 0;
3+
println!("{x}", x);
4+
//~^ ERROR: argument never used
5+
6+
println!("{x} {}", x, x);
7+
//~^ ERROR: argument never used
8+
9+
println!("{} {x}", x, x);
10+
//~^ ERROR: argument never used
11+
12+
let y = 0;
13+
println!("{x} {y}", x, y);
14+
//~^ ERROR: multiple unused formatting arguments
15+
16+
let y = 0;
17+
println!("{} {} {x} {y} {}", x, x, x, y, y);
18+
//~^ ERROR: multiple unused formatting arguments
19+
}

‎tests/ui/did_you_mean/issue-105225.stderr

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
error: argument never used
2+
--> $DIR/issue-105225.rs:3:21
3+
|
4+
LL | println!("{x}", x);
5+
| ^ argument never used
6+
|
7+
help: the formatting string already captures the binding directly, it doesn't need to be included in the argument list
8+
--> $DIR/issue-105225.rs:3:21
9+
|
10+
LL | println!("{x}", x);
11+
| - ^ this can be removed
12+
| |
13+
| this formatting specifier is referencing the `x` binding
14+
15+
error: argument never used
16+
--> $DIR/issue-105225.rs:6:27
17+
|
18+
LL | println!("{x} {}", x, x);
19+
| ^ argument never used
20+
|
21+
help: the formatting string already captures the binding directly, it doesn't need to be included in the argument list
22+
--> $DIR/issue-105225.rs:6:27
23+
|
24+
LL | println!("{x} {}", x, x);
25+
| - ^ this can be removed
26+
| |
27+
| this formatting specifier is referencing the `x` binding
28+
29+
error: argument never used
30+
--> $DIR/issue-105225.rs:9:27
31+
|
32+
LL | println!("{} {x}", x, x);
33+
| ^ argument never used
34+
|
35+
help: the formatting string already captures the binding directly, it doesn't need to be included in the argument list
36+
--> $DIR/issue-105225.rs:9:27
37+
|
38+
LL | println!("{} {x}", x, x);
39+
| - ^ this can be removed
40+
| |
41+
| this formatting specifier is referencing the `x` binding
42+
43+
error: multiple unused formatting arguments
44+
--> $DIR/issue-105225.rs:13:25
45+
|
46+
LL | println!("{x} {y}", x, y);
47+
| --------- ^ ^ argument never used
48+
| | |
49+
| | argument never used
50+
| multiple missing formatting specifiers
51+
|
52+
help: the formatting strings already captures the bindings directly, they don't need to be included in the argument list
53+
--> $DIR/issue-105225.rs:13:25
54+
|
55+
LL | println!("{x} {y}", x, y);
56+
| - - ^ ^ this can be removed
57+
| | | |
58+
| | | this can be removed
59+
| | this formatting specifier is referencing the `y` binding
60+
| this formatting specifier is referencing the `x` binding
61+
62+
error: multiple unused formatting arguments
63+
--> $DIR/issue-105225.rs:17:43
64+
|
65+
LL | println!("{} {} {x} {y} {}", x, x, x, y, y);
66+
| ------------------ ^ ^ argument never used
67+
| | |
68+
| | argument never used
69+
| multiple missing formatting specifiers
70+
|
71+
help: the formatting string already captures the binding directly, it doesn't need to be included in the argument list
72+
--> $DIR/issue-105225.rs:17:43
73+
|
74+
LL | println!("{} {} {x} {y} {}", x, x, x, y, y);
75+
| - ^ ^ this can be removed
76+
| | |
77+
| | this can be removed
78+
| this formatting specifier is referencing the `y` binding
79+
80+
error: aborting due to 5 previous errors
81+

0 commit comments

Comments
 (0)