Skip to content

Commit 2100565

Browse files
committed
Implement better suggestions based on additional tests and other code paths
1 parent a15edbb commit 2100565

File tree

6 files changed

+78
-29
lines changed

6 files changed

+78
-29
lines changed

compiler/rustc_builtin_macros/src/format.rs

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,7 @@ fn make_format_args(
565565
&used,
566566
&args,
567567
&pieces,
568+
&invalid_refs,
568569
detect_foreign_fmt,
569570
str_style,
570571
fmt_str,
@@ -645,6 +646,7 @@ fn report_missing_placeholders(
645646
used: &[bool],
646647
args: &FormatArguments,
647648
pieces: &[parse::Piece<'_>],
649+
invalid_refs: &[(usize, Option<Span>, PositionUsedAs, FormatArgPositionKind)],
648650
detect_foreign_fmt: bool,
649651
str_style: Option<usize>,
650652
fmt_str: &str,
@@ -758,31 +760,47 @@ fn report_missing_placeholders(
758760
check_foreign!(shell);
759761
}
760762
}
761-
if !found_foreign {
762-
if unused.len() == 1 {
763-
diag.span_label(fmt_span, "formatting specifier missing");
763+
if !found_foreign && unused.len() == 1 {
764+
diag.span_label(fmt_span, "formatting specifier missing");
765+
}
766+
767+
if !found_foreign && invalid_refs.is_empty() {
768+
// Show example if user didn't use any format specifiers
769+
let show_example = used.iter().all(|used| !used);
770+
771+
if !show_example && unused.len() > 1 {
772+
diag.note(format!("consider adding {} format specifiers", unused.len()));
764773
}
765-
if used.iter().all(|used| !used) {
774+
775+
let original_fmt_str = if fmt_str.len() >= 1 { &fmt_str[..fmt_str.len() - 1] } else { "" };
776+
777+
if show_example && unused.len() == 1 {
766778
diag.note("format specifiers use curly braces: `{}`");
767-
}
768779

769-
let mut suggest_fixed_fmt = format!("\"{}", &fmt_str[..fmt_str.len() - 1]);
770-
for _ in &unused {
771-
suggest_fixed_fmt.push_str("{}");
780+
diag.span_suggestion_verbose(
781+
fmt_span,
782+
"consider adding format specifier",
783+
format!("\"{}{{}}\"", original_fmt_str),
784+
Applicability::MaybeIncorrect,
785+
);
772786
}
773-
suggest_fixed_fmt.push('"');
774787

775-
let suggest_fmt_count = if unused.len() == 1 {
776-
"consider adding format specifier".to_string()
777-
} else {
778-
format!("consider adding {} format specifiers", unused.len())
779-
};
780-
diag.span_suggestion_verbose(
781-
fmt_span,
782-
suggest_fmt_count,
783-
suggest_fixed_fmt,
784-
Applicability::MaybeIncorrect,
785-
);
788+
if show_example && unused.len() > 1 {
789+
diag.note("format specifiers use curly braces: `{}`");
790+
791+
let mut suggest_fixed_fmt = format!("\"{}", original_fmt_str);
792+
for _ in &unused {
793+
suggest_fixed_fmt.push_str("{}");
794+
}
795+
suggest_fixed_fmt.push('"');
796+
797+
diag.span_suggestion_verbose(
798+
fmt_span,
799+
format!("consider adding {} format specifiers", unused.len()),
800+
suggest_fixed_fmt,
801+
Applicability::MaybeIncorrect,
802+
);
803+
}
786804
}
787805

788806
diag.emit();

tests/ui/fmt/ifmt-bad-arg.stderr

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ LL | format!("", 1, 2);
6262
| | |
6363
| | argument never used
6464
| multiple missing formatting specifiers
65+
|
66+
= note: format specifiers use curly braces: `{}`
67+
help: consider adding 2 format specifiers
68+
|
69+
LL | format!("{}{}", 1, 2);
70+
| ++++
6571

6672
error: argument never used
6773
--> $DIR/ifmt-bad-arg.rs:33:22
@@ -102,6 +108,12 @@ LL | format!("", foo=2);
102108
| -- ^ named argument never used
103109
| |
104110
| formatting specifier missing
111+
|
112+
= note: format specifiers use curly braces: `{}`
113+
help: consider adding format specifier
114+
|
115+
LL | format!("{}", foo=2);
116+
| ++
105117

106118
error: multiple unused formatting arguments
107119
--> $DIR/ifmt-bad-arg.rs:38:32
@@ -111,6 +123,8 @@ LL | format!("{} {}", 1, 2, foo=1, bar=2);
111123
| | |
112124
| | named argument never used
113125
| multiple missing formatting specifiers
126+
|
127+
= note: consider adding 2 format specifiers
114128

115129
error: duplicate argument named `foo`
116130
--> $DIR/ifmt-bad-arg.rs:40:29

tests/ui/macros/format-unused-lables.stderr

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ LL | println!("Test", 123, 456, 789);
77
| | | argument never used
88
| | argument never used
99
| multiple missing formatting specifiers
10+
|
11+
= note: format specifiers use curly braces: `{}`
12+
help: consider adding 3 format specifiers
13+
|
14+
LL | println!("Test{}{}{}", 123, 456, 789);
15+
| ++++++
1016

1117
error: multiple unused formatting arguments
1218
--> $DIR/format-unused-lables.rs:6:9
@@ -19,6 +25,12 @@ LL | 456,
1925
| ^^^ argument never used
2026
LL | 789
2127
| ^^^ argument never used
28+
|
29+
= note: format specifiers use curly braces: `{}`
30+
help: consider adding 3 format specifiers
31+
|
32+
LL | println!("Test2{}{}{}",
33+
| ++++++
2234

2335
error: named argument never used
2436
--> $DIR/format-unused-lables.rs:11:35
@@ -27,6 +39,12 @@ LL | println!("Some stuff", UNUSED="args");
2739
| ------------ ^^^^^^ named argument never used
2840
| |
2941
| formatting specifier missing
42+
|
43+
= note: format specifiers use curly braces: `{}`
44+
help: consider adding format specifier
45+
|
46+
LL | println!("Some stuff{}", UNUSED="args");
47+
| ++
3048

3149
error: multiple unused formatting arguments
3250
--> $DIR/format-unused-lables.rs:14:9

tests/ui/mir/unsized-extern-static.stderr

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ LL | println!("C", unsafe { &symbol });
55
| --- ^^^^^^^^^^^^^^^^^^ argument never used
66
| |
77
| formatting specifier missing
8+
|
9+
= note: format specifiers use curly braces: `{}`
10+
help: consider adding format specifier
11+
|
12+
LL | println!("C{}", unsafe { &symbol });
13+
| ++
814

915
error[E0277]: the size for values of type `[i8]` cannot be known at compilation time
1016
--> $DIR/unsized-extern-static.rs:6:5

tests/ui/suggestions/missing-format-specifiers-issue-68293.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ fn missing_format_specifiers_multiple_unused_args() {
2929
//~| NOTE multiple missing formatting specifiers
3030
//~| NOTE argument never used
3131
//~| NOTE argument never used
32+
//~| NOTE consider adding 2 format specifiers
3233
}
3334

3435
fn main() { }

tests/ui/suggestions/missing-format-specifiers-issue-68293.stderr

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,6 @@ LL | println!("list: {}, {}", 1, 2, 3);
3535
| -------------- ^ argument never used
3636
| |
3737
| formatting specifier missing
38-
|
39-
help: consider adding format specifier
40-
|
41-
LL | println!("list: {}, {}{}", 1, 2, 3);
42-
| ++
4338

4439
error: multiple unused formatting arguments
4540
--> $DIR/missing-format-specifiers-issue-68293.rs:27:29
@@ -50,10 +45,7 @@ LL | println!("list: {}", 1, 2, 3);
5045
| | argument never used
5146
| multiple missing formatting specifiers
5247
|
53-
help: consider adding 2 format specifiers
54-
|
55-
LL | println!("list: {}{}{}", 1, 2, 3);
56-
| ++++
48+
= note: consider adding 2 format specifiers
5749

5850
error: aborting due to 4 previous errors
5951

0 commit comments

Comments
 (0)