Auto merge of #62710 - estebank:bad-named-args, r=petrochenkov

Specific error for positional args after named args in `format!()` When writing positional arguments after named arguments in the `format!()` and `println!()` macros, provide a targeted diagnostic. Follow up to https://github.com/rust-lang/rust/pull/57522/files#r247278885
rust-lang · Jul 20, 2019 · f69b071 · f69b071
2 parents e9d2227 + 33ec182
commit f69b071
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 34 deletions.
diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs
@@ -146,16 +146,13 @@ fn parse_args<'a>(
         if p.token == token::Eof {
             break;
         } // accept trailing commas
-        if named || (p.token.is_ident() && p.look_ahead(1, |t| *t == token::Eq)) {
+        if p.token.is_ident() && p.look_ahead(1, |t| *t == token::Eq) {
             named = true;
             let name = if let token::Ident(name, _) = p.token.kind {
                 p.bump();
                 name
             } else {
-                return Err(ecx.struct_span_err(
-                    p.token.span,
-                    "expected ident, positional arguments cannot follow named arguments",
-                ));
+                unreachable!();
             };
 
             p.expect(&token::Eq)?;
@@ -176,6 +173,17 @@ fn parse_args<'a>(
             args.push(e);
         } else {
             let e = p.parse_expr()?;
+            if named {
+                let mut err = ecx.struct_span_err(
+                    e.span,
+                    "positional arguments cannot follow named arguments",
+                );
+                err.span_label(e.span, "positional arguments must be before named arguments");
+                for (_, pos) in &names {
+                    err.span_label(args[*pos].span, "named argument");
+                }
+                err.emit();
+            }
             args.push(e);
         }
     }
@@ -721,13 +729,14 @@ pub fn expand_format_args_nl<'cx>(
 
 /// Take the various parts of `format_args!(efmt, args..., name=names...)`
 /// and construct the appropriate formatting expression.
-pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt<'_>,
-                                    sp: Span,
-                                    efmt: P<ast::Expr>,
-                                    args: Vec<P<ast::Expr>>,
-                                    names: FxHashMap<Symbol, usize>,
-                                    append_newline: bool)
-                                    -> P<ast::Expr> {
+pub fn expand_preparsed_format_args(
+    ecx: &mut ExtCtxt<'_>,
+    sp: Span,
+    efmt: P<ast::Expr>,
+    args: Vec<P<ast::Expr>>,
+    names: FxHashMap<Symbol, usize>,
+    append_newline: bool,
+) -> P<ast::Expr> {
     // NOTE: this verbose way of initializing `Vec<Vec<ArgumentType>>` is because
     // `ArgumentType` does not derive `Clone`.
     let arg_types: Vec<_> = (0..args.len()).map(|_| Vec::new()).collect();
@@ -906,6 +915,8 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt<'_>,
         .map(|span| fmt.span.from_inner(*span))
         .collect();
 
+    let named_pos: FxHashSet<usize> = names.values().cloned().collect();
+
     let mut cx = Context {
         ecx,
         args,
@@ -971,14 +982,12 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt<'_>,
     }
 
     // Make sure that all arguments were used and all arguments have types.
-    let num_pos_args = cx.args.len() - cx.names.len();
-
     let errs = cx.arg_types
                  .iter()
                  .enumerate()
                  .filter(|(i, ty)| ty.is_empty() && !cx.count_positions.contains_key(&i))
                  .map(|(i, _)| {
-                    let msg = if i >= num_pos_args {
+                    let msg = if named_pos.contains(&i) {
                         // named argument
                         "named argument never used"
                     } else {

diff --git a/src/test/ui/if/ifmt-bad-arg.rs b/src/test/ui/if/ifmt-bad-arg.rs
@@ -38,7 +38,7 @@ fn main() {
     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
+    format!("{foo} {} {}", foo=1, 2);   //~ ERROR: positional arguments cannot follow
 
     // bad named arguments, #35082
 

diff --git a/src/test/ui/if/ifmt-bad-arg.stderr b/src/test/ui/if/ifmt-bad-arg.stderr
@@ -146,11 +146,13 @@ note: previously here
 LL |     format!("{foo}", foo=1, foo=2);
    |                          ^
 
-error: expected ident, positional arguments cannot follow named arguments
-  --> $DIR/ifmt-bad-arg.rs:41:24
+error: positional arguments cannot follow named arguments
+  --> $DIR/ifmt-bad-arg.rs:41:35
    |
-LL |     format!("", foo=1, 2);
-   |                        ^
+LL |     format!("{foo} {} {}", foo=1, 2);
+   |                                -  ^ positional arguments must be before named arguments
+   |                                |
+   |                                named argument
 
 error: there is no argument named `valueb`
   --> $DIR/ifmt-bad-arg.rs:45:23

diff --git a/src/test/ui/macros/format-parse-errors.rs b/src/test/ui/macros/format-parse-errors.rs
@@ -1,9 +1,15 @@
 fn main() {
+    let foo = "";
+    let bar = "";
     format!(); //~ ERROR requires at least a format string argument
     format!(struct); //~ ERROR expected expression
     format!("s", name =); //~ ERROR expected expression
-    format!("s", foo = foo, bar); //~ ERROR expected `=`
-    format!("s", foo = struct); //~ ERROR expected expression
+    format!(
+        "s {foo} {} {}",
+        foo = foo,
+        bar, //~ ERROR positional arguments cannot follow named arguments
+    );
+    format!("s {foo}", foo = struct); //~ ERROR expected expression
     format!("s", struct); //~ ERROR expected expression
 
     // This error should come after parsing errors to ensure they are non-fatal.

diff --git a/src/test/ui/macros/format-parse-errors.stderr b/src/test/ui/macros/format-parse-errors.stderr
@@ -1,43 +1,45 @@
 error: requires at least a format string argument
-  --> $DIR/format-parse-errors.rs:2:5
+  --> $DIR/format-parse-errors.rs:4:5
    |
 LL |     format!();
    |     ^^^^^^^^^^
    |
    = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
 
 error: expected expression, found keyword `struct`
-  --> $DIR/format-parse-errors.rs:3:13
+  --> $DIR/format-parse-errors.rs:5:13
    |
 LL |     format!(struct);
    |             ^^^^^^ expected expression
 
 error: expected expression, found end of macro arguments
-  --> $DIR/format-parse-errors.rs:4:24
+  --> $DIR/format-parse-errors.rs:6:24
    |
 LL |     format!("s", name =);
    |                        ^ expected expression
 
-error: expected `=`, found end of macro arguments
-  --> $DIR/format-parse-errors.rs:5:32
+error: positional arguments cannot follow named arguments
+  --> $DIR/format-parse-errors.rs:10:9
    |
-LL |     format!("s", foo = foo, bar);
-   |                                ^ expected `=`
+LL |         foo = foo,
+   |               --- named argument
+LL |         bar,
+   |         ^^^ positional arguments must be before named arguments
 
 error: expected expression, found keyword `struct`
-  --> $DIR/format-parse-errors.rs:6:24
+  --> $DIR/format-parse-errors.rs:12:30
    |
-LL |     format!("s", foo = struct);
-   |                        ^^^^^^ expected expression
+LL |     format!("s {foo}", foo = struct);
+   |                              ^^^^^^ expected expression
 
 error: expected expression, found keyword `struct`
-  --> $DIR/format-parse-errors.rs:7:18
+  --> $DIR/format-parse-errors.rs:13:18
    |
 LL |     format!("s", struct);
    |                  ^^^^^^ expected expression
 
 error: format argument must be a string literal
-  --> $DIR/format-parse-errors.rs:10:13
+  --> $DIR/format-parse-errors.rs:16:13
    |
 LL |     format!(123);
    |             ^^^