Retain information on whether a format argument has explicit position

4 files changed, 57 insertions, 32 deletions

diff --git a/src/libfmt_macros/lib.rs b/src/libfmt_macros/lib.rs
index 24430b2e377..44cdb5e8a36 100644
--- a/src/libfmt_macros/lib.rs
+++ b/src/libfmt_macros/lib.rs
@@ -73,7 +73,9 @@ pub struct FormatSpec<'a> {
 /// Enum describing where an argument for a format can be located.
 #[derive(Copy, Clone, PartialEq)]
 pub enum Position<'a> {
-    /// The argument is located at a specific index.
+    /// The arugment is implied to be located at an index
+    ArgumentImplicitlyIs(usize),
+    /// The argument is located at a specific index given in the format
     ArgumentIs(usize),
     /// The argument has a name.
     ArgumentNamed(&'a str),
@@ -275,7 +277,7 @@ impl<'a> Parser<'a> {
             None => {
                 let i = self.curarg;
                 self.curarg += 1;
-                ArgumentIs(i)
+                ArgumentImplicitlyIs(i)
             }
         };
 
@@ -517,7 +519,7 @@ mod tests {
     fn format_nothing() {
         same("{}",
              &[NextArgument(Argument {
-                   position: ArgumentIs(0),
+                   position: ArgumentImplicitlyIs(0),
                    format: fmtdflt(),
                })]);
     }
@@ -595,7 +597,7 @@ mod tests {
     fn format_counts() {
         same("{:10s}",
              &[NextArgument(Argument {
-                   position: ArgumentIs(0),
+                   position: ArgumentImplicitlyIs(0),
                    format: FormatSpec {
                        fill: None,
                        align: AlignUnknown,
@@ -607,7 +609,7 @@ mod tests {
                })]);
         same("{:10$.10s}",
              &[NextArgument(Argument {
-                   position: ArgumentIs(0),
+                   position: ArgumentImplicitlyIs(0),
                    format: FormatSpec {
                        fill: None,
                        align: AlignUnknown,
@@ -619,7 +621,7 @@ mod tests {
                })]);
         same("{:.*s}",
              &[NextArgument(Argument {
-                   position: ArgumentIs(1),
+                   position: ArgumentImplicitlyIs(1),
                    format: FormatSpec {
                        fill: None,
                        align: AlignUnknown,
@@ -631,7 +633,7 @@ mod tests {
                })]);
         same("{:.10$s}",
              &[NextArgument(Argument {
-                   position: ArgumentIs(0),
+                   position: ArgumentImplicitlyIs(0),
                    format: FormatSpec {
                        fill: None,
                        align: AlignUnknown,
@@ -643,7 +645,7 @@ mod tests {
                })]);
         same("{:a$.b$s}",
              &[NextArgument(Argument {
-                   position: ArgumentIs(0),
+                   position: ArgumentImplicitlyIs(0),
                    format: FormatSpec {
                        fill: None,
                        align: AlignUnknown,
@@ -658,7 +660,7 @@ mod tests {
     fn format_flags() {
         same("{:-}",
              &[NextArgument(Argument {
-                   position: ArgumentIs(0),
+                   position: ArgumentImplicitlyIs(0),
                    format: FormatSpec {
                        fill: None,
                        align: AlignUnknown,
@@ -670,7 +672,7 @@ mod tests {
                })]);
         same("{:+#}",
              &[NextArgument(Argument {
-                   position: ArgumentIs(0),
+                   position: ArgumentImplicitlyIs(0),
                    format: FormatSpec {
                        fill: None,
                        align: AlignUnknown,
diff --git a/src/librustc/traits/on_unimplemented.rs b/src/librustc/traits/on_unimplemented.rs
index 94f6efcad4a..16e200d56f9 100644
--- a/src/librustc/traits/on_unimplemented.rs
+++ b/src/librustc/traits/on_unimplemented.rs
@@ -254,7 +254,7 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedFormatString {
                         }
                     },
                     // `{:1}` and `{}` are not to be used
-                    Position::ArgumentIs(_) => {
+                    Position::ArgumentIs(_) | Position::ArgumentImplicitlyIs(_) => {
                         span_err!(tcx.sess, span, E0231,
                                   "only named substitution \
                                    parameters are allowed");
diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs
index ceefbfc271c..ad5bd39a453 100644
--- a/src/libsyntax_ext/format.rs
+++ b/src/libsyntax_ext/format.rs
@@ -228,7 +228,7 @@ impl<'a, 'b> Context<'a, 'b> {
                 // argument second, if it's an implicit positional parameter
                 // it's written second, so it should come after width/precision.
                 let pos = match arg.position {
-                    parse::ArgumentIs(i) => Exact(i),
+                    parse::ArgumentIs(i) | parse::ArgumentImplicitlyIs(i) => Exact(i),
                     parse::ArgumentNamed(s) => Named(s.to_string()),
                 };
 
@@ -254,25 +254,28 @@ impl<'a, 'b> Context<'a, 'b> {
     fn describe_num_args(&self) -> String {
         match self.args.len() {
             0 => "no arguments were given".to_string(),
-            1 => "there is only 1 argument".to_string(),
-            x => format!("there are only {} arguments", x),
+            1 => "there is 1 argument".to_string(),
+            x => format!("there are {} arguments", x),
         }
     }
 
     /// Handle invalid references to positional arguments. Output different
     /// errors for the case where all arguments are positional and for when
-    /// there are named arguments in the format string.
-    fn report_invalid_references(&self) {
+    /// there are named arguments or numbered positional arguments in the
+    /// 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 msg = if self.names.is_empty() {
-            format!("{} positional argument{} in format string, but {}",
-                    self.pieces.len(),
-                    if self.pieces.len() > 1 { "s" } else { "" },
-                    self.describe_num_args())
+        if self.names.is_empty() && !numbered_position_args {
+            e = self.ecx.mut_span_err(self.fmtsp,
+                &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()),
@@ -281,12 +284,14 @@ impl<'a, 'b> Context<'a, 'b> {
                              head=refs.join(", "))
             };
 
-            format!("invalid reference to positional {} ({})",
-                    arg_list,
-                    self.describe_num_args())
+            e = self.ecx.mut_span_err(self.fmtsp,
+                &format!("invalid reference to positional {} ({})",
+                        arg_list,
+                        self.describe_num_args()));
+            e.note("positional arguments are zero-based");
         };
 
-        self.ecx.span_err(self.fmtsp, &msg[..]);
+        e.emit();
     }
 
     /// Actually verifies and tracks a given format placeholder
@@ -431,7 +436,8 @@ impl<'a, 'b> Context<'a, 'b> {
                         }
                     };
                     match arg.position {
-                        parse::ArgumentIs(i) => {
+                        parse::ArgumentIs(i)
+                        | parse::ArgumentImplicitlyIs(i) => {
                             // Map to index in final generated argument array
                             // in case of multiple types specified
                             let arg_idx = match arg_index_consumed.get_mut(i) {
@@ -740,6 +746,18 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
         }
     }
 
+    let numbered_position_args = pieces.iter().any(|arg: &parse::Piece| {
+        match *arg {
+            parse::String(_) => false,
+            parse::NextArgument(arg) => {
+                match arg.position {
+                    parse::Position::ArgumentIs(_) => true,
+                    _ => false,
+                }
+            }
+        }
+    });
+
     cx.build_index_map();
 
     let mut arg_index_consumed = vec![0usize; cx.arg_index_map.len()];
@@ -766,7 +784,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
     }
 
     if cx.invalid_refs.len() >= 1 {
-        cx.report_invalid_references();
+        cx.report_invalid_references(numbered_position_args);
     }
 
     // Make sure that all arguments were used and all arguments have types.
diff --git a/src/test/compile-fail/ifmt-bad-arg.rs b/src/test/compile-fail/ifmt-bad-arg.rs
index 336a0d825cb..afe9bc152a3 100644
--- a/src/test/compile-fail/ifmt-bad-arg.rs
+++ b/src/test/compile-fail/ifmt-bad-arg.rs
@@ -17,22 +17,26 @@ fn main() {
     //~^ ERROR: 1 positional argument in format string, but no arguments were given
 
     format!("{1}", 1);
-    //~^ ERROR: 1 positional argument in format string, but there is only 1 argument
+    //~^ ERROR: invalid reference to positional argument 1 (there is 1 argument)
     //~^^ ERROR: argument never used
 
     format!("{} {}");
     //~^ ERROR: 2 positional arguments in format string, but no arguments were given
 
     format!("{0} {1}", 1);
-    //~^ ERROR: 2 positional arguments in format string, but there is only 1 argument
+    //~^ ERROR: invalid reference to positional argument 1 (there is 1 argument)
 
     format!("{0} {1} {2}", 1, 2);
-    //~^ ERROR: 3 positional arguments in format string, but there are only 2 arguments
+    //~^ ERROR: invalid reference to positional argument 2 (there are 2 arguments)
 
     format!("{} {value} {} {}", 1, value=2);
-    //~^ ERROR: invalid reference to positional argument 2 (there are only 2 arguments)
+    //~^ ERROR: invalid reference to positional argument 2 (there are 2 arguments)
     format!("{name} {value} {} {} {} {} {} {}", 0, name=1, value=2);
-    //~^ ERROR: invalid reference to positional arguments 3, 4 and 5 (there are only 3 arguments)
+    //~^ ERROR: invalid reference to positional arguments 3, 4 and 5 (there are 3 arguments)
+
+    format!("{} {foo} {} {bar} {}", 1, 2, 3);
+    //~^ ERROR: there is no argument named `foo`
+    //~^^ ERROR: there is no argument named `bar`
 
     format!("{foo}");                //~ ERROR: no argument named `foo`
     format!("", 1, 2);               //~ ERROR: multiple unused formatting arguments
@@ -41,6 +45,7 @@ fn main() {
     format!("{}", 1, foo=2);         //~ ERROR: named argument never used
     format!("{foo}", 1, foo=2);      //~ ERROR: argument never used
     format!("", foo=2);              //~ ERROR: named argument never used
+    format!("{} {}", 1, 2, foo=1, bar=2);  //~ ERROR: multiple unused formatting arguments
 
     format!("{foo}", foo=1, foo=2);  //~ ERROR: duplicate argument
     format!("", foo=1, 2);           //~ ERROR: positional arguments cannot follow