On format!() arg count mismatch provide extra info

When positional width and precision formatting flags are present in a
formatting string that has an argument count mismatch, provide extra
information pointing at them making it easiser to understand where the
problem may lay:

```
error: 4 positional arguments in format string, but there are 3 arguments
  --> $DIR/ifmt-bad-arg.rs:78:15
   |
LL |     println!("{} {:.*} {}", 1, 3.2, 4);
   |               ^^ ^^--^ ^^      --- this parameter corresponds to the precision flag
   |                    |
   |                    this precision flag adds an extra required argument at position 1, which is why there are 4 arguments expected
   |
   = note: positional arguments are zero-based
   = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html

error: 4 positional arguments in format string, but there are 3 arguments
  --> $DIR/ifmt-bad-arg.rs:81:15
   |
LL |     println!("{} {:07$.*} {}", 1, 3.2, 4);
   |               ^^ ^^-----^ ^^      --- this parameter corresponds to the precision flag
   |                    |  |
   |                    |  this precision flag adds an extra required argument at position 1, which is why there are 4 arguments expected
   |                    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: 3 positional arguments in format string, but there are 3 arguments
  --> $DIR/ifmt-bad-arg.rs:84:15
   |
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
```

Author: estebank

src/libfmt_macros/lib.rs

@@ -59,16 +59,20 @@ pub struct Argument<'a> {
 /// Specification for the formatting of an argument in the format string.
 #[derive(Copy, Clone, PartialEq)]
 pub struct FormatSpec<'a> {
-    /// Optionally specified character to fill alignment with
+    /// Optionally specified character to fill alignment with.
     pub fill: Option<char>,
-    /// Optionally specified alignment
+    /// Optionally specified alignment.
     pub align: Alignment,
-    /// Packed version of various flags provided
+    /// Packed version of various flags provided.
     pub flags: u32,
-    /// The integer precision to use
+    /// The integer precision to use.
     pub precision: Count,
-    /// The string width requested for the resulting format
+    /// The span of the precision formatting flag (for diagnostics).
+    pub precision_span: Option<InnerSpan>,
+    /// The string width requested for the resulting format.
     pub width: Count,
+    /// The span of the width formatting flag (for diagnostics).
+    pub width_span: Option<InnerSpan>,
     /// The descriptor string representing the name of the format desired for
     /// this argument, this can be empty or any number of characters, although
     /// it is required to be one word.
@@ -285,19 +289,24 @@ impl<'a> Parser<'a> {
     }
 
     /// Optionally consumes the specified character. If the character is not at
