Auto merge of #52394 - estebank:println, r=<try>

Improve suggestion for missing fmt str in println Avoid using `concat!(fmt, "\n")` to improve the diagnostics being emitted when the first `println!()` argument isn't a formatting string literal. Fix #52347.
rust-lang · Jul 15, 2018 · ac3b07a · ac3b07a
2 parents fb8bde0 + d9450a6
commit ac3b07a
Show file tree

Hide file tree

Showing 16 changed files with 143 additions and 104 deletions.
diff --git a/src/libstd/macros.rs b/src/libstd/macros.rs
@@ -155,8 +155,7 @@ macro_rules! print {
 #[stable(feature = "rust1", since = "1.0.0")]
 macro_rules! println {
     () => (print!("\n"));
-    ($fmt:expr) => (print!(concat!($fmt, "\n")));
-    ($fmt:expr, $($arg:tt)*) => (print!(concat!($fmt, "\n"), $($arg)*));
+    ($($arg:tt)*) => (print!("{}\n", format_args!($($arg)*)));
 }
 
 /// Macro for printing to the standard error.
@@ -212,8 +211,7 @@ macro_rules! eprint {
 #[stable(feature = "eprint", since = "1.19.0")]
 macro_rules! eprintln {
     () => (eprint!("\n"));
-    ($fmt:expr) => (eprint!(concat!($fmt, "\n")));
-    ($fmt:expr, $($arg:tt)*) => (eprint!(concat!($fmt, "\n"), $($arg)*));
+    ($($arg:tt)*) => (eprint!("{}\n", format_args!($($arg)*)));
 }
 
 #[macro_export]

diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs
@@ -956,29 +956,34 @@ impl<'a> ExtCtxt<'a> {
 /// Extract a string literal from the macro expanded version of `expr`,
 /// emitting `err_msg` if `expr` is not a string literal. This does not stop
 /// compilation on error, merely emits a non-fatal error and returns None.
-pub fn expr_to_spanned_string(cx: &mut ExtCtxt, expr: P<ast::Expr>, err_msg: &str)
-                              -> Option<Spanned<(Symbol, ast::StrStyle)>> {
+pub fn expr_to_spanned_string<'a>(
+    cx: &'a mut ExtCtxt,
+    expr: P<ast::Expr>,
+    err_msg: &str,
+) -> Result<Spanned<(Symbol, ast::StrStyle)>, DiagnosticBuilder<'a>> {
     // Update `expr.span`'s ctxt now in case expr is an `include!` macro invocation.
     let expr = expr.map(|mut expr| {
         expr.span = expr.span.apply_mark(cx.current_expansion.mark);
         expr
     });
 
-    // we want to be able to handle e.g. concat("foo", "bar")
+    // we want to be able to handle e.g. `concat!("foo", "bar")`
     let expr = cx.expander().fold_expr(expr);
-    match expr.node {
+    Err(match expr.node {
         ast::ExprKind::Lit(ref l) => match l.node {
-            ast::LitKind::Str(s, style) => return Some(respan(expr.span, (s, style))),
-            _ => cx.span_err(l.span, err_msg)
+            ast::LitKind::Str(s, style) => return Ok(respan(expr.span, (s, style))),
+            _ => cx.struct_span_err(l.span, err_msg)
         },
-        _ => cx.span_err(expr.span, err_msg)
-    }
-    None
+        _ => cx.struct_span_err(expr.span, err_msg)
+    })
 }
 
 pub fn expr_to_string(cx: &mut ExtCtxt, expr: P<ast::Expr>, err_msg: &str)
                       -> Option<(Symbol, ast::StrStyle)> {
-    expr_to_spanned_string(cx, expr, err_msg).map(|s| s.node)
+    expr_to_spanned_string(cx, expr, err_msg)
+        .map_err(|mut err| err.emit())
+        .ok()
+        .map(|s| s.node)
 }
 
 /// Non-fatally assert that `tts` is empty. Note that this function

diff --git a/src/libsyntax_ext/concat.rs b/src/libsyntax_ext/concat.rs
@@ -27,6 +27,7 @@ pub fn expand_syntax_ext(
         None => return base::DummyResult::expr(sp),
     };
     let mut accumulator = String::new();
+    let mut missing_literal = vec![];
     for e in es {
         match e.node {
             ast::ExprKind::Lit(ref lit) => match lit.node {
@@ -51,17 +52,15 @@ pub fn expand_syntax_ext(
                 }
             },
             _ => {
-                let mut err = cx.struct_span_err(e.span, "expected a literal");
-                let snippet = cx.codemap().span_to_snippet(e.span).unwrap();
-                err.span_suggestion(
-                    e.span,
-                    "you might be missing a string literal to format with",
-                    format!("\"{{}}\", {}", snippet),
-                );
-                err.emit();
+                missing_literal.push(e.span);
             }
         }
     }
+    if missing_literal.len() > 0 {
+        let mut err = cx.struct_span_err(missing_literal, "expected a literal");
+        err.note("only literals (like `\"foo\"`, `42` and `3.14`) can be passed to `concat!()`");
+        err.emit();
+    }
     let sp = sp.apply_mark(cx.current_expansion.mark);
     base::MacEager::expr(cx.expr_str(sp, Symbol::intern(&accumulator)))
 }
diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs
@@ -703,10 +703,24 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
     let arg_unique_types: Vec<_> = (0..args.len()).map(|_| Vec::new()).collect();
     let mut macsp = ecx.call_site();
     macsp = macsp.apply_mark(ecx.current_expansion.mark);
-    let msg = "format argument must be a string literal.";
+    let msg = "format argument must be a string literal";
+    let fmt_sp = efmt.span;
     let fmt = match expr_to_spanned_string(ecx, efmt, msg) {
-        Some(fmt) => fmt,
-        None => return DummyResult::raw_expr(sp),
+        Ok(fmt) => fmt,
+        Err(mut err) => {
+            let sugg_fmt = match args.len() {
+                0 => "{}".to_string(),
+                _ => format!("{}{{}}", "{}, ".repeat(args.len())),
+
+            };
+            err.span_suggestion(
+                fmt_sp.shrink_to_lo(),
+                "you might be missing a string literal to format with",
+                format!("\"{}\", ", sugg_fmt),
+            );
+            err.emit();
+            return DummyResult::raw_expr(sp);
+        },
     };
 
     let mut cx = Context {
@@ -818,7 +832,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
                     errs.iter().map(|&(sp, _)| sp).collect::<Vec<Span>>(),
                     "multiple unused formatting arguments"
                 );
-                diag.span_label(cx.fmtsp, "multiple unused arguments in this statement");
+                diag.span_label(cx.fmtsp, "multiple missing formatting arguments");
                 diag
             }
         };
