Auto merge of #45807 - tommyip:format_err, r=estebank

Make positional argument error in format! clearer

r? @estebank

Fixes #44954

1 files changed, 59 insertions, 8 deletions

diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs
index 63c533df198..ad5bd39a453 100644
--- a/src/libsyntax_ext/format.rs
+++ b/src/libsyntax_ext/format.rs
@@ -110,6 +110,8 @@ struct Context<'a, 'b: 'a> {
     /// still existed in this phase of processing.
     /// Used only for `all_pieces_simple` tracking in `trans_piece`.
     curarg: usize,
+    /// Keep track of invalid references to positional arguments
+    invalid_refs: Vec<usize>,
 }
 
 /// Parses the arguments from the given list of tokens, returning None
@@ -226,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()),
                 };
 
@@ -251,23 +253,54 @@ impl<'a, 'b> Context<'a, 'b> {
 
     fn describe_num_args(&self) -> String {
         match self.args.len() {
-            0 => "no arguments given".to_string(),
+            0 => "no arguments were given".to_string(),
             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 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();
+
+        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()),
+                _ => format!("arguments {head} and {tail}",
+                             tail=refs.pop().unwrap(),
+                             head=refs.join(", "))
+            };
+
+            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");
+        };
+
+        e.emit();
+    }
+
     /// Actually verifies and tracks a given format placeholder
     /// (a.k.a. argument).
     fn verify_arg_type(&mut self, arg: Position, ty: ArgumentType) {
         match arg {
             Exact(arg) => {
                 if self.args.len() <= arg {
-                    let msg = format!("invalid reference to argument `{}` ({})",
-                                      arg,
-                                      self.describe_num_args());
-
-                    self.ecx.span_err(self.fmtsp, &msg[..]);
+                    self.invalid_refs.push(arg);
                     return;
                 }
                 match ty {
@@ -403,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) {
@@ -691,6 +725,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
         all_pieces_simple: true,
         macsp,
         fmtsp: fmt.span,
+        invalid_refs: Vec::new(),
     };
 
     let fmt_str = &*fmt.node.0.as_str();
@@ -711,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()];
@@ -736,6 +783,10 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
         cx.str_pieces.push(s);
     }
 
+    if cx.invalid_refs.len() >= 1 {
+        cx.report_invalid_references(numbered_position_args);
+    }
+
     // Make sure that all arguments were used and all arguments have types.
     let num_pos_args = cx.args.len() - cx.names.len();
     let mut errs = vec![];