Rollup merge of #63121 - estebank:formatting-pos, r=alexcrichton

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: 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
```

Fix #49384.

2 files changed, 235 insertions, 170 deletions

diff --git a/src/libfmt_macros/lib.rs b/src/libfmt_macros/lib.rs
index 83e24a48ea0..985abaf2c1b 100644
--- a/src/libfmt_macros/lib.rs
+++ b/src/libfmt_macros/lib.rs
@@ -56,16 +56,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.
@@ -282,19 +286,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 {
@@ -462,7 +471,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(':') {
@@ -499,6 +510,7 @@ impl<'a> Parser<'a> {
         }
         // Width and precision
         let mut havewidth = false;
+
         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 '$'
@@ -512,17 +524,28 @@ impl<'a> Parser<'a> {
             }
         }
         if !havewidth {
-            spec.width = self.count();
+            let width_span_start = if let Some((pos, _)) = self.cur.peek() {
+                *pos
+            } else {
+                0
+            };
+            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
@@ -551,24 +574,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)
             }
         }
     }
diff --git a/src/libfmt_macros/tests.rs b/src/libfmt_macros/tests.rs
index 7282d4a5f24..e2ddb8810e9 100644
--- a/src/libfmt_macros/tests.rs
+++ b/src/libfmt_macros/tests.rs
@@ -12,6 +12,8 @@ fn fmtdflt() -> FormatSpec<'static> {
         flags: 0,
         precision: CountImplied,
         width: CountImplied,
+        precision_span: None,
+        width_span: None,
         ty: "",
     };
 }
@@ -79,165 +81,204 @@ fn format_position_nothing_else() {
 }
 #[test]
 fn format_type() {
-    same("{3:a}",
-         &[NextArgument(Argument {
-               position: ArgumentIs(3),
-               format: FormatSpec {
-                   fill: None,
-                   align: AlignUnknown,
-                   flags: 0,
-                   precision: CountImplied,
-                   width: CountImplied,
-                   ty: "a",
-               },
-           })]);
+    same(
+        "{3:a}",
+        &[NextArgument(Argument {
+            position: ArgumentIs(3),
+            format: FormatSpec {
+                fill: None,
+                align: AlignUnknown,
+                flags: 0,
+                precision: CountImplied,
+                width: CountImplied,
+                precision_span: None,
+                width_span: None,
+                ty: "a",
+            },
+        })]);
 }
 #[test]
 fn format_align_fill() {
-    same("{3:>}",
-         &[NextArgument(Argument {
-               position: ArgumentIs(3),
-               format: FormatSpec {
-                   fill: None,
-                   align: AlignRight,
-                   flags: 0,
-                   precision: CountImplied,
-                   width: CountImplied,
-                   ty: "",
-               },
-           })]);
-    same("{3:0<}",
-         &[NextArgument(Argument {
-               position: ArgumentIs(3),
-               format: FormatSpec {
-                   fill: Some('0'),
-                   align: AlignLeft,
-                   flags: 0,
-                   precision: CountImplied,
-                   width: CountImplied,
-                   ty: "",
-               },
-           })]);
-    same("{3:*<abcd}",
-         &[NextArgument(Argument {
-               position: ArgumentIs(3),
-               format: FormatSpec {
-                   fill: Some('*'),
-                   align: AlignLeft,
-                   flags: 0,
-                   precision: CountImplied,
-                   width: CountImplied,
-                   ty: "abcd",
-               },
-           })]);
+    same(
+        "{3:>}",
+        &[NextArgument(Argument {
+            position: ArgumentIs(3),
+            format: FormatSpec {
+                fill: None,
+                align: AlignRight,
+                flags: 0,
+                precision: CountImplied,
+                width: CountImplied,
+                precision_span: None,
+                width_span: None,
+                ty: "",
+            },
+        })]);
+    same(
+        "{3:0<}",
+        &[NextArgument(Argument {
+            position: ArgumentIs(3),
+            format: FormatSpec {
+                fill: Some('0'),
+                align: AlignLeft,
+                flags: 0,
+                precision: CountImplied,
+                width: CountImplied,
+                precision_span: None,
+                width_span: None,
+                ty: "",
+            },
+        })]);
+    same(
+        "{3:*<abcd}",
+        &[NextArgument(Argument {
+            position: ArgumentIs(3),
+            format: FormatSpec {
+                fill: Some('*'),
+                align: AlignLeft,
+                flags: 0,
+                precision: CountImplied,
+                width: CountImplied,
+                precision_span: None,
+                width_span: None,
+                ty: "abcd",
+            },
+        })]);
 }
 #[test]
 fn format_counts() {
     use syntax_pos::{GLOBALS, Globals, edition};
     GLOBALS.set(&Globals::new(edition::DEFAULT_EDITION), || {
-    same("{:10s}",
-         &[NextArgument(Argument {
-               position: ArgumentImplicitlyIs(0),
-               format: FormatSpec {
-                   fill: None,
-                   align: AlignUnknown,
-                   flags: 0,
-                   precision: CountImplied,
-                   width: CountIs(10),
-                   ty: "s",
-               },
-           })]);
-    same("{:10$.10s}",
-         &[NextArgument(Argument {
-               position: ArgumentImplicitlyIs(0),
-               format: FormatSpec {
-                   fill: None,
-                   align: AlignUnknown,
-                   flags: 0,
-                   precision: CountIs(10),
-                   width: CountIsParam(10),
-                   ty: "s",
-               },
-           })]);
-    same("{:.*s}",
-         &[NextArgument(Argument {
-               position: ArgumentImplicitlyIs(1),
-               format: FormatSpec {
-                   fill: None,
-                   align: AlignUnknown,
-                   flags: 0,
-                   precision: CountIsParam(0),
-                   width: CountImplied,
-                   ty: "s",
-               },
-           })]);
-    same("{:.10$s}",
-         &[NextArgument(Argument {
-               position: ArgumentImplicitlyIs(0),
-               format: FormatSpec {
-                   fill: None,
-                   align: AlignUnknown,
-                   flags: 0,
-                   precision: CountIsParam(10),
-                   width: CountImplied,
-                   ty: "s",
-               },
-           })]);
-    same("{:a$.b$s}",
-         &[NextArgument(Argument {
-               position: ArgumentImplicitlyIs(0),
-               format: FormatSpec {
-                   fill: None,
-                   align: AlignUnknown,
-                   flags: 0,
-                   precision: CountIsName(Symbol::intern("b")),
-                   width: CountIsName(Symbol::intern("a")),
-                   ty: "s",
-               },
-           })]);
+    same(
+        "{:10s}",
+        &[NextArgument(Argument {
+            position: ArgumentImplicitlyIs(0),
+            format: FormatSpec {
+                fill: None,
+                align: AlignUnknown,
+                flags: 0,
+                precision: CountImplied,
+                width: CountIs(10),
+                precision_span: None,
+                width_span: None,
+                ty: "s",
+            },
+        })]);
+    same(
+        "{:10$.10s}",
+        &[NextArgument(Argument {
+            position: ArgumentImplicitlyIs(0),
+            format: FormatSpec {
+                fill: None,
+                align: AlignUnknown,
+                flags: 0,
+                precision: CountIs(10),
+                width: CountIsParam(10),
+                precision_span: None,
+                width_span: Some(InnerSpan::new(3, 6)),
+                ty: "s",
+            },
+        })]);
+    same(
+        "{:.*s}",
+        &[NextArgument(Argument {
+            position: ArgumentImplicitlyIs(1),
+            format: FormatSpec {
+                fill: None,
+                align: AlignUnknown,
+                flags: 0,
+                precision: CountIsParam(0),
+                width: CountImplied,
+                precision_span: Some(InnerSpan::new(3, 5)),
+                width_span: None,
+                ty: "s",
+            },
+        })]);
+    same(
+        "{:.10$s}",
+        &[NextArgument(Argument {
+            position: ArgumentImplicitlyIs(0),
+            format: FormatSpec {
+                fill: None,
+                align: AlignUnknown,
+                flags: 0,
+                precision: CountIsParam(10),
+                width: CountImplied,
+                precision_span: Some(InnerSpan::new(3, 7)),
+                width_span: None,
+                ty: "s",
+            },
+        })]);
+    same(
+        "{:a$.b$s}",
+        &[NextArgument(Argument {
+            position: ArgumentImplicitlyIs(0),
+            format: FormatSpec {
+                fill: None,
+                align: AlignUnknown,
+                flags: 0,
+                precision: CountIsName(Symbol::intern("b")),
+                width: CountIsName(Symbol::intern("a")),
+                precision_span: None,
+                width_span: None,
+                ty: "s",
+            },
+        })]);
     });
 }
 #[test]
 fn format_flags() {
-    same("{:-}",
-         &[NextArgument(Argument {
-               position: ArgumentImplicitlyIs(0),
-               format: FormatSpec {
-                   fill: None,
-                   align: AlignUnknown,
-                   flags: (1 << FlagSignMinus as u32),
-                   precision: CountImplied,
-                   width: CountImplied,
-                   ty: "",
-               },
-           })]);
-    same("{:+#}",
-         &[NextArgument(Argument {
-               position: ArgumentImplicitlyIs(0),
-               format: FormatSpec {
-                   fill: None,
-                   align: AlignUnknown,
-                   flags: (1 << FlagSignPlus as u32) | (1 << FlagAlternate as u32),
-                   precision: CountImplied,
-                   width: CountImplied,
-                   ty: "",
-               },
-           })]);
+    same(
+        "{:-}",
+        &[NextArgument(Argument {
+            position: ArgumentImplicitlyIs(0),
+            format: FormatSpec {
+                fill: None,
+                align: AlignUnknown,
+                flags: (1 << FlagSignMinus as u32),
+                precision: CountImplied,
+                width: CountImplied,
+                precision_span: None,
+                width_span: None,
+                ty: "",
+            },
+        })]);
+    same(
+        "{:+#}",
+        &[NextArgument(Argument {
+            position: ArgumentImplicitlyIs(0),
+            format: FormatSpec {
+                fill: None,
+                align: AlignUnknown,
+                flags: (1 << FlagSignPlus as u32) | (1 << FlagAlternate as u32),
+                precision: CountImplied,
+                width: CountImplied,
+                precision_span: None,
+                width_span: None,
+                ty: "",
+            },
+        })]);
 }
 #[test]
 fn format_mixture() {
-    same("abcd {3:a} efg",
-         &[String("abcd "),
-           NextArgument(Argument {
-               position: ArgumentIs(3),
-               format: FormatSpec {
-                   fill: None,
-                   align: AlignUnknown,
-                   flags: 0,
-                   precision: CountImplied,
-                   width: CountImplied,
-                   ty: "a",
-               },
-           }),
-           String(" efg")]);
+    same(
+        "abcd {3:a} efg",
+        &[
+            String("abcd "),
+            NextArgument(Argument {
+                position: ArgumentIs(3),
+                format: FormatSpec {
+                    fill: None,
+                    align: AlignUnknown,
+                    flags: 0,
+                    precision: CountImplied,
+                    width: CountImplied,
+                    precision_span: None,
+                    width_span: None,
+                    ty: "a",
+                },
+            }),
+            String(" efg"),
+        ],
+    );
 }