@@ -861,8 +875,10 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
                     }
 
                     if show_doc_note {
-                        diag.note(concat!(stringify!($kind), " formatting not supported; see \
-                                the documentation for `std::fmt`"));
+                        diag.note(concat!(
+                            stringify!($kind),
+                            " formatting not supported; see the documentation for `std::fmt`",
+                        ));
                     }
                 }};
             }

diff --git a/src/test/ui/const-eval/conditional_array_execution.nll.stderr b/src/test/ui/const-eval/conditional_array_execution.nll.stderr
@@ -28,25 +28,19 @@ LL |     println!("{}", FOO);
    |                    ^^^ referenced constant has errors
 
 error[E0080]: referenced constant has errors
-  --> $DIR/conditional_array_execution.rs:19:5
+  --> $DIR/conditional_array_execution.rs:19:14
    |
 LL | const FOO: u32 = [X - Y, Y - X][(X < Y) as usize];
    |                   ----- attempt to subtract with overflow
 ...
 LL |     println!("{}", FOO);
-   |     ^^^^^^^^^^^^^^^^^^^^
-   |
-   = 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[E0080]: erroneous constant used
-  --> $DIR/conditional_array_execution.rs:19:5
+  --> $DIR/conditional_array_execution.rs:19:14
    |
 LL |     println!("{}", FOO);
-   |     ^^^^^^^^^^^^^^^---^^
-   |                    |
-   |                    referenced constant has errors
-   |
-   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
+   |              ^^^^  --- referenced constant has errors
 
 error[E0080]: referenced constant has errors
   --> $DIR/conditional_array_execution.rs:19:20