-    /// the current position, then the current iterator isn't moved and false is
-    /// returned, otherwise the character is consumed and true is returned.
+    /// the current position, then the current iterator isn't moved and `false` is
+    /// returned, otherwise the character is consumed and `true` is returned.
     fn consume(&mut self, c: char) -> bool {
-        if let Some(&(_, maybe)) = self.cur.peek() {
+        self.consume_pos(c).is_some()
+    }
+
+    /// Optionally consumes the specified character. If the character is not at
+    /// the current position, then the current iterator isn't moved and `None` is
+    /// returned, otherwise the character is consumed and the current position is
+    /// returned.
+    fn consume_pos(&mut self, c: char) -> Option<usize> {
+        if let Some(&(pos, maybe)) = self.cur.peek() {
             if c == maybe {
                 self.cur.next();
-                true
-            } else {
-                false
+                return Some(pos);
             }
-        } else {
-            false
         }
+        None
     }
 
     fn to_span_index(&self, pos: usize) -> InnerOffset {
@@ -465,7 +474,9 @@ impl<'a> Parser<'a> {
             align: AlignUnknown,
             flags: 0,
             precision: CountImplied,
+            precision_span: None,
             width: CountImplied,
+            width_span: None,
             ty: &self.input[..0],
         };
         if !self.consume(':') {
@@ -502,6 +513,11 @@ impl<'a> Parser<'a> {
         }
         // Width and precision
         let mut havewidth = false;
+
+        let mut width_span_start = 0;
+        if let Some((pos, '0')) = self.cur.peek() {
+            width_span_start = *pos;
+        }
         if self.consume('0') {
             // small ambiguity with '0$' as a format string. In theory this is a
             // '0' flag and then an ill-formatted format string with just a '$'
@@ -515,17 +531,28 @@ impl<'a> Parser<'a> {
             }
         }
         if !havewidth {
-            spec.width = self.count();
+            if width_span_start == 0 {
+                if let Some((pos, _)) = self.cur.peek() {
+                    width_span_start = *pos;
+                }
+            }
+            let (w, sp) = self.count(width_span_start);
+            spec.width = w;
+            spec.width_span = sp;
         }
-        if self.consume('.') {
-            if self.consume('*') {
+        if let Some(start) = self.consume_pos('.') {
+            if let Some(end) = self.consume_pos('*') {
                 // Resolve `CountIsNextParam`.
                 // We can do this immediately as `position` is resolved later.
                 let i = self.curarg;
                 self.curarg += 1;
                 spec.precision = CountIsParam(i);
+                spec.precision_span =
+                    Some(self.to_span_index(start).to(self.to_span_index(end + 1)));
             } else {
-                spec.precision = self.count();
+                let (p, sp) = self.count(start);
+                spec.precision = p;
+                spec.precision_span = sp;
             }
         }
         // Optional radix followed by the actual format specifier
@@ -554,24 +581,25 @@ impl<'a> Parser<'a> {
     /// Parses a Count parameter at the current position. This does not check
     /// for 'CountIsNextParam' because that is only used in precision, not
     /// width.
-    fn count(&mut self) -> Count {
+    fn count(&mut self, start: usize) -> (Count, Option<InnerSpan>) {
         if let Some(i) = self.integer() {
-            if self.consume('$') {
-                CountIsParam(i)
+            if let Some(end) = self.consume_pos('$') {
+                let span = self.to_span_index(start).to(self.to_span_index(end + 1));
+                (CountIsParam(i), Some(span))
             } else {
-                CountIs(i)
+                (CountIs(i), None)
             }
         } else {
             let tmp = self.cur.clone();
             let word = self.word();
             if word.is_empty() {
                 self.cur = tmp;
-                CountImplied
+                (CountImplied, None)
             } else if self.consume('$') {
-                CountIsName(Symbol::intern(word))
+                (CountIsName(Symbol::intern(word)), None)
             } else {
                 self.cur = tmp;
-                CountImplied
+                (CountImplied, None)
             }
         }
     }

src/libsyntax_ext/format.rs

@@ -109,6 +109,8 @@ struct Context<'a, 'b> {
     invalid_refs: Vec<(usize, usize)>,
     /// Spans of all the formatting arguments, in order.
     arg_spans: Vec<Span>,
+    /// All the formatting arguments that have formatting flags set, in order for diagnostics.
+    arg_with_formatting: Vec<parse::FormatSpec<'a>>,
     /// Whether this formatting string is a literal or it comes from a macro.
     is_literal: bool,
 }
@@ -279,14 +281,20 @@ impl<'a, 'b> Context<'a, 'b> {
             .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();
             e = self.ecx.mut_span_err(
                 sp,
                 &format!(
                     "{} positional argument{} in format string, but {}",
-                         self.pieces.len(),
-                         if self.pieces.len() > 1 { "s" } else { "" },
-                    self.describe_num_args()
+                    count,
+                    if count > 1 { "s" } else { "" },
+                    self.describe_num_args(),
                 ),
             );
         } else {
@@ -317,9 +325,70 @@ impl<'a, 'b> Context<'a, 'b> {
                 &format!("invalid reference to positional {} ({})",
                          arg_list,
                          self.describe_num_args()));
-            e.note("positional arguments are zero-based");
+            zero_based_note = true;
         };
 
+        for fmt in &self.arg_with_formatting {
+            if let Some(span) = fmt.precision_span {
+                let span = self.fmtsp.from_inner(span);
+                match fmt.precision {
+                    parse::CountIsParam(pos) if pos > self.args.len() => {
+                        e.span_label(span, &format!(
+                            "this precision flag expects an `usize` argument at position {}, \
+                             but {}",
+                            pos,
+                            self.describe_num_args(),
+                        ));
+                        zero_based_note = true;
+                    }
+                    parse::CountIsParam(pos) => {
+                        let count = self.pieces.len() + self.arg_with_formatting
+                            .iter()
+                            .filter(|fmt| fmt.precision_span.is_some())
+                            .count();
+                        e.span_label(span, &format!(
+                            "this precision flag adds an extra required argument at position {}, \
+                             which is why there {} expected",
+                            pos,
+                            if count == 1 {
+                                "is 1 argument".to_string()
+                            } else {
+                                format!("are {} arguments", count)
+                            },
+                        ));
+                        e.span_label(
+                            self.args[pos].span,
+                            "this parameter corresponds to the precision flag",
+                        );
+                        zero_based_note = true;
+                    }
+                    _ => {}
+                }
+            }
+            if let Some(span) = fmt.width_span {
+                let span = self.fmtsp.from_inner(span);
+                match fmt.width {
+                    parse::CountIsParam(pos) if pos > self.args.len() => {
+                        e.span_label(span, &format!(
+                            "this width flag expects an `usize` argument at position {}, \
+                             but {}",
+                            pos,
+                            self.describe_num_args(),
+                        ));
+                        zero_based_note = true;
+                    }
+                    _ => {}
+                }
+            }
+        }
+        if zero_based_note {
+            e.note("positional arguments are zero-based");
+        }
+        if !self.arg_with_formatting.is_empty() {
+            e.note("for information about formatting flags, visit \
+                    https://doc.rust-lang.org/std/fmt/index.html");
+        }
+
         e.emit();
     }
 
@@ -435,10 +504,11 @@ impl<'a, 'b> Context<'a, 'b> {
 
     /// Builds a static `rt::Argument` from a `parse::Piece` or append
     /// to the `literal` string.
-    fn build_piece(&mut self,
-                   piece: &parse::Piece<'_>,
-                   arg_index_consumed: &mut Vec<usize>)
-                   -> Option<P<ast::Expr>> {
+    fn build_piece(
+        &mut self,
+        piece: &parse::Piece<'a>,
+        arg_index_consumed: &mut Vec<usize>,
+    ) -> Option<P<ast::Expr>> {
         let sp = self.macsp;
         match *piece {
             parse::String(s) => {
@@ -496,7 +566,9 @@ impl<'a, 'b> Context<'a, 'b> {
                         align: parse::AlignUnknown,
                         flags: 0,
                         precision: parse::CountImplied,
+                        precision_span: None,
                         width: parse::CountImplied,
+                        width_span: None,
                         ty: arg.format.ty,
                     },
                 };
@@ -506,6 +578,9 @@ impl<'a, 'b> Context<'a, 'b> {
                 let pos_simple =
                     arg.position.index() == simple_arg.position.index();
 
+                if arg.format.precision_span.is_some() || arg.format.width_span.is_some() {
+                    self.arg_with_formatting.push(arg.format); //'liself.fmtsp.from_inner(span));
+                }
                 if !pos_simple || arg.format != simple_arg.format || fill != ' ' {
                     self.all_pieces_simple = false;
                 }
@@ -530,7 +605,7 @@ impl<'a, 'b> Context<'a, 'b> {
                 let path = self.ecx.path_global(sp, Context::rtpath(self.ecx, "FormatSpec"));
                 let fmt = self.ecx.expr_struct(
                     sp,
-                                         path,
+                    path,
                     vec![
                         self.ecx.field_imm(sp, self.ecx.ident_of("fill"), fill),
                         self.ecx.field_imm(sp, self.ecx.ident_of("align"), align),
@@ -657,12 +732,13 @@ impl<'a, 'b> Context<'a, 'b> {
         self.ecx.expr_call_global(self.macsp, path, fn_args)
     }
 
-    fn format_arg(ecx: &ExtCtxt<'_>,
-                  macsp: Span,
-                  mut sp: Span,
-                  ty: &ArgumentType,
-                  arg: ast::Ident)
-                  -> P<ast::Expr> {
+    fn format_arg(
+        ecx: &ExtCtxt<'_>,
+        macsp: Span,
+        mut sp: Span,
+        ty: &ArgumentType,
+        arg: ast::Ident,
+    ) -> P<ast::Expr> {
         sp = sp.apply_mark(ecx.current_expansion.id);
         let arg = ecx.expr_ident(sp, arg);
         let trait_ = match *ty {
@@ -941,6 +1017,7 @@ pub fn expand_preparsed_format_args(
         fmtsp: fmt.span,
         invalid_refs: Vec::new(),
         arg_spans,
+        arg_with_formatting: Vec::new(),
         is_literal,
     };
 

src/test/ui/if/ifmt-bad-arg.rs

@@ -75,4 +75,12 @@ ninth number: {
 tenth number: {}",
         1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
     //~^^ ERROR: invalid format string
+    println!("{} {:.*} {}", 1, 3.2, 4);
+    //~^ ERROR 4 positional arguments in format string, but there are 3 arguments
+    //~| ERROR mismatched types
+    println!("{} {:07$.*} {}", 1, 3.2, 4);
+    //~^ 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
 }

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

@@ -220,5 +220,58 @@ LL | tenth number: {}",
    |
    = note: if you intended to print `{`, you can escape it using `{{`
 
-error: aborting due to 28 previous errors
+error: 4 positional arguments in format string, but there are 3 arguments
+  --> $DIR/ifmt-bad-arg.rs:78:15
+   |
+LL |     println!("{} {:.*} {}", 1, 3.2, 4);
+   |               ^^ ^^--^ ^^      --- this parameter corresponds to the precision flag
+   |                    |
+   |                    this precision flag adds an extra required argument at position 1, which is why there are 4 arguments expected
+   |
+   = note: positional arguments are zero-based
+   = note: for information about formatting flags, visit https://doc.rust-lang.org/std/fmt/index.html
+
+error: 4 positional arguments in format string, but there are 3 arguments
+  --> $DIR/ifmt-bad-arg.rs:81:15
+   |
+LL |     println!("{} {:07$.*} {}", 1, 3.2, 4);
+   |               ^^ ^^-----^ ^^      --- this parameter corresponds to the precision flag
+   |                    |  |
+   |                    |  this precision flag adds an extra required argument at position 1, which is why there are 4 arguments expected
+   |                    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: 3 positional arguments in format string, but there are 3 arguments
+  --> $DIR/ifmt-bad-arg.rs:84:15
+   |
+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[E0308]: mismatched types
+  --> $DIR/ifmt-bad-arg.rs:78:32
+   |
+LL |     println!("{} {:.*} {}", 1, 3.2, 4);
+   |                                ^^^ expected usize, found floating-point number
+   |
+   = note: expected type `&usize`
+              found type `&{float}`
+
+error[E0308]: mismatched types
+  --> $DIR/ifmt-bad-arg.rs:81:35
+   |
+LL |     println!("{} {:07$.*} {}", 1, 3.2, 4);
+   |                                   ^^^ expected usize, found floating-point number
+   |
+   = note: expected type `&usize`
+              found type `&{float}`
+
+error: aborting due to 33 previous errors
 
+For more information about this error, try `rustc --explain E0308`.