Skip to content

Commit 03fe775

Browse files
francorbachofrancorbacho
committed
Use diagnostic impls and add suggestions in redundant format!() args
1 parent 5a91715 commit 03fe775

File tree

5 files changed

+110
-79
lines changed

5 files changed

+110
-79
lines changed

‎compiler/rustc_builtin_macros/messages.ftl

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,17 @@ builtin_macros_format_positional_after_named = positional arguments cannot follo
137137
.label = positional arguments must be before named arguments
138138
.named_args = named argument
139139
140+
builtin_macros_format_redundant_args = redundant argument
141+
.help = {$n ->
142+
[one] the formatting string already captures the binding directly, it doesn't need to be included in the argument list
143+
*[more] the formatting strings already captures the bindings directly, they don't need to be included in the argument list
144+
}
145+
.note = {$n ->
146+
[one] the formatting specifier is referencing the binding already
147+
*[more] the formatting specifiers are referencing the bindings already
148+
}
149+
.suggestion = this can be removed
150+
140151
builtin_macros_format_requires_string = requires at least a format string argument
141152
142153
builtin_macros_format_string_invalid = invalid format string: {$desc}

‎compiler/rustc_builtin_macros/src/errors.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,27 @@ pub(crate) struct FormatPositionalMismatch {
635635
pub(crate) highlight: SingleLabelManySpans,
636636
}
637637