diff --git a/src/test/ui/const-eval/issue-43197.nll.stderr b/src/test/ui/const-eval/issue-43197.nll.stderr
@@ -51,25 +51,19 @@ LL |     println!("{} {}", X, Y);
    |                          ^ referenced constant has errors
 
 error[E0080]: referenced constant has errors
-  --> $DIR/issue-43197.rs:24:5
+  --> $DIR/issue-43197.rs:24:14
    |
 LL |     const X: u32 = 0-1;
    |                    --- attempt to subtract with overflow
 ...
 LL |     println!("{} {}", X, Y);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-   = 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[E0080]: erroneous constant used
-  --> $DIR/issue-43197.rs:24:5
+  --> $DIR/issue-43197.rs:24:14
    |
 LL |     println!("{} {}", X, Y);
-   |     ^^^^^^^^^^^^^^^^^^-^^^^^
-   |                       |
-   |                       referenced constant has errors
-   |
-   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
+   |              ^^^^^^^  - referenced constant has errors
 
 error[E0080]: referenced constant has errors
   --> $DIR/issue-43197.rs:24:26

diff --git a/src/test/ui/const-eval/issue-44578.nll.stderr b/src/test/ui/const-eval/issue-44578.nll.stderr
@@ -1,23 +1,17 @@
 error[E0080]: referenced constant has errors
-  --> $DIR/issue-44578.rs:35:5
+  --> $DIR/issue-44578.rs:35:14
    |
 LL |     const AMT: usize = [A::AMT][(A::AMT > B::AMT) as usize];
    |                        ------------------------------------ index out of bounds: the len is 1 but the index is 1
 ...
 LL |     println!("{}", <Bar<u16, u8> as Foo>::AMT);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-   = 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[E0080]: erroneous constant used
-  --> $DIR/issue-44578.rs:35:5
+  --> $DIR/issue-44578.rs:35:14
    |
 LL |     println!("{}", <Bar<u16, u8> as Foo>::AMT);
-   |     ^^^^^^^^^^^^^^^--------------------------^^
-   |                    |
-   |                    referenced constant has errors
-   |
-   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
+   |              ^^^^  -------------------------- referenced constant has errors
 
 error[E0080]: referenced constant has errors
   --> $DIR/issue-44578.rs:35:20

