From 82d5ea4b12d8543c6c8f871881fbbefbf5e63dc2 Mon Sep 17 00:00:00 2001
From: Tommy Ip <hkmp7tommy@gmail.com>
Date: Mon, 6 Nov 2017 08:49:47 +0000
Subject: [PATCH] Make format! positional argument errors clear

---
 src/libsyntax_ext/format.rs | 49 +++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs
index 63c533df198..ceefbfc271c 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
@@ -251,23 +253,49 @@ impl<'a, 'b> Context<'a, 'b> {
 
     fn describe_num_args(&self) -> String {
         match self.args.len() {
-            0 => "no arguments given".to_string(),
-            1 => "there is 1 argument".to_string(),
-            x => format!("there are {} arguments", x),
+            0 => "no arguments were given".to_string(),
+            1 => "there is only 1 argument".to_string(),
+            x => format!("there are only {} 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) {
+        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())
+        } 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(", "))
+            };
+
+            format!("invalid reference to positional {} ({})",
+                    arg_list,
+                    self.describe_num_args())
+        };
+
+        self.ecx.span_err(self.fmtsp, &msg[..]);
+    }
+
     /// 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 {
@@ -691,6 +719,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();
@@ -736,6 +765,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();
+    }
+
     // 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![];