Point spans to inner elements of format strings #52649

estebank · 2018-07-23T15:39:41Z

Point at missing positional specifiers in string literal

error: invalid reference to positional arguments 3, 4 and 5 (there are 3 arguments)
  --> $DIR/ifmt-bad-arg.rs:34:38
   |
LL |     format!("{name} {value} {} {} {} {} {} {}", 0, name=1, value=2);
   |                                      ^^ ^^ ^^
   |
   = note: positional arguments are zero-based

Point at named formatting specifier in string literal

error: there is no argument named `foo`
  --> $DIR/ifmt-bad-arg.rs:37:17
   |
LL |     format!("{} {foo} {} {bar} {}", 1, 2, 3);
   |                 ^^^^^

Update label for formatting string in "multiple unused formatting arguments" to be more correct

error: multiple unused formatting arguments
  --> $DIR/ifmt-bad-arg.rs:42:17
   |
LL |     format!("", 1, 2);               //~ ERROR: multiple unused formatting arguments
   |             --  ^  ^
   |             |
   |             multiple missing formatting specifiers

When using printf string formatting, provide a structured suggestion instead of a note

error: multiple unused formatting arguments
  --> $DIR/format-foreign.rs:12:30
   |
LL |     println!("%.*3$s %s!/n", "Hello,", "World", 4); //~ ERROR multiple unused formatting arguments
   |              --------------  ^^^^^^^^  ^^^^^^^  ^
   |              |
   |              multiple missing formatting specifiers
   |
   = note: printf formatting not supported; see the documentation for `std::fmt`
help: format specifiers in Rust are written using `{}`
   |
LL |     println!("{:.2$} {}!/n", "Hello,", "World", 4); //~ ERROR multiple unused formatting arguments
   |               ^^^^^^ ^^

rust-highfive · 2018-07-23T15:39:52Z

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

estebank · 2018-07-23T15:40:13Z

src/test/ui/macros/macro-backtrace-println.stderr

   |
 LL |     ($fmt:expr) => (myprint!(concat!($fmt, "/n"))); //~ ERROR no arguments were given
-   |                              ^^^^^^^^^^^^^^^^^^^
+   |                               ^^


Do not apply if not a string literal.

estebank · 2018-07-23T15:40:53Z

src/test/ui/ifmt-bad-arg.stderr

+  --> $DIR/ifmt-bad-arg.rs:34:14
+   |
+LL |     format!("{name} {value} {} {} {} {} {} {}", 0, name=1, value=2);
+   |              ^^^^^^ ^^^^^^^ ^^ ^^ ^^ ^^ ^^ ^^


Point only to positional arguments 3, 4 and 5.

estebank · 2018-07-23T15:41:07Z

src/test/ui/ifmt-bad-arg.stderr

+  --> $DIR/ifmt-bad-arg.rs:29:14
+   |
+LL |     format!("{0} {1} {2}", 1, 2);
+   |              ^^^ ^^^ ^^^


Point only to positional argument 2.

alexcrichton · 2018-07-23T21:44:35Z

This looks like a fantastic idea to me! I'll take a closer look with tests passing?

…terals

estebank · 2018-07-23T23:04:35Z

I'm really happy with how it ended up looking.

@alexcrichton do you think I should add a secondary span pointing at the complete string for all of these errors? Also, I feel we should also add labels to the primary spans, but I can do these things in a separate PR.

estebank · 2018-07-24T01:23:22Z

cc @nikomatsakis and @oli-obk who might have ideas around what these should look like.

oli-obk

Absolutely great. This deserves its own blog/reddit/users post. It's gonna blow ppls minds

oli-obk · 2018-07-24T07:55:56Z

src/libfmt_macros/lib.rs

@@ -154,6 +154,7 @@ pub struct Parser<'a> {
    style: Option<usize>,
    /// How many newlines have been seen in the string so far, to adjust the error spans
    seen_newlines: usize,
+    pub arg_places: Vec<(usize, usize)>,


