Point spans to inner elements of format strings

src/libfmt_macros/lib.rs

@@ -154,6 +154,7 @@ 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,
+    pub arg_places: Vec<(usize, usize)>,
 }
 
 impl<'a> Iterator for Parser<'a> {
@@ -168,9 +169,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 +216,7 @@ impl<'a> Parser<'a> {
             curarg: 0,
             style,
             seen_newlines: 0,
+            arg_places: vec![],
         }
     }
 
@@ -271,20 +277,22 @@ 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);
 
         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 + 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 +310,7 @@ impl<'a> Parser<'a> {
             } else {
                 self.err(msg, format!("expected `{:?}`", c), pos, pos);
             }
+            None
         }
     }
 

src/libsyntax_ext/format.rs

@@ -21,7 +21,7 @@ use syntax::feature_gate;
 use syntax::parse::token;
 use syntax::ptr::P;
 use syntax::symbol::Symbol;
-use syntax_pos::{Span, DUMMY_SP};
+use syntax_pos::{Span, MultiSpan, DUMMY_SP};
 use syntax::tokenstream;
 
 use std::collections::{HashMap, HashSet};
@@ -111,8 +111,10 @@ struct Context<'a, 'b: 'a> {
     /// still existed in this phase of processing.
     /// Used only for `all_pieces_simple` tracking in `build_piece`.
     curarg: usize,
+    curpiece: usize,
     /// Keep track of invalid references to positional arguments
     invalid_refs: Vec<usize>,
+    arg_spans: Vec<Span>,
 }
 
 /// Parses the arguments from the given list of tokens, returning None
@@ -235,6 +237,7 @@ impl<'a, 'b> Context<'a, 'b> {
 
                 let ty = Placeholder(arg.format.ty.to_string());
                 self.verify_arg_type(pos, ty);
+                self.curpiece += 1;
             }
         }
     }
@@ -264,28 +267,38 @@ impl<'a, 'b> Context<'a, 'b> {
     /// errors for the case where all arguments are positional and for when
     /// there are named arguments or numbered positional arguments in the
     /// format string.
-    fn report_invalid_references(&self, numbered_position_args: bool) {
+    fn report_invalid_references(&self, numbered_position_args: bool, arg_places: &[(usize, usize)]) {
         let mut e;
-        let mut refs: Vec<String> = self.invalid_refs
-                                        .iter()
-                                        .map(|r| r.to_string())
-                                        .collect();
+        let sps = arg_places.iter()
+            .map(|&(start, end)| self.fmtsp.from_inner_byte_pos(start, end))
+            .collect::<Vec<_>>();
+        let sp = MultiSpan::from_spans(sps);
+        let mut refs: Vec<_> = self.invalid_refs
+            .iter()
+            .map(|r| r.to_string())
+            .collect();
 
         if self.names.is_empty() && !numbered_position_args {
-            e = self.ecx.mut_span_err(self.fmtsp,
+            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()));
         } else {
             let arg_list = match refs.len() {
-                1 => format!("argument {}", refs.pop().unwrap()),
-                _ => format!("arguments {head} and {tail}",
-                             tail=refs.pop().unwrap(),
+                1 => {
+                    let reg = refs.pop().unwrap();
+                    format!("argument {}", reg)
+                },
+                _ => {
+                    let reg = refs.pop().unwrap();
+                    format!("arguments {head} and {tail}",
+                             tail=reg,
                              head=refs.join(", "))
+                }
             };
 
-            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()));
@@ -337,7 +350,9 @@ 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 = *self.arg_spans.get(self.curpiece).unwrap_or(&self.fmtsp);
+                        let mut err = self.ecx.struct_span_err(sp, &msg[..]);
+                        err.emit();
                         return;
                     }
                 };
@@ -763,6 +778,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 +791,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
         macsp,
         fmtsp: fmt.span,
         invalid_refs: Vec::new(),
+        arg_spans: Vec::new(),
     };
 
     let fmt_str = &*fmt.node.0.as_str();
@@ -783,12 +800,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);
@@ -835,7 +862,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
     }
 
     if cx.invalid_refs.len() >= 1 {
-        cx.report_invalid_references(numbered_position_args);
+        cx.report_invalid_references(numbered_position_args, &parser.arg_places);
     }
 
     // Make sure that all arguments were used and all arguments have types.

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

@@ -0,0 +1,187 @@
+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);
+   |                    ^
+
+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:14
+   |
+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:14
+   |
+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:14
+   |
+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:14
+   |
+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 arguments
+
+error: argument never used
+  --> $DIR/ifmt-bad-arg.rs:43:22
+   |
+LL |     format!("{}", 1, 2);             //~ ERROR: argument never used
+   |                      ^
+
+error: argument never used
+  --> $DIR/ifmt-bad-arg.rs:44:20
+   |
+LL |     format!("{1}", 1, 2);            //~ ERROR: argument never used
+   |                    ^
+
+error: named argument never used
+  --> $DIR/ifmt-bad-arg.rs:45:26
+   |
+LL |     format!("{}", 1, foo=2);         //~ ERROR: named argument never used
+   |                          ^
+
+error: argument never used
+  --> $DIR/ifmt-bad-arg.rs:46:22
+   |
+LL |     format!("{foo}", 1, foo=2);      //~ ERROR: argument never used
+   |                      ^
+
+error: named argument never used
+  --> $DIR/ifmt-bad-arg.rs:47:21
+   |
+LL |     format!("", foo=2);              //~ ERROR: named argument never used
+   |                     ^
+
+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 arguments
+
+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);
+   |                                                   ^
+
+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: `%s` should be written as `{}`
+   = note: printf formatting not supported; see the documentation for `std::fmt`
+
+error: aborting due to 26 previous errors
+