Rollup merge of #52649 - estebank:fmt-span, r=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
   |               ^^^^^^ ^^
```

8 files changed, 494 insertions, 116 deletions

diff --git a/src/libfmt_macros/lib.rs b/src/libfmt_macros/lib.rs
index 9952e5f64d6..30a3bbdc58e 100644
--- a/src/libfmt_macros/lib.rs
+++ b/src/libfmt_macros/lib.rs
@@ -154,6 +154,8 @@ 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,
+    /// Start and end byte offset of every successfuly parsed argument
+    pub arg_places: Vec<(usize, usize)>,
 }
 
 impl<'a> Iterator for Parser<'a> {
@@ -168,9 +170,13 @@ impl<'a> Iterator for Parser<'a> {
                     if self.consume('{') {
                         Some(String(self.string(pos + 1)))
                     } else {
-                        let ret = Some(NextArgument(self.argument()));
-                        self.must_consume('}');
-                        ret
+                        let mut arg = self.argument();
+                        if let Some(arg_pos) = self.must_consume('}').map(|end| {
+                            (pos + raw + 1, end + raw + 2)
+                        }) {
+                            self.arg_places.push(arg_pos);
+                        }
+                        Some(NextArgument(arg))
                     }
                 }
                 '}' => {
@@ -211,6 +217,7 @@ impl<'a> Parser<'a> {
             curarg: 0,
             style,
             seen_newlines: 0,
+            arg_places: vec![],
         }
     }
 
@@ -271,7 +278,7 @@ impl<'a> Parser<'a> {
 
     /// Forces consumption of the specified character. If the character is not
     /// found, an error is emitted.
-    fn must_consume(&mut self, c: char) {
+    fn must_consume(&mut self, c: char) -> Option<usize> {
         self.ws();
         let raw = self.style.unwrap_or(0);
 
@@ -279,12 +286,14 @@ impl<'a> Parser<'a> {
         if let Some(&(pos, maybe)) = self.cur.peek() {
             if c == maybe {
                 self.cur.next();
+                Some(pos)
             } else {
                 let pos = pos + padding + 1;
                 self.err(format!("expected `{:?}`, found `{:?}`", c, maybe),
                          format!("expected `{}`", c),
                          pos,
                          pos);
+                None
             }
         } else {
             let msg = format!("expected `{:?}` but string was terminated", c);
@@ -302,6 +311,7 @@ impl<'a> Parser<'a> {
             } else {
                 self.err(msg, format!("expected `{:?}`", c), pos, pos);
             }
+            None
         }
     }
 
diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs
index 755d2b476b7..98de3d80b1e 100644
--- a/src/libsyntax_ext/format.rs
+++ b/src/libsyntax_ext/format.rs
@@ -14,18 +14,18 @@ use self::Position::*;
 use fmt_macros as parse;
 
 use syntax::ast;
-use syntax::ext::base::*;
 use syntax::ext::base;
+use syntax::ext::base::*;
 use syntax::ext::build::AstBuilder;
 use syntax::feature_gate;
 use syntax::parse::token;
 use syntax::ptr::P;
 use syntax::symbol::Symbol;
-use syntax_pos::{Span, DUMMY_SP};
 use syntax::tokenstream;
+use syntax_pos::{MultiSpan, Span, DUMMY_SP};
 
-use std::collections::{HashMap, HashSet};
 use std::collections::hash_map::Entry;
+use std::collections::{HashMap, HashSet};
 
 #[derive(PartialEq)]
 enum ArgumentType {
@@ -111,8 +111,14 @@ struct Context<'a, 'b: 'a> {
     /// still existed in this phase of processing.
     /// Used only for `all_pieces_simple` tracking in `build_piece`.
     curarg: usize,
-    /// Keep track of invalid references to positional arguments
-    invalid_refs: Vec<usize>,
+    /// Current piece being evaluated, used for error reporting.
+    curpiece: usize,
+    /// Keep track of invalid references to positional arguments.
+    invalid_refs: Vec<(usize, usize)>,
+    /// Spans of all the formatting arguments, in order.
+    arg_spans: Vec<Span>,
+    /// Wether this formatting string is a literal or it comes from a macro.
+    is_literal: bool,
 }
 
 /// Parses the arguments from the given list of tokens, returning None
@@ -155,15 +161,20 @@ fn parse_args(ecx: &mut ExtCtxt,
                     i
                 }
                 _ if named => {
-                    ecx.span_err(p.span,
-                                 "expected ident, positional arguments \
-                                 cannot follow named arguments");
+                    ecx.span_err(
+                        p.span,
+                        "expected ident, positional arguments cannot follow named arguments",
+                    );
                     return None;
                 }
                 _ => {
-                    ecx.span_err(p.span,
-                                 &format!("expected ident for named argument, found `{}`",
-                                          p.this_token_to_string()));
+                    ecx.span_err(
+                        p.span,
+                        &format!(
+                            "expected ident for named argument, found `{}`",
+                            p.this_token_to_string()
+                        ),
+                    );
                     return None;
                 }
             };
@@ -235,6 +246,7 @@ impl<'a, 'b> Context<'a, 'b> {
 
                 let ty = Placeholder(arg.format.ty.to_string());
                 self.verify_arg_type(pos, ty);
+                self.curpiece += 1;
             }
         }
     }
@@ -266,29 +278,59 @@ impl<'a, 'b> Context<'a, 'b> {
     /// format string.
     fn report_invalid_references(&self, numbered_position_args: bool) {
         let mut e;
-        let mut refs: Vec<String> = self.invalid_refs
-                                        .iter()
-                                        .map(|r| r.to_string())
-                                        .collect();
+        let sp = if self.is_literal {
+            MultiSpan::from_spans(self.arg_spans.clone())
+        } else {
+            MultiSpan::from_span(self.fmtsp)
+        };
+        let mut refs: Vec<_> = self
+            .invalid_refs
+            .iter()
+            .map(|(r, pos)| (r.to_string(), self.arg_spans.get(*pos)))
+            .collect();
 
         if self.names.is_empty() && !numbered_position_args {
-            e = self.ecx.mut_span_err(self.fmtsp,
-                &format!("{} positional argument{} in format string, but {}",
+            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()));
+                    self.describe_num_args()
+                ),
+            );
         } else {
-            let arg_list = match refs.len() {
-                1 => format!("argument {}", refs.pop().unwrap()),
-                _ => format!("arguments {head} and {tail}",
-                             tail=refs.pop().unwrap(),
-                             head=refs.join(", "))
+            let (arg_list, mut sp) = match refs.len() {
+                1 => {
+                    let (reg, pos) = refs.pop().unwrap();
+                    (
+                        format!("argument {}", reg),
+                        MultiSpan::from_span(*pos.unwrap_or(&self.fmtsp)),
+                    )
+                }
+                _ => {
+                    let pos =
+                        MultiSpan::from_spans(refs.iter().map(|(_, p)| *p.unwrap()).collect());
+                    let mut refs: Vec<String> = refs.iter().map(|(s, _)| s.to_owned()).collect();
+                    let reg = refs.pop().unwrap();
+                    (
+                        format!(
+                            "arguments {head} and {tail}",
+                            tail = reg,
+                            head = refs.join(", ")
+                        ),
+                        pos,
+                    )
+                }
             };
+            if !self.is_literal {
+                sp = MultiSpan::from_span(self.fmtsp);
+            }
 
-            e = self.ecx.mut_span_err(self.fmtsp,
+            e = self.ecx.mut_span_err(sp,
                 &format!("invalid reference to positional {} ({})",
-                        arg_list,
-                        self.describe_num_args()));
+                         arg_list,
+                         self.describe_num_args()));
             e.note("positional arguments are zero-based");
         };
 
@@ -301,7 +343,7 @@ impl<'a, 'b> Context<'a, 'b> {
         match arg {
             Exact(arg) => {
                 if self.args.len() <= arg {
-                    self.invalid_refs.push(arg);
+                    self.invalid_refs.push((arg, self.curpiece));
                     return;
                 }
                 match ty {
@@ -337,7 +379,13 @@ impl<'a, 'b> Context<'a, 'b> {
                     Some(e) => *e,
                     None => {
                         let msg = format!("there is no argument named `{}`", name);
-                        self.ecx.span_err(self.fmtsp, &msg[..]);
+                        let sp = if self.is_literal {
+                            *self.arg_spans.get(self.curpiece).unwrap_or(&self.fmtsp)
+                        } else {
+                            self.fmtsp
+                        };
+                        let mut err = self.ecx.struct_span_err(sp, &msg[..]);
+                        err.emit();
                         return;
                     }
                 };
@@ -505,33 +553,27 @@ impl<'a, 'b> Context<'a, 'b> {
                 let prec = self.build_count(arg.format.precision);
                 let width = self.build_count(arg.format.width);
                 let path = self.ecx.path_global(sp, Context::rtpath(self.ecx, "FormatSpec"));
-                let fmt =
-                    self.ecx.expr_struct(sp,
+                let fmt = self.ecx.expr_struct(
+                    sp,
                                          path,
-                                         vec![self.ecx
-                                                  .field_imm(sp, self.ecx.ident_of("fill"), fill),
-                                              self.ecx.field_imm(sp,
-                                                                 self.ecx.ident_of("align"),
-                                                                 align),
-                                              self.ecx.field_imm(sp,
-                                                                 self.ecx.ident_of("flags"),
-                                                                 flags),
-                                              self.ecx.field_imm(sp,
-                                                                 self.ecx.ident_of("precision"),
-                                                                 prec),
-                                              self.ecx.field_imm(sp,
-                                                                 self.ecx.ident_of("width"),
-                                                                 width)]);
+                    vec![
+                        self.ecx.field_imm(sp, self.ecx.ident_of("fill"), fill),
+                        self.ecx.field_imm(sp, self.ecx.ident_of("align"), align),
+                        self.ecx.field_imm(sp, self.ecx.ident_of("flags"), flags),
+                        self.ecx.field_imm(sp, self.ecx.ident_of("precision"), prec),
+                        self.ecx.field_imm(sp, self.ecx.ident_of("width"), width),
+                    ],
+                );
 
                 let path = self.ecx.path_global(sp, Context::rtpath(self.ecx, "Argument"));
-                Some(self.ecx.expr_struct(sp,
+                Some(self.ecx.expr_struct(
+                    sp,
                                           path,
-                                          vec![self.ecx.field_imm(sp,
-                                                                  self.ecx.ident_of("position"),
-                                                                  pos),
-                                               self.ecx.field_imm(sp,
-                                                                  self.ecx.ident_of("format"),
-                                                                  fmt)]))
+                    vec![
+                        self.ecx.field_imm(sp, self.ecx.ident_of("position"), pos),
+                        self.ecx.field_imm(sp, self.ecx.ident_of("format"), fmt),
+                    ],
+                ))
             }
         }
     }
@@ -544,9 +586,9 @@ impl<'a, 'b> Context<'a, 'b> {
         let mut pats = Vec::new();
         let mut heads = Vec::new();
 
-        let names_pos: Vec<_> = (0..self.args.len()).map(|i| {
-            self.ecx.ident_of(&format!("arg{}", i)).gensym()
-        }).collect();
+        let names_pos: Vec<_> = (0..self.args.len())
+            .map(|i| self.ecx.ident_of(&format!("arg{}", i)).gensym())
+            .collect();
 
         // First, build up the static array which will become our precompiled
         // format "string"
@@ -690,10 +732,11 @@ pub fn expand_format_args<'cx>(ecx: &'cx mut ExtCtxt,
     }
 }
 
-pub fn expand_format_args_nl<'cx>(ecx: &'cx mut ExtCtxt,
-                                  mut sp: Span,
-                                  tts: &[tokenstream::TokenTree])
-                                  -> Box<dyn base::MacResult + 'cx> {
+pub fn expand_format_args_nl<'cx>(
+    ecx: &'cx mut ExtCtxt,
+    mut sp: Span,
+    tts: &[tokenstream::TokenTree],
+) -> Box<dyn base::MacResult + 'cx> {
     //if !ecx.ecfg.enable_allow_internal_unstable() {
 
     // For some reason, the only one that actually works for `println` is the first check
@@ -744,7 +787,6 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
             let sugg_fmt = match args.len() {
                 0 => "{}".to_string(),
                 _ => format!("{}{{}}", "{} ".repeat(args.len())),
-
             };
             err.span_suggestion(
                 fmt_sp.shrink_to_lo(),
@@ -753,7 +795,11 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
             );
             err.emit();
             return DummyResult::raw_expr(sp);
-        },
+        }
+    };
+    let is_literal = match ecx.codemap().span_to_snippet(fmt_sp) {
+        Ok(ref s) if s.starts_with("\"") || s.starts_with("r#") => true,
+        _ => false,
     };
 
     let mut cx = Context {
@@ -763,6 +809,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
         arg_unique_types,
         names,
         curarg: 0,
+        curpiece: 0,
         arg_index_map: Vec::new(),
         count_args: Vec::new(),
         count_positions: HashMap::new(),
@@ -775,6 +822,8 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
         macsp,
         fmtsp: fmt.span,
         invalid_refs: Vec::new(),
+        arg_spans: Vec::new(),
+        is_literal,
     };
 
     let fmt_str = &*fmt.node.0.as_str();
@@ -783,12 +832,22 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
         ast::StrStyle::Raw(raw) => Some(raw as usize),
     };
     let mut parser = parse::Parser::new(fmt_str, str_style);
+    let mut unverified_pieces = vec![];
     let mut pieces = vec![];
 
-    while let Some(mut piece) = parser.next() {
+    while let Some(piece) = parser.next() {
         if !parser.errors.is_empty() {
             break;
         }
+        unverified_pieces.push(piece);
+    }
+
+    cx.arg_spans = parser.arg_places.iter()
+        .map(|&(start, end)| fmt.span.from_inner_byte_pos(start, end))
+        .collect();
+
+    // This needs to happen *after* the Parser has consumed all pieces to create all the spans
+    for mut piece in unverified_pieces {
         cx.verify_piece(&piece);
         cx.resolve_name_inplace(&mut piece);
         pieces.push(piece);
@@ -856,24 +915,27 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
             errs.push((cx.args[i].span, msg));
         }
     }
-    if errs.len() > 0 {
-        let args_used = cx.arg_types.len() - errs.len();
-        let args_unused = errs.len();
+    let errs_len = errs.len();
+    if errs_len > 0 {
+        let args_used = cx.arg_types.len() - errs_len;
+        let args_unused = errs_len;
 
         let mut diag = {
-            if errs.len() == 1 {
+            if errs_len == 1 {
                 let (sp, msg) = errs.into_iter().next().unwrap();
                 cx.ecx.struct_span_err(sp, msg)
             } else {
                 let mut diag = cx.ecx.struct_span_err(
                     errs.iter().map(|&(sp, _)| sp).collect::<Vec<Span>>(),
-                    "multiple unused formatting arguments"
+                    "multiple unused formatting arguments",
                 );
-                diag.span_label(cx.fmtsp, "multiple missing formatting arguments");
+                diag.span_label(cx.fmtsp, "multiple missing formatting specifiers");
                 diag
             }
         };
 
+        // Used to ensure we only report translations for *one* kind of foreign format.
+        let mut found_foreign = false;
         // Decide if we want to look for foreign formatting directives.
         if args_used < args_unused {
             use super::format_foreign as foreign;
@@ -882,13 +944,11 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
             // with `%d should be written as {}` over and over again.
             let mut explained = HashSet::new();
 
-            // Used to ensure we only report translations for *one* kind of foreign format.
-            let mut found_foreign = false;
-
             macro_rules! check_foreign {
                 ($kind:ident) => {{
                     let mut show_doc_note = false;
 
+                    let mut suggestions = vec![];
                     for sub in foreign::$kind::iter_subs(fmt_str) {
                         let trn = match sub.translate() {
                             Some(trn) => trn,
@@ -897,6 +957,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
                             None => continue,
                         };
 
+                        let pos = sub.position();
                         let sub = String::from(sub.as_str());
                         if explained.contains(&sub) {
                             continue;
@@ -908,7 +969,14 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
                             show_doc_note = true;
                         }
 
-                        diag.help(&format!("`{}` should be written as `{}`", sub, trn));
+                        if let Some((start, end)) = pos {
+                            // account for `"` and account for raw strings `r#`
+                            let padding = str_style.map(|i| i + 2).unwrap_or(1);
+                            let sp = fmt_sp.from_inner_byte_pos(start + padding, end + padding);
+                            suggestions.push((sp, trn));
+                        } else {
+                            diag.help(&format!("`{}` should be written as `{}`", sub, trn));
+                        }
                     }
 
                     if show_doc_note {
@@ -917,6 +985,12 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
                             " formatting not supported; see the documentation for `std::fmt`",
                         ));
                     }