so these are offsets into the format string? A comment would be nice (I'm using this struct in clippy ^^)

Added docs.

oli-obk · 2018-07-24T07:57:57Z

src/test/ui/ifmt-bad-arg.stderr

+  --> $DIR/ifmt-bad-arg.rs:34:38
+   |
+LL |     format!("{name} {value} {} {} {} {} {} {}", 0, name=1, value=2);
+   |                                      ^^ ^^ ^^


this is absolutely great

oli-obk · 2018-07-24T07:58:47Z

src/test/ui/ifmt-bad-arg.stderr

+LL |     format!("", 1, 2);               //~ ERROR: multiple unused formatting arguments
+   |             --  ^  ^
+   |             |
+   |             multiple missing formatting arguments


it's not missing formatting arguments, it's missing formatting specifiers, right?

oli-obk · 2018-07-24T08:00:16Z

src/test/ui/ifmt-bad-arg.stderr

+LL |     format!("{"); //~ ERROR: expected `'}'` but string was terminated
+   |               ^ expected `'}'` in format string
+   |
+   = note: if you intended to print `{`, you can escape it using `{{`


Can you make this a structured suggestion?

I'll look into this as a follow up, because the output would look a bit bad (it'd have to suggest {" as it is now, which is hard to understand).

oli-obk · 2018-07-24T08:01:00Z

src/test/ui/ifmt-bad-arg.stderr

+LL |     format!("foo %s baz", "bar"); //~ ERROR: argument never used
+   |                           ^^^^^
+   |
+   = help: `%s` should be written as `{}`


this has no span associated with it

oli-obk · 2018-07-25T07:16:07Z

@bors r+ wonderful

bors · 2018-07-25T07:16:08Z

📌 Commit 9a893cc has been approved by oli-obk

Point spans to inner elements of format strings - Point at missing positional specifiers in string literal ``` error: invalid reference to positional arguments 3, 4 and 5 (there are 3 arguments) --> $DIR/ifmt-bad-arg.rs:34:38 | LL | format!("{name} {value} {} {} {} {} {} {}", 0, name=1, value=2); | ^^ ^^ ^^ | = note: positional arguments are zero-based ``` - Point at named formatting specifier in string literal ``` error: there is no argument named `foo` --> $DIR/ifmt-bad-arg.rs:37:17 | LL | format!("{} {foo} {} {bar} {}", 1, 2, 3); | ^^^^^ ``` - Update label for formatting string in "multiple unused formatting arguments" to be more correct ``` error: multiple unused formatting arguments --> $DIR/ifmt-bad-arg.rs:42:17 | LL | format!("", 1, 2); //~ ERROR: multiple unused formatting arguments | -- ^ ^ | | | multiple missing formatting specifiers ``` - When using `printf` string formatting, provide a structured suggestion instead of a note ``` error: multiple unused formatting arguments --> $DIR/format-foreign.rs:12:30 | LL | println!("%.*3$s %s!/n", "Hello,", "World", 4); //~ ERROR multiple unused formatting arguments | -------------- ^^^^^^^^ ^^^^^^^ ^ | | | multiple missing formatting specifiers | = note: printf formatting not supported; see the documentation for `std::fmt` help: format specifiers in Rust are written using `{}` | LL | println!("{:.2$} {}!/n", "Hello,", "World", 4); //~ ERROR multiple unused formatting arguments | ^^^^^^ ^^ ```

@ghost

Rollup of 16 pull requests Successful merges: - #52558 (Add tests for ICEs which no longer repro) - #52610 (Clarify what a task is) - #52617 (Don't match on region kinds when reporting NLL errors) - #52635 (Fix #[linkage] propagation though generic functions) - #52647 (Suggest to take and ignore args while closure args count mismatching) - #52649 (Point spans to inner elements of format strings) - #52654 (Format linker args in a way that works for gcc and ld) - #52667 (update the stdsimd submodule) - #52674 (Impl Executor for Box<E: Executor>) - #52690 (ARM: expose `rclass` and `dsp` target features) - #52692 (Improve readability in a few sorts) - #52695 (Hide some lints which are not quite right the way they are reported to the user) - #52718 (State default capacity for BufReader/BufWriter) - #52721 (std::ops::Try impl for std::task::Poll) - #52723 (rustc: Register crates under their real names) - #52734 (sparc ABI issue - structure returning from function is returned in 64bit registers (with tests)) Failed merges: - #52678 ([NLL] Use better spans in some errors) r? @ghost

Use suggestions for shell format arguments Follow up to rust-lang#52649.

Esteban Küber added 2 commits July 22, 2018 23:09

Point at internal span in format string

38abca8

Point at incorrect named arg in format string

4230659

rust-highfive assigned alexcrichton Jul 23, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 23, 2018

estebank commented Jul 23, 2018

View reviewed changes

This comment has been minimized.

Sign in to view

estebank added 2 commits July 23, 2018 15:09

Point only at invalid positional arguments

6bcf877

Only point at inside of string literals if they're actually string li…

c55a698

…terals

estebank changed the title ~~[WIP] Point spans to inner elements of format strings~~ Point spans to inner elements of format strings Jul 23, 2018

This comment has been minimized.

Sign in to view

Add test for raw string

bde2be0

estebank force-pushed the fmt-span branch from 665096a to bde2be0 Compare July 23, 2018 23:03

oli-obk reviewed Jul 24, 2018

View reviewed changes

estebank added 3 commits July 24, 2018 09:46

Add documentation for Parser::arg_places

f487e39

Reword missing formatting arguments label

f9e3762

Use suggestions for printf format

4d8aa59

This comment has been minimized.

Sign in to view

estebank added 3 commits July 24, 2018 18:44

Fix unittest

3298b9f

Rename method and remove commented out code

7bd94e0

Add span label for format str missing specifier

9a893cc

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 25, 2018

Mark-Simulacrum mentioned this pull request Jul 26, 2018

Rollup of 16 pull requests #52735

Merged

bors merged commit 9a893cc into rust-lang:master Jul 26, 2018

This was referenced Jul 31, 2018

Use suggestions for shell format arguments #52888

Merged

Suggest comma when writing println!("{}" a); #52397

Merged

pietroalbini added a commit to pietroalbini/rust that referenced this pull request Aug 1, 2018

Rollup merge of rust-lang#52888 - estebank:shell-sugg, r=oli-obk

a8ed603

Use suggestions for shell format arguments Follow up to rust-lang#52649.

This was referenced Aug 30, 2018

Incorrect span for multiline format string with errors #53836

Closed

Subpar span on multiline format string with unclosed brace #53837

Closed

estebank mentioned this pull request May 28, 2020

Inline asm errors don't report file/line no information #72664

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Point spans to inner elements of format strings #52649

Point spans to inner elements of format strings #52649

estebank commented Jul 23, 2018 •

edited

Loading

rust-highfive commented Jul 23, 2018

estebank Jul 23, 2018

estebank Jul 23, 2018

estebank Jul 23, 2018

This comment has been minimized.

alexcrichton commented Jul 23, 2018

This comment has been minimized.

estebank commented Jul 23, 2018

estebank commented Jul 24, 2018

oli-obk left a comment

oli-obk Jul 24, 2018

estebank Jul 24, 2018

oli-obk Jul 24, 2018

oli-obk Jul 24, 2018

estebank Jul 24, 2018

oli-obk Jul 24, 2018

estebank Jul 24, 2018

oli-obk Jul 24, 2018

estebank Jul 24, 2018

This comment has been minimized.

oli-obk commented Jul 25, 2018

bors commented Jul 25, 2018

Point spans to inner elements of format strings #52649

Point spans to inner elements of format strings #52649

Conversation

estebank commented Jul 23, 2018 • edited Loading

rust-highfive commented Jul 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

alexcrichton commented Jul 23, 2018

This comment has been minimized.

estebank commented Jul 23, 2018

estebank commented Jul 24, 2018

oli-obk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

oli-obk commented Jul 25, 2018

bors commented Jul 25, 2018

estebank commented Jul 23, 2018 •

edited

Loading