638+
#[derive(Diagnostic)]
639+
#[diag(builtin_macros_format_redundant_args)]
640+
pub(crate) struct FormatRedundantArgs {
641+
#[primary_span]
642+
pub(crate) fmt_span: Span,
643+
pub(crate) n: usize,
644+
645+
#[note]
646+
pub(crate) note: MultiSpan,
647+
648+
#[subdiagnostic]
649+
pub(crate) sugg: FormatRedundantArgsSugg,
650+
}
651+
652+
#[derive(Subdiagnostic)]
653+
#[multipart_suggestion(builtin_macros_suggestion, applicability = "machine-applicable")]
654+
pub(crate) struct FormatRedundantArgsSugg {
655+
#[suggestion_part(code = "")]
656+
pub(crate) spans: Vec<Span>,
657+
}
658+
638659
#[derive(Diagnostic)]
639660
#[diag(builtin_macros_test_case_non_item)]
640661
pub(crate) struct TestCaseNonItem {

‎compiler/rustc_builtin_macros/src/format.rs

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ use rustc_ast::{
77
FormatArgsPiece, FormatArgument, FormatArgumentKind, FormatArguments, FormatCount,
88
FormatDebugHex, FormatOptions, FormatPlaceholder, FormatSign, FormatTrait,
99
};
10-
use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
11-
use rustc_errors::{Applicability, DiagnosticBuilder, MultiSpan, PResult, SingleLabelManySpans};
10+
use rustc_data_structures::fx::FxHashSet;
11+
use rustc_errors::{Applicability, MultiSpan, PResult, SingleLabelManySpans};
1212
use rustc_expand::base::{self, *};
1313
use rustc_parse_format as parse;
1414
use rustc_span::symbol::{Ident, Symbol};
15-
use rustc_span::{BytePos, ErrorGuaranteed, InnerSpan, Span};
15+
use rustc_span::{BytePos, InnerSpan, Span};
1616

1717
use rustc_lint_defs::builtin::NAMED_ARGUMENTS_USED_POSITIONALLY;
1818
use rustc_lint_defs::{BufferedEarlyLint, BuiltinLintDiagnostics, LintId};
@@ -607,7 +607,9 @@ fn report_missing_placeholders(
607607
.collect::<Vec<_>>();
608608

609609
if !placeholders.is_empty() {
610-
report_redundant_placeholders(&mut diag, &args, used, placeholders);
610+
report_redundant_placeholders(ecx, fmt_span, &args, used, placeholders);
611+
diag.cancel();
612+
return;
611613
}
612614

613615
// Used to ensure we only report translations for *one* kind of foreign format.
@@ -698,13 +700,16 @@ fn report_missing_placeholders(
698700
}
699701

700702
fn report_redundant_placeholders(
701-
diag: &mut DiagnosticBuilder<'_, ErrorGuaranteed>,
703+
ecx: &mut ExtCtxt<'_>,
704+
fmt_span: Span,
702705
args: &FormatArguments,
703706
used: &[bool],
704707
placeholders: Vec<(Span, &str)>,
705708
) {
709+
let mut fmt_arg_indices = vec![];
706710
let mut args_spans = vec![];
707-
let mut fmt_spans = FxIndexSet::default();
711+
let mut fmt_spans = vec![];
712+
let mut bindings = vec![];
708713

709714
for (i, unnamed_arg) in args.unnamed_args().iter().enumerate().rev() {
710715
let Some(ty) = unnamed_arg.expr.to_ty() else { continue };
@@ -721,36 +726,39 @@ fn report_redundant_placeholders(
721726
.collect::<Vec<_>>();
722727

723728
if !matching_placeholders.is_empty() {
729+
fmt_arg_indices.push(i);
724730
args_spans.push(unnamed_arg.expr.span);
725-
for placeholder in &matching_placeholders {
726-
fmt_spans.insert(*placeholder);
731+
for (span, binding) in &matching_placeholders {
732+
if fmt_spans.contains(span) {
733+
continue;
734+
}
735+
fmt_spans.push(*span);
736+
bindings.push(binding);
727737
}
728738
}
729739
}
730740

731741
if !args_spans.is_empty() {
732-
let mut multispan = MultiSpan::from(args_spans.clone());
742+
let multispan = MultiSpan::from(fmt_spans);
743+
let mut suggestion_spans = vec![];
733744

734-
let msg = if fmt_spans.len() > 1 {
735-
"the formatting strings already captures the bindings \
736-
directly, they don't need to be included in the argument list"
737-
} else {
738-
"the formatting string already captures the binding \
739-
directly, it doesn't need to be included in the argument list"
740-
};
745+
for (arg_span, fmt_arg_idx) in args_spans.iter().zip(fmt_arg_indices.iter()) {
746+
let span = if fmt_arg_idx + 1 == args.explicit_args().len() {
747+
*arg_span
748+
} else {
749+
arg_span.until(args.explicit_args()[*fmt_arg_idx + 1].expr.span)
750+
};
741751

742-
for (span, binding) in fmt_spans {
743-
multispan.push_span_label(
744-
*span,
745-
format!("this formatting specifier is referencing the `{binding}` binding"),
746-
);
752+
suggestion_spans.push(span);
747753
}
748754

749-
for span in &args_spans {
750-
multispan.push_span_label(*span, "this can be removed");
751-
}
755+
let mut diag = ecx.create_err(errors::FormatRedundantArgs {
756+
fmt_span,
757+
note: multispan,
758+
n: args_spans.len(),
759+
sugg: errors::FormatRedundantArgsSugg { spans: suggestion_spans },
760+
});
752761

753-
diag.span_help(multispan, msg);
754762
diag.emit();
755763
}
756764
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
11
fn main() {
22
let x = 0;
33
println!("{x}", x);
4-
//~^ ERROR: argument never used
4+
//~^ ERROR: redundant argument
55

66
println!("{x} {}", x, x);
7-
//~^ ERROR: argument never used
7+
//~^ ERROR: redundant argument
88

99
println!("{} {x}", x, x);
10-
//~^ ERROR: argument never used
10+
//~^ ERROR: redundant argument
1111

1212
let y = 0;
1313
println!("{x} {y}", x, y);
14-
//~^ ERROR: multiple unused formatting arguments
14+
//~^ ERROR: redundant argument
1515

1616
let y = 0;
1717
println!("{} {} {x} {y} {}", x, x, x, y, y);
18-
//~^ ERROR: multiple unused formatting arguments
18+
//~^ ERROR: redundant argument
1919
}

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

Lines changed: 40 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,81 +1,72 @@
1-
error: argument never used
2-
--> $DIR/issue-105225.rs:3:21
1+
error: redundant argument
2+
--> $DIR/issue-105225.rs:3:14
33
|
44
LL | println!("{x}", x);
5-
| ^ argument never used
5+
| ^^^^^ - help: this can be removed
66
|
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
7+
note: the formatting specifier is referencing the binding already
8+
--> $DIR/issue-105225.rs:3:16
99
|
1010
LL | println!("{x}", x);
11-
| - ^ this can be removed
12-
| |
13-
| this formatting specifier is referencing the `x` binding
11+
| ^
1412

15-
error: argument never used
16-
--> $DIR/issue-105225.rs:6:27
13+
error: redundant argument
14+
--> $DIR/issue-105225.rs:6:14
1715
|
1816
LL | println!("{x} {}", x, x);
19-
| ^ argument never used
17+
| ^^^^^^^^ - help: this can be removed
2018
|
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
19+
note: the formatting specifier is referencing the binding already
20+
--> $DIR/issue-105225.rs:6:16
2321
|
2422
LL | println!("{x} {}", x, x);
25-
| - ^ this can be removed
26-
| |
27-
| this formatting specifier is referencing the `x` binding
23+
| ^
2824

29-
error: argument never used
30-
--> $DIR/issue-105225.rs:9:27
25+
error: redundant argument
26+
--> $DIR/issue-105225.rs:9:14
3127
|
3228
LL | println!("{} {x}", x, x);
33-
| ^ argument never used
29+
| ^^^^^^^^ - help: this can be removed
3430
|
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
31+
note: the formatting specifier is referencing the binding already
32+
--> $DIR/issue-105225.rs:9:19
3733
|
3834
LL | println!("{} {x}", x, x);
39-
| - ^ this can be removed
40-
| |
41-
| this formatting specifier is referencing the `x` binding
35+
| ^
4236

43-
error: multiple unused formatting arguments
44-
--> $DIR/issue-105225.rs:13:25
37+
error: redundant argument
38+
--> $DIR/issue-105225.rs:13:14
4539
|
4640
LL | println!("{x} {y}", x, y);
47-
| --------- ^ ^ argument never used
48-
| | |
49-
| | argument never used
50-
| multiple missing formatting specifiers
41+
| ^^^^^^^^^
5142
|
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
43+
note: the formatting specifiers are referencing the bindings already
44+
--> $DIR/issue-105225.rs:13:16
5445
|
5546
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
47+
| ^ ^
48+
help: this can be removed
49+
|
50+
LL - println!("{x} {y}", x, y);
51+
LL + println!("{x} {y}", );
52+
|
6153

62-
error: multiple unused formatting arguments
63-
--> $DIR/issue-105225.rs:17:43
54+
error: redundant argument
55+
--> $DIR/issue-105225.rs:17:14
6456
|
6557
LL | println!("{} {} {x} {y} {}", x, x, x, y, y);
66-
| ------------------ ^ ^ argument never used
67-
| | |
68-
| | argument never used
69-
| multiple missing formatting specifiers
58+
| ^^^^^^^^^^^^^^^^^^
7059
|
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
60+
note: the formatting specifiers are referencing the bindings already
61+
--> $DIR/issue-105225.rs:17:26
7362
|
7463
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
64+
| ^
65+
help: this can be removed
66+
|
67+
LL - println!("{} {} {x} {y} {}", x, x, x, y, y);
68+
LL + println!("{} {} {x} {y} {}", x, x, x, );
69+
|
7970

8071
error: aborting due to 5 previous errors
8172

0 commit comments

Comments
 (0)