+                    if suggestions.len() > 0 {
+                        diag.multipart_suggestion(
+                            "format specifiers use curly braces",
+                            suggestions,
+                        );
+                    }
                 }};
             }
 
@@ -925,6 +999,9 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
                 check_foreign!(shell);
             }
         }
+        if !found_foreign && errs_len == 1 {
+            diag.span_label(cx.fmtsp, "formatting specifier missing");
+        }
 
         diag.emit();
     }
diff --git a/src/libsyntax_ext/format_foreign.rs b/src/libsyntax_ext/format_foreign.rs
index 8ccb3be1ad9..23a37ca3485 100644
--- a/src/libsyntax_ext/format_foreign.rs
+++ b/src/libsyntax_ext/format_foreign.rs
@@ -14,7 +14,7 @@ pub mod printf {
     /// Represents a single `printf`-style substitution.
     #[derive(Clone, PartialEq, Debug)]
     pub enum Substitution<'a> {
-        /// A formatted output substitution.
+        /// A formatted output substitution with its internal byte offset.
         Format(Format<'a>),
         /// A literal `%%` escape.
         Escape,
@@ -28,6 +28,23 @@ pub mod printf {
             }
         }
 
+        pub fn position(&self) -> Option<(usize, usize)> {
+            match *self {
+                Substitution::Format(ref fmt) => Some(fmt.position),
+                _ => None,
+            }
+        }
+
+        pub fn set_position(&mut self, start: usize, end: usize) {
+            match self {
+                Substitution::Format(ref mut fmt) => {
+                    fmt.position = (start, end);
+                }
+                _ => {}
+            }
+        }
+
+
         /// Translate this substitution into an equivalent Rust formatting directive.
         ///
         /// This ignores cases where the substitution does not have an exact equivalent, or where
@@ -57,6 +74,8 @@ pub mod printf {
         pub length: Option<&'a str>,
         /// Type of parameter being converted.
         pub type_: &'a str,
+        /// Byte offset for the start and end of this formatting directive.
+        pub position: (usize, usize),
     }
 
     impl<'a> Format<'a> {
@@ -257,19 +276,28 @@ pub mod printf {
     pub fn iter_subs(s: &str) -> Substitutions {
         Substitutions {
             s,
+            pos: 0,
         }
     }
 
     /// Iterator over substitutions in a string.
     pub struct Substitutions<'a> {
         s: &'a str,
+        pos: usize,
     }
 
     impl<'a> Iterator for Substitutions<'a> {
         type Item = Substitution<'a>;
         fn next(&mut self) -> Option<Self::Item> {
-            let (sub, tail) = parse_next_substitution(self.s)?;
+            let (mut sub, tail) = parse_next_substitution(self.s)?;
             self.s = tail;
+            match sub {
+                Substitution::Format(_) => if let Some((start, end)) = sub.position() {
+                    sub.set_position(start + self.pos, end + self.pos);
+                    self.pos += end;
+                }
+                Substitution::Escape => self.pos += 2,
+            }
             Some(sub)
         }
 
@@ -301,7 +329,7 @@ pub mod printf {
                 _ => {/* fall-through */},
             }
 
-            Cur::new_at_start(&s[start..])
+            Cur::new_at(&s[..], start)
         };
 
         // This is meant to be a translation of the following regex:
@@ -355,6 +383,7 @@ pub mod printf {
                     precision: None,
                     length: None,
                     type_: at.slice_between(next).unwrap(),
+                    position: (start.at, next.at),
                 }),
                 next.slice_after()
             ));
