-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Suggest examples of format specifiers in error messages #146123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment has been minimized.
This comment has been minimized.
r? @estebank though feel free to reassign |
if !found_foreign && invalid_refs.is_empty() { | ||
// Show example if user didn't use any format specifiers | ||
let show_example = used.iter().all(|used| !used); | ||
|
||
if !show_example && unused.len() > 1 { | ||
diag.note(format!("consider adding {} format specifiers", unused.len())); | ||
} | ||
|
||
let original_fmt_str = if fmt_str.len() >= 1 { &fmt_str[..fmt_str.len() - 1] } else { "" }; | ||
|
||
if show_example && unused.len() == 1 { | ||
diag.note("format specifiers use curly braces: `{}`"); | ||
|
||
diag.span_suggestion_verbose( | ||
fmt_span, | ||
"consider adding format specifier", | ||
format!("\"{}{{}}\"", original_fmt_str), | ||
Applicability::MaybeIncorrect, | ||
); | ||
} | ||
|
||
if show_example && unused.len() > 1 { | ||
diag.note("format specifiers use curly braces: `{}`"); | ||
|
||
let mut suggest_fixed_fmt = format!("\"{}", original_fmt_str); | ||
for _ in &unused { | ||
suggest_fixed_fmt.push_str("{}"); | ||
} | ||
suggest_fixed_fmt.push('"'); | ||
|
||
diag.span_suggestion_verbose( | ||
fmt_span, | ||
format!("consider adding {} format specifiers", unused.len()), | ||
suggest_fixed_fmt, | ||
Applicability::MaybeIncorrect, | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if !found_foreign && invalid_refs.is_empty() { | |
// Show example if user didn't use any format specifiers | |
let show_example = used.iter().all(|used| !used); | |
if !show_example && unused.len() > 1 { | |
diag.note(format!("consider adding {} format specifiers", unused.len())); | |
} | |
let original_fmt_str = if fmt_str.len() >= 1 { &fmt_str[..fmt_str.len() - 1] } else { "" }; | |
if show_example && unused.len() == 1 { | |
diag.note("format specifiers use curly braces: `{}`"); | |
diag.span_suggestion_verbose( | |
fmt_span, | |
"consider adding format specifier", | |
format!("\"{}{{}}\"", original_fmt_str), | |
Applicability::MaybeIncorrect, | |
); | |
} | |
if show_example && unused.len() > 1 { | |
diag.note("format specifiers use curly braces: `{}`"); | |
let mut suggest_fixed_fmt = format!("\"{}", original_fmt_str); | |
for _ in &unused { | |
suggest_fixed_fmt.push_str("{}"); | |
} | |
suggest_fixed_fmt.push('"'); | |
diag.span_suggestion_verbose( | |
fmt_span, | |
format!("consider adding {} format specifiers", unused.len()), | |
suggest_fixed_fmt, | |
Applicability::MaybeIncorrect, | |
); | |
} | |
} | |
if !found_foreign && invalid_refs.is_empty() { | |
// Show example if user didn't use any format specifiers | |
let show_example = used.iter().all(|used| !used); | |
if !show_example { | |
if unused.len() > 1 { | |
diag.note(format!("consider adding {} format specifiers", unused.len())); | |
} | |
} else { | |
let original_fmt_str = if fmt_str.len() >= 1 { &fmt_str[..fmt_str.len() - 1] } else { "" }; | |
let msg = if unused.len() == 1 { | |
"a format specifier".to_string() | |
} else { | |
format!("{} format specifiers", unused.len()) | |
}; | |
let sugg = format!("\"{}{}\"", original_fmt_str, "{}".repeat(unused.len())); | |
let msg = format!("format specifiers use curly braces, consider adding {msg}"); | |
diag.span_suggestion_verbose(fmt_span, msg, sugg, Applicability::MaybeIncorrect); | |
} | |
} |
tests/ui/fmt/ifmt-bad-arg.stderr
Outdated
= note: format specifiers use curly braces: `{}` | ||
help: consider adding 2 format specifiers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having both a note and a suggestion talking about the same thing is redundant. Instead, make the message of the suggestion also work as the explanation.
2100565
to
43a6f56
Compare
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
@rustbot label -S-waiting-on-author +S-waiting-on-review |
@bors r+ rollup |
Rollup of 8 pull requests Successful merges: - #145327 (std: make address resolution weirdness local to SGX) - #145879 (default auto traits: use default supertraits instead of `Self: Trait` bounds on associated items) - #146123 (Suggest examples of format specifiers in error messages) - #146311 (Minor symbol comment fixes.) - #146322 (Make Barrier RefUnwindSafe again) - #146327 (Add tests for deref on pin) - #146340 (Strip frontmatter in fewer places) - #146342 (Improve C-variadic error messages: part 2) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #146123 - IoaNNUwU:issue-68293, r=estebank Suggest examples of format specifiers in error messages Format macro now suggests adding `{}` if no formatting specifiers are present. It also gives an example: ```rust LL | println!("Hello", "World"); | ------- ^^^^^^^ argument never used | | | formatting specifier missing | = note: format specifiers use curly braces: `{}` help: consider adding format specifier | LL | println!("Hello{}", "World"); | ++ ``` When one or more `{}` are present, it doesn't show 'format specifiers use curly braces: `{}`' and example, just small hint on how many you missing: ```rust LL | println!("list: {}", 1, 2, 3); | ---------- ^ ^ argument never used | | | | | argument never used | multiple missing formatting specifiers | = help: consider adding 2 format specifiers ``` Original issue: #68293 Based on discussion in this PR: #76443 Let me know if something is missing
Fix panic and incorrectly suggested examples in `format_args` macro. Follow up on rust-lang#146123 Fixes: rust-lang#146446 r? `@estebank`
Fix panic and incorrectly suggested examples in `format_args` macro. Follow up on rust-lang/rust#146123 Fixes: rust-lang/rust#146446 r? `@estebank`
Format macro now suggests adding
{}
if no formatting specifiers are present. It also gives an example:When one or more
{}
are present, it doesn't show 'format specifiers use curly braces:{}
' and example, just small hint on how many you missing:Original issue: #68293
Based on discussion in this PR: #76443
Let me know if something is missing