From a99c2da25e4830e580b9d471c07941590e699ba9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 14 Jul 2018 20:50:30 -0700 Subject: [PATCH] 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. --- src/libstd/macros.rs | 10 ++++- src/libsyntax/ext/base.rs | 25 ++++++----- src/libsyntax_ext/concat.rs | 15 +++---- src/libsyntax_ext/format.rs | 28 +++++++++--- .../conditional_array_execution.nll.stderr | 14 ++---- src/test/ui/const-eval/issue-43197.nll.stderr | 14 ++---- src/test/ui/const-eval/issue-44578.nll.stderr | 14 ++---- src/test/ui/fmt/format-string-error.rs | 2 + src/test/ui/fmt/format-string-error.stderr | 20 ++++----- src/test/ui/macros/bad_hello.rs | 5 ++- src/test/ui/macros/bad_hello.stderr | 20 ++++++--- src/test/ui/macros/format-foreign.stderr | 5 ++- .../ui/macros/format-unused-lables.stderr | 43 +++++++++---------- src/test/ui/macros/trace-macro.stderr | 9 ++-- 14 files changed, 122 insertions(+), 102 deletions(-) diff --git a/src/libstd/macros.rs b/src/libstd/macros.rs index 75f038407c127..a4a6ed73c61cf 100644 --- a/src/libstd/macros.rs +++ b/src/libstd/macros.rs @@ -155,8 +155,14 @@ 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)*)); + ($fmt:expr) => ({ + print!($fmt); + print!("\n"); + }); + ($fmt:expr, $($arg:tt)*) => ({ + print!($fmt, $($arg)*); + print!("\n"); + }); } /// Macro for printing to the standard error. diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index e49a521040fdd..953403644d2ff 100644 --- 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, err_msg: &str) - -> Option> { +pub fn expr_to_spanned_string<'a>( + cx: &'a mut ExtCtxt, + expr: P, + err_msg: &str, +) -> Result, 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, 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 index 69b4a83764e46..d58f4ce17e2f7 100644 --- 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 `&str` literals 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 index 8587d11b22786..6abf987d0ab70 100644 --- 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::>(), "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 index 8bc302a2befa4..3f82311d469ae 100644 --- 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 index 5819e6a9254a7..e25cb29c11475 100644 --- 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 index eeb152e00ea47..da040747991a4 100644 --- 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!("{}", 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!("{}", 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 index 5b13686240e7c..e48aa489f4359 100644 --- 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 index ff766ddc8fa67..4c7ef11b29ef0 100644 --- a/src/test/ui/fmt/format-string-error.stderr +++ b/src/test/ui/fmt/format-string-error.stderr @@ -1,23 +1,21 @@ 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 @@ -25,7 +23,7 @@ LL | let _ = format!("{_foo}", _foo = 6usize); = 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 @@ -33,7 +31,7 @@ LL | let _ = format!("{_}", _ = 6usize); = 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 @@ -41,7 +39,7 @@ LL | let _ = format!("{"); = 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 @@ -49,7 +47,7 @@ LL | let _ = format!("}"); = 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_hello.rs b/src/test/ui/macros/bad_hello.rs index 174dcc9b6cd3f..cccaf7998c98b 100644 --- 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 index 0bfb060f84416..87ea515182f15 100644 --- 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 index 6804ce95fa873..401b2f6d67e39 100644 --- 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 diff --git a/src/test/ui/macros/format-unused-lables.stderr b/src/test/ui/macros/format-unused-lables.stderr index 777b492dcb65f..f764190438f33 100644 --- a/src/test/ui/macros/format-unused-lables.stderr +++ b/src/test/ui/macros/format-unused-lables.stderr @@ -2,24 +2,21 @@ error: multiple unused formatting arguments --> $DIR/format-unused-lables.rs:12:22 | LL | println!("Test", 123, 456, 789); - | -----------------^^^--^^^--^^^-- multiple unused arguments in this statement - | - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + | ------ ^^^ ^^^ ^^^ + | | + | multiple missing formatting arguments error: multiple unused formatting arguments --> $DIR/format-unused-lables.rs:16:9 | -LL | / println!("Test2", -LL | | 123, //~ ERROR multiple unused formatting arguments - | | ^^^ -LL | | 456, - | | ^^^ -LL | | 789 - | | ^^^ -LL | | ); - | |______- multiple unused arguments in this statement - | - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) +LL | println!("Test2", + | ------- multiple missing formatting arguments +LL | 123, //~ ERROR multiple unused formatting arguments + | ^^^ +LL | 456, + | ^^^ +LL | 789 + | ^^^ error: named argument never used --> $DIR/format-unused-lables.rs:21:35 @@ -30,18 +27,18 @@ LL | println!("Some stuff", UNUSED="args"); //~ ERROR named argument never u error: multiple unused formatting arguments --> $DIR/format-unused-lables.rs:24:9 | -LL | / println!("Some more $STUFF", -LL | | "woo!", //~ ERROR multiple unused formatting arguments - | | ^^^^^^ -LL | | STUFF= -LL | | "things" - | | ^^^^^^^^ -LL | | , UNUSED="args"); - | |_______________________^^^^^^_- multiple unused arguments in this statement +LL | println!("Some more $STUFF", + | ------------------ multiple missing formatting arguments +LL | "woo!", //~ ERROR multiple unused formatting arguments + | ^^^^^^ +LL | STUFF= +LL | "things" + | ^^^^^^^^ +LL | , UNUSED="args"); + | ^^^^^^ | = help: `$STUFF` should be written as `{STUFF}` = note: shell 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: aborting due to 4 previous errors diff --git a/src/test/ui/macros/trace-macro.stderr b/src/test/ui/macros/trace-macro.stderr index 938f876872f2f..c98e42ea647d0 100644 --- a/src/test/ui/macros/trace-macro.stderr +++ b/src/test/ui/macros/trace-macro.stderr @@ -5,8 +5,9 @@ LL | println!("Hello, World!"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: expanding `println! { "Hello, World!" }` - = note: to `print ! ( concat ! ( "Hello, World!" , "/n" ) )` - = note: expanding `print! { concat ! ( "Hello, World!" , "/n" ) }` - = note: to `$crate :: io :: _print ( format_args ! ( concat ! ( "Hello, World!" , "/n" ) ) - )` + = note: to `{ print ! ( "Hello, World!" ) ; print ! ( "/n" ) ; }` + = note: expanding `print! { "Hello, World!" }` + = note: to `$crate :: io :: _print ( format_args ! ( "Hello, World!" ) )` + = note: expanding `print! { "/n" }` + = note: to `$crate :: io :: _print ( format_args ! ( "/n" ) )`