@@ -541,6 +570,7 @@ pub mod printf {
         drop(next);
 
         end = at;
+        let position = (start.at, end.at);
 
         let f = Format {
             span: start.slice_between(end).unwrap(),
@@ -550,6 +580,7 @@ pub mod printf {
             precision,
             length,
             type_,
+            position,
         };
         Some((Substitution::Format(f), end.slice_after()))
     }
@@ -616,6 +647,7 @@ pub mod printf {
                 ($in_:expr, {
                     $param:expr, $flags:expr,
                     $width:expr, $prec:expr, $len:expr, $type_:expr,
+                    $pos:expr,
                 }) => {
                     assert_eq!(
                         pns(concat!($in_, "!")),
@@ -628,6 +660,7 @@ pub mod printf {
                                 precision: $prec,
                                 length: $len,
                                 type_: $type_,
+                                position: $pos,
                             }),
                             "!"
                         ))
@@ -636,53 +669,53 @@ pub mod printf {
             }
 
             assert_pns_eq_sub!("%!",
-                { None, "", None, None, None, "!", });
+                { None, "", None, None, None, "!", (0, 2), });
             assert_pns_eq_sub!("%c",
-                { None, "", None, None, None, "c", });
+                { None, "", None, None, None, "c", (0, 2), });
             assert_pns_eq_sub!("%s",
-                { None, "", None, None, None, "s", });
+                { None, "", None, None, None, "s", (0, 2), });
             assert_pns_eq_sub!("%06d",
-                { None, "0", Some(N::Num(6)), None, None, "d", });
+                { None, "0", Some(N::Num(6)), None, None, "d", (0, 4), });
             assert_pns_eq_sub!("%4.2f",
-                { None, "", Some(N::Num(4)), Some(N::Num(2)), None, "f", });
+                { None, "", Some(N::Num(4)), Some(N::Num(2)), None, "f", (0, 5), });
             assert_pns_eq_sub!("%#x",
-                { None, "#", None, None, None, "x", });
+                { None, "#", None, None, None, "x", (0, 3), });
             assert_pns_eq_sub!("%-10s",
-                { None, "-", Some(N::Num(10)), None, None, "s", });
+                { None, "-", Some(N::Num(10)), None, None, "s", (0, 5), });
             assert_pns_eq_sub!("%*s",
-                { None, "", Some(N::Next), None, None, "s", });
+                { None, "", Some(N::Next), None, None, "s", (0, 3), });
             assert_pns_eq_sub!("%-10.*s",
-                { None, "-", Some(N::Num(10)), Some(N::Next), None, "s", });
+                { None, "-", Some(N::Num(10)), Some(N::Next), None, "s", (0, 7), });
             assert_pns_eq_sub!("%-*.*s",
-                { None, "-", Some(N::Next), Some(N::Next), None, "s", });
+                { None, "-", Some(N::Next), Some(N::Next), None, "s", (0, 6), });
             assert_pns_eq_sub!("%.6i",
-                { None, "", None, Some(N::Num(6)), None, "i", });
+                { None, "", None, Some(N::Num(6)), None, "i", (0, 4), });
             assert_pns_eq_sub!("%+i",
-                { None, "+", None, None, None, "i", });
+                { None, "+", None, None, None, "i", (0, 3), });
             assert_pns_eq_sub!("%08X",
-                { None, "0", Some(N::Num(8)), None, None, "X", });
+                { None, "0", Some(N::Num(8)), None, None, "X", (0, 4), });
             assert_pns_eq_sub!("%lu",
-                { None, "", None, None, Some("l"), "u", });
+                { None, "", None, None, Some("l"), "u", (0, 3), });
             assert_pns_eq_sub!("%Iu",
-                { None, "", None, None, Some("I"), "u", });
+                { None, "", None, None, Some("I"), "u", (0, 3), });
             assert_pns_eq_sub!("%I32u",
-                { None, "", None, None, Some("I32"), "u", });
+                { None, "", None, None, Some("I32"), "u", (0, 5), });
             assert_pns_eq_sub!("%I64u",
-                { None, "", None, None, Some("I64"), "u", });
+                { None, "", None, None, Some("I64"), "u", (0, 5), });
             assert_pns_eq_sub!("%'d",
-                { None, "'", None, None, None, "d", });
+                { None, "'", None, None, None, "d", (0, 3), });
             assert_pns_eq_sub!("%10s",
-                { None, "", Some(N::Num(10)), None, None, "s", });
+                { None, "", Some(N::Num(10)), None, None, "s", (0, 4), });
             assert_pns_eq_sub!("%-10.10s",
-                { None, "-", Some(N::Num(10)), Some(N::Num(10)), None, "s", });
+                { None, "-", Some(N::Num(10)), Some(N::Num(10)), None, "s", (0, 8), });
             assert_pns_eq_sub!("%1$d",
-                { Some(1), "", None, None, None, "d", });
+                { Some(1), "", None, None, None, "d", (0, 4), });
             assert_pns_eq_sub!("%2$.*3$d",
-                { Some(2), "", None, Some(N::Arg(3)), None, "d", });
+                { Some(2), "", None, Some(N::Arg(3)), None, "d", (0, 8), });
             assert_pns_eq_sub!("%1$*2$.*3$d",
-                { Some(1), "", Some(N::Arg(2)), Some(N::Arg(3)), None, "d", });
+                { Some(1), "", Some(N::Arg(2)), Some(N::Arg(3)), None, "d", (0, 11), });
             assert_pns_eq_sub!("%-8ld",
-                { None, "-", Some(N::Num(8)), None, Some("l"), "d", });
+                { None, "-", Some(N::Num(8)), None, Some("l"), "d", (0, 5), });
         }
 
         #[test]