diff --git a/src/test/ui/fmt/format-string-error.rs b/src/test/ui/fmt/format-string-error.rs
@@ -10,8 +10,10 @@
 
 fn main() {
     println!("{");
+    //~^ ERROR invalid format string: expected `'}'` but string was terminated
     println!("{{}}");
     println!("}");
+    //~^ ERROR invalid format string: unmatched `}` found
     let _ = format!("{_foo}", _foo = 6usize);
     //~^ ERROR invalid format string: invalid argument name `_foo`
     let _ = format!("{_}", _ = 6usize);

diff --git a/src/test/ui/fmt/format-string-error.stderr b/src/test/ui/fmt/format-string-error.stderr
@@ -1,55 +1,53 @@
 error: invalid format string: expected `'}'` but string was terminated
-  --> $DIR/format-string-error.rs:12:5
+  --> $DIR/format-string-error.rs:12:16
    |
 LL |     println!("{");
-   |     ^^^^^^^^^^^^^^ expected `'}'` in format string
+   |                ^ expected `'}'` in format string
    |
    = note: if you intended to print `{`, you can escape it using `{{`
-   = 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: invalid format string: unmatched `}` found
-  --> $DIR/format-string-error.rs:14:5
+  --> $DIR/format-string-error.rs:15:15
    |
 LL |     println!("}");
-   |     ^^^^^^^^^^^^^^ unmatched `}` in format string
+   |               ^ unmatched `}` in format string
    |
    = note: if you intended to print `}`, you can escape it using `}}`
-   = 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: invalid format string: invalid argument name `_foo`
-  --> $DIR/format-string-error.rs:15:23
+  --> $DIR/format-string-error.rs:17:23
    |
 LL |     let _ = format!("{_foo}", _foo = 6usize);
    |                       ^^^^ invalid argument name in format string
    |
    = note: argument names cannot start with an underscore
 
 error: invalid format string: invalid argument name `_`
-  --> $DIR/format-string-error.rs:17:23
+  --> $DIR/format-string-error.rs:19:23
    |
 LL |     let _ = format!("{_}", _ = 6usize);
    |                       ^ invalid argument name in format string
    |
    = note: argument names cannot start with an underscore
 
 error: invalid format string: expected `'}'` but string was terminated
-  --> $DIR/format-string-error.rs:19:23
+  --> $DIR/format-string-error.rs:21:23
    |
 LL |     let _ = format!("{");
    |                       ^ expected `'}'` in format string
    |
    = note: if you intended to print `{`, you can escape it using `{{`
 
 error: invalid format string: unmatched `}` found
-  --> $DIR/format-string-error.rs:21:22
+  --> $DIR/format-string-error.rs:23:22
    |
 LL |     let _ = format!("}");
    |                      ^ unmatched `}` in format string
    |
    = note: if you intended to print `}`, you can escape it using `}}`
 
 error: invalid format string: expected `'}'`, found `'/'`
-  --> $DIR/format-string-error.rs:23:23
+  --> $DIR/format-string-error.rs:25:23
    |
 LL |     let _ = format!("{/}");
    |                       ^ expected `}` in format string

diff --git a/src/test/ui/macros/bad-concat.rs b/src/test/ui/macros/bad-concat.rs
@@ -0,0 +1,18 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+fn main() {
+    let x: u32 = 42;
+    let y: f64 = 3.14;
+    let z = "foo";
+    let _ = concat!(x, y, z, "bar");
+    //~^ ERROR expected a literal
+    //~| NOTE only literals
+}
diff --git a/src/test/ui/macros/bad-concat.stderr b/src/test/ui/macros/bad-concat.stderr
@@ -0,0 +1,10 @@
+error: expected a literal
+  --> $DIR/bad-concat.rs:15:21
+   |
+LL |     let _ = concat!(x, y, z, "bar");
+   |                     ^  ^  ^
+   |
+   = note: only literals (like `"foo"`, `42` and `3.14`) can be passed to `concat!()`
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/macros/bad_hello.rs b/src/test/ui/macros/bad_hello.rs
@@ -9,5 +9,8 @@
 // except according to those terms.
 
 fn main() {
-    println!(3 + 4); //~ ERROR expected a literal
+    println!(3 + 4);
+    //~^ ERROR format argument must be a string literal
+    println!(3, 4);
+    //~^ ERROR format argument must be a string literal
 }
diff --git a/src/test/ui/macros/bad_hello.stderr b/src/test/ui/macros/bad_hello.stderr
@@ -1,12 +1,22 @@
-error: expected a literal
+error: format argument must be a string literal
   --> $DIR/bad_hello.rs:12:14
    |
-LL |     println!(3 + 4); //~ ERROR expected a literal
+LL |     println!(3 + 4);
    |              ^^^^^
 help: you might be missing a string literal to format with
    |
-LL |     println!("{}", 3 + 4); //~ ERROR expected a literal
-   |              ^^^^^^^^^^^
+LL |     println!("{}", 3 + 4);
+   |              ^^^^^
+
+error: format argument must be a string literal
+  --> $DIR/bad_hello.rs:14:14
+   |
+LL |     println!(3, 4);
+   |              ^
+help: you might be missing a string literal to format with
+   |
+LL |     println!("{}, {}", 3, 4);
+   |              ^^^^^^^^^
 
-error: aborting due to previous error
+error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/macros/format-foreign.stderr b/src/test/ui/macros/format-foreign.stderr
@@ -2,12 +2,13 @@ error: multiple unused formatting arguments
   --> $DIR/format-foreign.rs:12:30
    |
 LL |     println!("%.*3$s %s!/n", "Hello,", "World", 4); //~ ERROR multiple unused formatting arguments
-   |     -------------------------^^^^^^^^--^^^^^^^--^-- multiple unused arguments in this statement
+   |              --------------  ^^^^^^^^  ^^^^^^^  ^
+   |              |
+   |              multiple missing formatting arguments
    |
    = help: `%.*3$s` should be written as `{:.2$}`
    = help: `%s` should be written as `{}`
    = note: printf formatting not supported; see the documentation for `std::fmt`
-   = 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: argument never used
   --> $DIR/format-foreign.rs:13:29