Add format specifier suggestion #76443
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
jyn514
added
A-diagnostics
|
This is really good! r? @estebank |
There was a problem hiding this comment.
Thank you for your contribution!
I am slightly concerned by making the used formatting specifiers part of the primary multi span, as it semantically implies that there's something problematic about them. Moving that to a note makes it easier to read and I think should help with clarity.
src/test/ui/if/ifmt-bad-arg.stderr
Outdated
There was a problem hiding this comment.
I think that ideally this would look like
error: argument never used
--> $DIR/ifmt-bad-arg.rs:33:22
|
LL | format!("{}", 1, 2);
| ---- ^ argument never used
| |
| formatting specifier missing
note: the following existing formatting specifiers are used
|
LL | format!("{}", 1, 2);
| ^^ - used by positional formatting specifier `0`
| |
| this is positional formatting specifier `0`
In order to accomplish that we would need to
- keep track of the spans for each formatting specifier and a way to refer to them ("positional specifier
0", "named specifierfoo", etc.) - look at the mapping for which formatting specifier is used by which argument
- we'll need to account for named and positional specifiers that are not used
For the proposed output we would need to do something like (simplified):
let note_span: MultiSpan = cx.arg_spans.into();
for span in cx.arg_spans {
note_span.push_span_label(span, "used argument");
}
for span in errs.iter().map(|&(sp, _)| sp) {
note_span.push_span_label(span, "argument used here");
}
err.span_note(note_span, "the following existing formatting specifiers are used");Would you be ok with taking on trying to accomplish this?
There was a problem hiding this comment.
yup, I'll work on making these changes. Thanks for the feedback!
jyn514
added
S-waiting-on-author
|
Ping from triage: |
|
@JohnCSimon I made some progress on this but I'm a bit stuck at the moment, so I asked for help on zulip |
When format specifier is missing from the format string: - Found format specifiers are pointed at - Suggest format specifier if none is present Fix #68293
|
Sorry folks, I don't have much time to commit to this at the moment |
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: rust-lang#68293
Based on discussion in this PR: rust-lang#76443
Let me know if something is missing
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
When format specifier is missing from the format string:
Fix #68293