Auto merge of #57069 - estebank:str-err, r=@cramertj

Various changes to string format diagnostics

- Point at opening mismatched formatting brace
- Account for differences between raw and regular strings
- Account for differences between the code snippet and `InternedString`
- Add more tests

```
error: invalid format string: expected `'}'`, found `'t'`
  --> $DIR/ifmt-bad-arg.rs:85:1
   |
LL | ninth number: {
   |               - because of this opening brace
LL | tenth number: {}",
   | ^ expected `}` in format string
   |
   = note: if you intended to print `{`, you can escape it using `{{`
```

Fix #53837.

Author: bors

src/libfmt_macros/lib.rs

@@ -123,8 +123,9 @@ pub struct ParseError {
     pub description: string::String,
     pub note: Option<string::String>,
     pub label: string::String,
-    pub start: usize,
-    pub end: usize,
+    pub start: SpanIndex,
+    pub end: SpanIndex,
+    pub secondary_label: Option<(string::String, SpanIndex, SpanIndex)>,
 }
 
 /// The parser structure for interpreting the input format string. This is
@@ -142,22 +143,39 @@ pub struct Parser<'a> {
     curarg: usize,
     /// `Some(raw count)` when the string is "raw", used to position spans correctly
     style: Option<usize>,
-    /// How many newlines have been seen in the string so far, to adjust the error spans
-    seen_newlines: usize,
     /// Start and end byte offset of every successfully parsed argument
     pub arg_places: Vec<(usize, usize)>,
+    /// Characters that need to be shifted
+    skips: Vec<usize>,
+    /// Span offset of the last opening brace seen, used for error reporting
+    last_opening_brace_pos: Option<SpanIndex>,
+    /// Wether the source string is comes from `println!` as opposed to `format!` or `print!`
+    append_newline: bool,
+}
+
+#[derive(Clone, Copy, Debug)]
+pub struct SpanIndex(usize);
+
+impl SpanIndex {
+    pub fn unwrap(self) -> usize {
+        self.0
+    }
 }
 
 impl<'a> Iterator for Parser<'a> {
     type Item = Piece<'a>;
 
     fn next(&mut self) -> Option<Piece<'a>> {
-        let raw = self.style.map(|raw| raw + self.seen_newlines).unwrap_or(0);
+        let raw = self.raw();
         if let Some(&(pos, c)) = self.cur.peek() {
             match c {
                 '{' => {
+                    let curr_last_brace = self.last_opening_brace_pos;
+                    self.last_opening_brace_pos = Some(self.to_span_index(pos));
                     self.cur.next();
                     if self.consume('{') {
+                        self.last_opening_brace_pos = curr_last_brace;
+
                         Some(String(self.string(pos + 1)))
                     } else {
                         let arg = self.argument();
@@ -174,7 +192,7 @@ impl<'a> Iterator for Parser<'a> {
                     if self.consume('}') {
                         Some(String(self.string(pos + 1)))
                     } else {
-                        let err_pos = pos + raw + 1;
+                        let err_pos = self.to_span_index(pos);
                         self.err_with_note(
                             "unmatched `}` found",
                             "unmatched `}`",
@@ -186,7 +204,6 @@ impl<'a> Iterator for Parser<'a> {
                     }
                 }
                 '\n' => {
-                    self.seen_newlines += 1;
                     Some(String(self.string(pos)))
                 }
                 _ => Some(String(self.string(pos))),
@@ -199,15 +216,22 @@ impl<'a> Iterator for Parser<'a> {
 
 impl<'a> Parser<'a> {
     /// Creates a new parser for the given format string
-    pub fn new(s: &'a str, style: Option<usize>) -> Parser<'a> {
+    pub fn new(
+        s: &'a str,
+        style: Option<usize>,
+        skips: Vec<usize>,
+        append_newline: bool,
+    ) -> Parser<'a> {
         Parser {
             input: s,
             cur: s.char_indices().peekable(),
             errors: vec![],
             curarg: 0,
             style,
-            seen_newlines: 0,
             arg_places: vec![],
+            skips,
+            last_opening_brace_pos: None,
+            append_newline,
         }
     }
 
@@ -218,15 +242,16 @@ impl<'a> Parser<'a> {
         &mut self,
         description: S1,
         label: S2,
-        start: usize,
-        end: usize,
+        start: SpanIndex,
+        end: SpanIndex,
     ) {
         self.errors.push(ParseError {
             description: description.into(),
             note: None,
             label: label.into(),
             start,
             end,
+            secondary_label: None,
         });
     }
 
@@ -238,15 +263,16 @@ impl<'a> Parser<'a> {
         description: S1,
         label: S2,
         note: S3,
-        start: usize,
-        end: usize,
+        start: SpanIndex,
+        end: SpanIndex,
     ) {
         self.errors.push(ParseError {
             description: description.into(),
             note: Some(note.into()),
             label: label.into(),
             start,
             end,
+            secondary_label: None,
         });
     }
 
@@ -266,47 +292,86 @@ impl<'a> Parser<'a> {
         }
     }
 
+    fn raw(&self) -> usize {
+        self.style.map(|raw| raw + 1).unwrap_or(0)
+    }
+
+    fn to_span_index(&self, pos: usize) -> SpanIndex {
+        let mut pos = pos;
+        for skip in &self.skips {
+            if pos > *skip {
+                pos += 1;
+            } else if pos == *skip && self.raw() == 0 {
+                pos += 1;
+            } else {
+                break;
+            }
+        }
+        SpanIndex(self.raw() + pos + 1)
+    }
+
     /// Forces consumption of the specified character. If the character is not
     /// found, an error is emitted.
     fn must_consume(&mut self, c: char) -> Option<usize> {
         self.ws();
-        let raw = self.style.unwrap_or(0);
 
-        let padding = raw + self.seen_newlines;
         if let Some(&(pos, maybe)) = self.cur.peek() {
             if c == maybe {
                 self.cur.next();
                 Some(pos)
             } else {
-                let pos = pos + raw + 1;
-                self.err(format!("expected `{:?}`, found `{:?}`", c, maybe),
-                         format!("expected `{}`", c),
-                         pos,
-                         pos);
+                let pos = self.to_span_index(pos);
+                let description = format!("expected `'}}'`, found `{:?}`", maybe);
+                let label = "expected `}`".to_owned();
+                let (note, secondary_label) = if c == '}' {
+                    (Some("if you intended to print `{`, you can escape it using `{{`".to_owned()),
+                     self.last_opening_brace_pos.map(|pos| {
+                        ("because of this opening brace".to_owned(), pos, pos)
+                     }))
+                } else {
+                    (None, None)
+                };
+                self.errors.push(ParseError {
+                    description,
+                    note,
+                    label,
+                    start: pos,
+                    end: pos,
+                    secondary_label,
+                });
                 None
             }
         } else {
-            let msg = format!("expected `{:?}` but string was terminated", c);
-            // point at closing `"`, unless the last char is `\n` to account for `println`
-            let pos = match self.input.chars().last() {
-                Some('\n') => self.input.len(),
-                _ => self.input.len() + 1,
-            };
+            let description = format!("expected `{:?}` but string was terminated", c);
+            // point at closing `"`
+            let pos = self.input.len() - if self.append_newline { 1 } else { 0 };
+            let pos = self.to_span_index(pos);
             if c == '}' {
-                self.err_with_note(msg,
-                                   format!("expected `{:?}`", c),
-                                   "if you intended to print `{`, you can escape it using `{{`",
-                                   pos + padding,
-                                   pos + padding);
+                let label = format!("expected `{:?}`", c);
+                let (note, secondary_label) = if c == '}' {
+                    (Some("if you intended to print `{`, you can escape it using `{{`".to_owned()),
+                     self.last_opening_brace_pos.map(|pos| {
+                        ("because of this opening brace".to_owned(), pos, pos)
+                     }))
+                } else {
+                    (None, None)
+                };
+                self.errors.push(ParseError {
+                    description,
+                    note,
+                    label,
+                    start: pos,
+                    end: pos,
+                    secondary_label,
+                });
             } else {
-                self.err(msg, format!("expected `{:?}`", c), pos, pos);
+                self.err(description, format!("expected `{:?}`", c), pos, pos);
             }
             None
         }
     }
 
-    /// Consumes all whitespace characters until the first non-whitespace
-    /// character
+    /// Consumes all whitespace characters until the first non-whitespace character
     fn ws(&mut self) {
         while let Some(&(_, c)) = self.cur.peek() {
             if c.is_whitespace() {
@@ -334,8 +399,7 @@ impl<'a> Parser<'a> {
         &self.input[start..self.input.len()]
     }
 
-    /// Parses an Argument structure, or what's contained within braces inside
-    /// the format string
+    /// Parses an Argument structure, or what's contained within braces inside the format string
     fn argument(&mut self) -> Argument<'a> {
         let pos = self.position();
         let format = self.format();
@@ -371,8 +435,8 @@ impl<'a> Parser<'a> {
                     self.err_with_note(format!("invalid argument name `{}`", invalid_name),
                                        "invalid argument name",
                                        "argument names cannot start with an underscore",
-                                       pos + 1, // add 1 to account for leading `{`
-                                       pos + 1 + invalid_name.len());
+                                       self.to_span_index(pos),
+                                       self.to_span_index(pos + invalid_name.len()));
                     Some(ArgumentNamed(invalid_name))
                 },
 
@@ -553,7 +617,7 @@ mod tests {
     use super::*;
 
     fn same(fmt: &'static str, p: &[Piece<'static>]) {
-        let parser = Parser::new(fmt, None);
+        let parser = Parser::new(fmt, None, vec![], false);
         assert!(parser.collect::<Vec<Piece<'static>>>() == p);
     }
 
@@ -569,7 +633,7 @@ mod tests {
     }
 
     fn musterr(s: &str) {
-        let mut p = Parser::new(s, None);
+        let mut p = Parser::new(s, None, vec![], false);
         p.next();
         assert!(!p.errors.is_empty());
     }

src/librustc/traits/on_unimplemented.rs

@@ -234,7 +234,7 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedFormatString {
     {
         let name = tcx.item_name(trait_def_id);
         let generics = tcx.generics_of(trait_def_id);
-        let parser = Parser::new(&self.0, None);
+        let parser = Parser::new(&self.0, None, vec![], false);
         let mut result = Ok(());
         for token in parser {
             match token {
@@ -293,7 +293,7 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedFormatString {
         }).collect::<FxHashMap<String, String>>();
         let empty_string = String::new();
 
-        let parser = Parser::new(&self.0, None);
+        let parser = Parser::new(&self.0, None, vec![], false);
         parser.map(|p|
             match p {
                 Piece::String(s) => s,