Improve handling of invalid references in format!()

estebank · estebank · commit 86f4f68b70fb · 2019-07-29T21:24:10.000-07:00
diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs
@@ -275,19 +275,18 @@ impl<'a, 'b> Context<'a, 'b> {
         } else {
             MultiSpan::from_span(self.fmtsp)
         };
-        let refs_len = self.invalid_refs.len();
-        let mut refs = self
+        let refs = self
             .invalid_refs
             .iter()
             .map(|(r, pos)| (r.to_string(), self.arg_spans.get(*pos)));
 
         let mut zero_based_note = false;
 
-        if self.names.is_empty() && !numbered_position_args {
-            let count = self.pieces.len() + self.arg_with_formatting
-                .iter()
-                .filter(|fmt| fmt.precision_span.is_some())
-                .count();
+        let count = self.pieces.len() + self.arg_with_formatting
+            .iter()
+            .filter(|fmt| fmt.precision_span.is_some())
+            .count();
+        if self.names.is_empty() && !numbered_position_args && count != self.args.len() {
             e = self.ecx.mut_span_err(
                 sp,
                 &format!(
@@ -298,14 +297,22 @@ impl<'a, 'b> Context<'a, 'b> {
                 ),
             );
         } else {
-            let (arg_list, mut sp) = if refs_len == 1 {
-                let (reg, pos) = refs.next().unwrap();
+            let (mut refs, spans): (Vec<_>, Vec<_>) = refs.unzip();
+            // Avoid `invalid reference to positional arguments 7 and 7 (there is 1 argument)`
+            // for `println!("{7:7$}", 1);`
+            refs.dedup();
+            refs.sort();
+            let (arg_list, mut sp) = if refs.len() == 1 {
+                let spans: Vec<_> = spans.into_iter().filter_map(|sp| sp.map(|sp| *sp)).collect();
                 (
-                    format!("argument {}", reg),
-                    MultiSpan::from_span(*pos.unwrap_or(&self.fmtsp)),
+                    format!("argument {}", refs[0]),
+                    if spans.is_empty() {
+                        MultiSpan::from_span(self.fmtsp)
+                    } else {
+                        MultiSpan::from_spans(spans)
+                    },
                 )
             } else {
-                let (mut refs, spans): (Vec<_>, Vec<_>) = refs.unzip();
                 let pos = MultiSpan::from_spans(spans.into_iter().map(|s| *s.unwrap()).collect());
                 let reg = refs.pop().unwrap();
                 (
@@ -754,7 +761,21 @@ impl<'a, 'b> Context<'a, 'b> {
                     "x" => "LowerHex",
                     "X" => "UpperHex",
                     _ => {
-                        ecx.span_err(sp, &format!("unknown format trait `{}`", *tyname));
+                        let mut err = ecx.struct_span_err(
+                            sp,
+                            &format!("unknown format trait `{}`", *tyname),
+                        );
+                        err.note("the only appropriate formatting traits are:\n\
+                                  - ``, which uses the `Display` trait\n\
+                                  - `?`, which uses the `Debug` trait\n\
+                                  - `e`, which uses the `LowerExp` trait\n\
+                                  - `E`, which uses the `UpperExp` trait\n\
+                                  - `o`, which uses the `Octal` trait\n\
+                                  - `p`, which uses the `Pointer` trait\n\
+                                  - `b`, which uses the `Binary` trait\n\
+                                  - `x`, which uses the `LowerHex` trait\n\
+                                  - `X`, which uses the `UpperHex` trait");
+                        err.emit();
                         return DummyResult::raw_expr(sp, true);
                     }
                 }
diff --git a/src/test/ui/if/ifmt-bad-arg.rs b/src/test/ui/if/ifmt-bad-arg.rs
@@ -82,5 +82,8 @@ tenth number: {}",
     //~^ ERROR 4 positional arguments in format string, but there are 3 arguments
     //~| ERROR mismatched types
     println!("{} {:07$} {}", 1, 3.2, 4);
-    //~^ ERROR 3 positional arguments in format string, but there are 3 arguments
+    //~^ ERROR invalid reference to positional argument 7 (there are 3 arguments)
+    println!("{:foo}", 1); //~ ERROR unknown format trait `foo`
+    println!("{5} {:4$} {6:7$}", 1);
+    //~^ ERROR invalid reference to positional arguments 4, 5, 6 and 7 (there is 1 argument)
 }
diff --git a/src/test/ui/if/ifmt-bad-arg.stderr b/src/test/ui/if/ifmt-bad-arg.stderr
@@ -243,17 +243,46 @@ LL |     println!("{} {:07$.*} {}", 1, 3.2, 4);
    = note: positional arguments are zero-based
    = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html
 
-error: 3 positional arguments in format string, but there are 3 arguments
-  --> $DIR/ifmt-bad-arg.rs:84:15
+error: invalid reference to positional argument 7 (there are 3 arguments)
+  --> $DIR/ifmt-bad-arg.rs:84:18
    |
 LL |     println!("{} {:07$} {}", 1, 3.2, 4);
-   |               ^^ ^^---^ ^^
+   |                  ^^---^
    |                    |
    |                    this width flag expects an `usize` argument at position 7, but there are 3 arguments
    |
    = note: positional arguments are zero-based
    = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html
 
+error: unknown format trait `foo`
+  --> $DIR/ifmt-bad-arg.rs:86:24
+   |
+LL |     println!("{:foo}", 1);
+   |                        ^
+   |
+   = note: the only appropriate formatting traits are:
+           - ``, which uses the `Display` trait
+           - `?`, which uses the `Debug` trait
+           - `e`, which uses the `LowerExp` trait
+           - `E`, which uses the `UpperExp` trait
+           - `o`, which uses the `Octal` trait
+           - `p`, which uses the `Pointer` trait
+           - `b`, which uses the `Binary` trait
+           - `x`, which uses the `LowerHex` trait
+           - `X`, which uses the `UpperHex` trait
+
+error: invalid reference to positional arguments 4, 5, 6 and 7 (there is 1 argument)
+  --> $DIR/ifmt-bad-arg.rs:87:15
+   |
+LL |     println!("{5} {:4$} {6:7$}", 1);
+   |               ^^^ ^^--^ ^^^--^
+   |                     |      |
+   |                     |      this width flag expects an `usize` argument at position 7, but there is 1 argument
+   |                     this width flag expects an `usize` argument at position 4, but there is 1 argument
+   |
+   = note: positional arguments are zero-based
+   = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html
+
 error[E0308]: mismatched types
   --> $DIR/ifmt-bad-arg.rs:78:32
    |
@@ -272,6 +301,6 @@ LL |     println!("{} {:07$.*} {}", 1, 3.2, 4);
    = note: expected type `&usize`
               found type `&{float}`
 
-error: aborting due to 33 previous errors
+error: aborting due to 35 previous errors
 
 For more information about this error, try `rustc --explain E0308`.
diff --git a/src/test/ui/if/ifmt-unknown-trait.stderr b/src/test/ui/if/ifmt-unknown-trait.stderr
@@ -3,6 +3,17 @@ error: unknown format trait `notimplemented`
    |
 LL |     format!("{:notimplemented}", "3");
    |                                  ^^^
+   |
+   = note: the only appropriate formatting traits are:
+           - ``, which uses the `Display` trait
+           - `?`, which uses the `Debug` trait
+           - `e`, which uses the `LowerExp` trait
+           - `E`, which uses the `UpperExp` trait
+           - `o`, which uses the `Octal` trait
+           - `p`, which uses the `Pointer` trait
+           - `b`, which uses the `Binary` trait
+           - `x`, which uses the `LowerHex` trait
+           - `X`, which uses the `UpperHex` trait
 
 error: aborting due to previous error
 