@@ -755,6 +788,12 @@ pub mod shell {
             }
         }
 
+        pub fn position(&self) -> Option<(usize, usize)> {
+            match *self {
+                _ => None,
+            }
+        }
+
         pub fn translate(&self) -> Option<String> {
             match *self {
                 Substitution::Ordinal(n) => Some(format!("{{{}}}", n)),
@@ -918,7 +957,7 @@ mod strcursor {
 
     pub struct StrCursor<'a> {
         s: &'a str,
-        at: usize,
+        pub at: usize,
     }
 
     impl<'a> StrCursor<'a> {
@@ -929,6 +968,13 @@ mod strcursor {
             }
         }
 
+        pub fn new_at(s: &'a str, at: usize) -> StrCursor<'a> {
+            StrCursor {
+                s,
+                at,
+            }
+        }
+
         pub fn at_next_cp(mut self) -> Option<StrCursor<'a>> {
             match self.try_seek_right_cp() {
                 true => Some(self),
diff --git a/src/test/compile-fail/ifmt-bad-arg.rs b/src/test/ui/ifmt-bad-arg.rs
index afe9bc152a3..bbd75f30d6c 100644
--- a/src/test/compile-fail/ifmt-bad-arg.rs
+++ b/src/test/ui/ifmt-bad-arg.rs
@@ -64,4 +64,11 @@ fn main() {
     format!("foo }"); //~ ERROR: unmatched `}` found
 
     format!("foo %s baz", "bar"); //~ ERROR: argument never used
+
+    format!(r##"
+
+        {foo}
+
+    "##);
+    //~^^^ ERROR: there is no argument named `foo`
 }
diff --git a/src/test/ui/ifmt-bad-arg.stderr b/src/test/ui/ifmt-bad-arg.stderr
new file mode 100644
index 00000000000..c8fd8bad19b
--- /dev/null
+++ b/src/test/ui/ifmt-bad-arg.stderr
@@ -0,0 +1,208 @@
+error: 1 positional argument in format string, but no arguments were given
+  --> $DIR/ifmt-bad-arg.rs:16:14
+   |
+LL |     format!("{}");
+   |              ^^
+
+error: invalid reference to positional argument 1 (there is 1 argument)
+  --> $DIR/ifmt-bad-arg.rs:19:14
+   |
+LL |     format!("{1}", 1);
+   |              ^^^
+   |
+   = note: positional arguments are zero-based
+
+error: argument never used
+  --> $DIR/ifmt-bad-arg.rs:19:20
+   |
+LL |     format!("{1}", 1);
+   |             -----  ^
+   |             |
+   |             formatting specifier missing
+
+error: 2 positional arguments in format string, but no arguments were given
+  --> $DIR/ifmt-bad-arg.rs:23:14
+   |
+LL |     format!("{} {}");
+   |              ^^ ^^
+
+error: invalid reference to positional argument 1 (there is 1 argument)
+  --> $DIR/ifmt-bad-arg.rs:26:18
+   |
+LL |     format!("{0} {1}", 1);
+   |                  ^^^
+   |
+   = note: positional arguments are zero-based
+
+error: invalid reference to positional argument 2 (there are 2 arguments)
+  --> $DIR/ifmt-bad-arg.rs:29:22
+   |
+LL |     format!("{0} {1} {2}", 1, 2);
+   |                      ^^^
+   |
+   = note: positional arguments are zero-based
+
+error: invalid reference to positional argument 2 (there are 2 arguments)
+  --> $DIR/ifmt-bad-arg.rs:32:28
+   |
+LL |     format!("{} {value} {} {}", 1, value=2);
+   |                            ^^
+   |
+   = note: positional arguments are zero-based
+
+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
+
+error: there is no argument named `foo`
+  --> $DIR/ifmt-bad-arg.rs:37:17
+   |
+LL |     format!("{} {foo} {} {bar} {}", 1, 2, 3);
+   |                 ^^^^^
+
+error: there is no argument named `bar`
+  --> $DIR/ifmt-bad-arg.rs:37:26
+   |
+LL |     format!("{} {foo} {} {bar} {}", 1, 2, 3);
+   |                          ^^^^^
+
+error: there is no argument named `foo`
+  --> $DIR/ifmt-bad-arg.rs:41:14
+   |
+LL |     format!("{foo}");                //~ ERROR: no argument named `foo`
+   |              ^^^^^
+
+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
+
+error: argument never used
+  --> $DIR/ifmt-bad-arg.rs:43:22
+   |
+LL |     format!("{}", 1, 2);             //~ ERROR: argument never used
+   |             ----     ^
+   |             |
+   |             formatting specifier missing
+
+error: argument never used
+  --> $DIR/ifmt-bad-arg.rs:44:20
+   |
+LL |     format!("{1}", 1, 2);            //~ ERROR: argument never used
+   |             -----  ^
+   |             |
+   |             formatting specifier missing
+
+error: named argument never used
+  --> $DIR/ifmt-bad-arg.rs:45:26
+   |
+LL |     format!("{}", 1, foo=2);         //~ ERROR: named argument never used
+   |             ----         ^
+   |             |
+   |             formatting specifier missing
+
+error: argument never used
+  --> $DIR/ifmt-bad-arg.rs:46:22
+   |
+LL |     format!("{foo}", 1, foo=2);      //~ ERROR: argument never used
+   |             -------  ^
+   |             |
+   |             formatting specifier missing
+
+error: named argument never used
+  --> $DIR/ifmt-bad-arg.rs:47:21
+   |
+LL |     format!("", foo=2);              //~ ERROR: named argument never used
+   |             --      ^
+   |             |
+   |             formatting specifier missing
+
+error: multiple unused formatting arguments
+  --> $DIR/ifmt-bad-arg.rs:48:32
+   |
+LL |     format!("{} {}", 1, 2, foo=1, bar=2);  //~ ERROR: multiple unused formatting arguments
+   |             -------            ^      ^
+   |             |
+   |             multiple missing formatting specifiers
+
+error: duplicate argument named `foo`
+  --> $DIR/ifmt-bad-arg.rs:50:33
+   |
+LL |     format!("{foo}", foo=1, foo=2);  //~ ERROR: duplicate argument
+   |                                 ^
+   |
+note: previously here
+  --> $DIR/ifmt-bad-arg.rs:50:26
+   |
+LL |     format!("{foo}", foo=1, foo=2);  //~ ERROR: duplicate argument
+   |                          ^
+
+error: expected ident, positional arguments cannot follow named arguments
+  --> $DIR/ifmt-bad-arg.rs:51:24
+   |
+LL |     format!("", foo=1, 2);           //~ ERROR: positional arguments cannot follow
+   |                        ^
+
+error: there is no argument named `valueb`
+  --> $DIR/ifmt-bad-arg.rs:55:23
+   |
+LL |     format!("{valuea} {valueb}", valuea=5, valuec=7);
+   |                       ^^^^^^^^
+
+error: named argument never used
+  --> $DIR/ifmt-bad-arg.rs:55:51
+   |
+LL |     format!("{valuea} {valueb}", valuea=5, valuec=7);
+   |             -------------------                   ^
+   |             |
+   |             formatting specifier missing
+
+error: invalid format string: expected `'}'` but string was terminated
+  --> $DIR/ifmt-bad-arg.rs:61:15
+   |
+LL |     format!("{"); //~ ERROR: expected `'}'` but string was terminated
+   |               ^ expected `'}'` in format string
+   |
+   = note: if you intended to print `{`, you can escape it using `{{`
+
+error: invalid format string: unmatched `}` found
+  --> $DIR/ifmt-bad-arg.rs:63:18
+   |
+LL |     format!("foo } bar"); //~ ERROR: unmatched `}` found
+   |                  ^ unmatched `}` in format string
+   |
+   = note: if you intended to print `}`, you can escape it using `}}`
+
+error: invalid format string: unmatched `}` found
+  --> $DIR/ifmt-bad-arg.rs:64:18
+   |
+LL |     format!("foo }"); //~ ERROR: unmatched `}` found
+   |                  ^ unmatched `}` in format string
+   |
+   = note: if you intended to print `}`, you can escape it using `}}`
+
+error: argument never used
+  --> $DIR/ifmt-bad-arg.rs:66:27
+   |
+LL |     format!("foo %s baz", "bar"); //~ ERROR: argument never used
+   |                  --       ^^^^^
+   |                  |
+   |                  help: format specifiers use curly braces: `{}`
+   |
+   = note: printf formatting not supported; see the documentation for `std::fmt`
+
+error: there is no argument named `foo`
+  --> $DIR/ifmt-bad-arg.rs:70:9
+   |
+LL |         {foo}
+   |         ^^^^^
+
+error: aborting due to 27 previous errors
+
diff --git a/src/test/ui/macros/format-foreign.rs b/src/test/ui/macros/format-foreign.rs
index ec0eaed43ae..33401424c9a 100644
--- a/src/test/ui/macros/format-foreign.rs
+++ b/src/test/ui/macros/format-foreign.rs
@@ -11,6 +11,11 @@
 fn main() {
     println!("%.*3$s %s!\n", "Hello,", "World", 4); //~ ERROR multiple unused formatting arguments
     println!("%1$*2$.*3$f", 123.456); //~ ERROR never used
+    println!(r###"%.*3$s
+        %s!\n
+"###, "Hello,", "World", 4);
+    //~^ ERROR multiple unused formatting arguments
+    // correctly account for raw strings in inline suggestions
 
     // This should *not* produce hints, on the basis that there's equally as
     // many "correct" format specifiers.  It's *probably* just an actual typo.
diff --git a/src/test/ui/macros/format-foreign.stderr b/src/test/ui/macros/format-foreign.stderr
index 401b2f6d67e..5e76c0a322e 100644
--- a/src/test/ui/macros/format-foreign.stderr
+++ b/src/test/ui/macros/format-foreign.stderr
@@ -4,29 +4,52 @@ error: multiple unused formatting arguments
 LL |     println!("%.*3$s %s!/n", "Hello,", "World", 4); //~ ERROR multiple unused formatting arguments
    |              --------------  ^^^^^^^^  ^^^^^^^  ^
    |              |
-   |              multiple missing formatting arguments
+   |              multiple missing formatting specifiers
    |
-   = help: `%.*3$s` should be written as `{:.2$}`
-   = help: `%s` should be written as `{}`
    = note: printf formatting not supported; see the documentation for `std::fmt`
+help: format specifiers use curly braces
+   |
+LL |     println!("{:.2$} {}!/n", "Hello,", "World", 4); //~ ERROR multiple unused formatting arguments
+   |               ^^^^^^ ^^
 
 error: argument never used
   --> $DIR/format-foreign.rs:13:29
    |
 LL |     println!("%1$*2$.*3$f", 123.456); //~ ERROR never used
-   |                             ^^^^^^^
+   |               -----------   ^^^^^^^
+   |               |
+   |               help: format specifiers use curly braces: `{0:1$.2$}`
    |
-   = help: `%1$*2$.*3$f` should be written as `{0:1$.2$}`
    = note: printf formatting not supported; see the documentation for `std::fmt`
 
+error: multiple unused formatting arguments
+  --> $DIR/format-foreign.rs:16:7
+   |
+LL |       println!(r###"%.*3$s
+   |  ______________-
+LL | |         %s!/n
+LL | | "###, "Hello,", "World", 4);
+   | |    -  ^^^^^^^^  ^^^^^^^  ^
+   | |____|
+   |      multiple missing formatting specifiers
+   |
+   = note: printf formatting not supported; see the documentation for `std::fmt`
+help: format specifiers use curly braces
+   |
+LL |     println!(r###"{:.2$}
+LL |         {}!/n
+   |
+
 error: argument never used
-  --> $DIR/format-foreign.rs:17:30
+  --> $DIR/format-foreign.rs:22:30
    |
 LL |     println!("{} %f", "one", 2.0); //~ ERROR never used
-   |                              ^^^
+   |              -------         ^^^
+   |              |
+   |              formatting specifier missing
 
 error: named argument never used
-  --> $DIR/format-foreign.rs:19:39
+  --> $DIR/format-foreign.rs:24:39
    |
 LL |     println!("Hi there, $NAME.", NAME="Tim"); //~ ERROR never used
    |                                       ^^^^^
@@ -34,5 +57,5 @@ LL |     println!("Hi there, $NAME.", NAME="Tim"); //~ ERROR never used
    = help: `$NAME` should be written as `{NAME}`
    = note: shell formatting not supported; see the documentation for `std::fmt`
 
-error: aborting due to 4 previous errors
+error: aborting due to 5 previous errors
 
diff --git a/src/test/ui/macros/format-unused-lables.stderr b/src/test/ui/macros/format-unused-lables.stderr
index f764190438f..81171a1ed01 100644
--- a/src/test/ui/macros/format-unused-lables.stderr
+++ b/src/test/ui/macros/format-unused-lables.stderr
@@ -4,13 +4,13 @@ error: multiple unused formatting arguments
 LL |     println!("Test", 123, 456, 789);
    |              ------  ^^^  ^^^  ^^^
    |              |
-   |              multiple missing formatting arguments
+   |              multiple missing formatting specifiers
 
 error: multiple unused formatting arguments
   --> $DIR/format-unused-lables.rs:16:9
    |
 LL |     println!("Test2",
-   |              ------- multiple missing formatting arguments
+   |              ------- multiple missing formatting specifiers
 LL |         123,  //~ ERROR multiple unused formatting arguments
    |         ^^^
 LL |         456,
@@ -22,13 +22,15 @@ error: named argument never used
   --> $DIR/format-unused-lables.rs:21:35
    |
 LL |     println!("Some stuff", UNUSED="args"); //~ ERROR named argument never used
-   |                                   ^^^^^^
+   |              ------------         ^^^^^^
+   |              |
+   |              formatting specifier missing
 
 error: multiple unused formatting arguments
   --> $DIR/format-unused-lables.rs:24:9
    |
 LL |     println!("Some more $STUFF",
-   |              ------------------ multiple missing formatting arguments
+   |              ------------------ multiple missing formatting specifiers
 LL |         "woo!",  //~ ERROR multiple unused formatting arguments
    |         ^^^^^^
 LL |             STUFF=