Skip to content

Commit effb34d

Browse files
borsbors
committed
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.
2 parents 49f1e5d + a99c2da commit effb34d

14 files changed

+122
-102
lines changed

Diff for: ‎src/libstd/macros.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,14 @@ macro_rules! print {
155155
#[stable(feature = "rust1", since = "1.0.0")]
156156
macro_rules! println {
157157
() => (print!("\n"));
158-
($fmt:expr) => (print!(concat!($fmt, "\n")));
159-
($fmt:expr, $($arg:tt)*) => (print!(concat!($fmt, "\n"), $($arg)*));
158+
($fmt:expr) => ({
159+
print!($fmt);
160+
print!("\n");
161+
});
162+
($fmt:expr, $($arg:tt)*) => ({
163+
print!($fmt, $($arg)*);
164+
print!("\n");
165+
});
160166
}
161167

162168
/// Macro for printing to the standard error.

Diff for: ‎src/libsyntax/ext/base.rs

+15-10
Original file line numberDiff line numberDiff line change
@@ -956,29 +956,34 @@ impl<'a> ExtCtxt<'a> {
956956
/// Extract a string literal from the macro expanded version of `expr`,
957957
/// emitting `err_msg` if `expr` is not a string literal. This does not stop
958958
/// compilation on error, merely emits a non-fatal error and returns None.
959-
pub fn expr_to_spanned_string(cx: &mut ExtCtxt, expr: P<ast::Expr>, err_msg: &str)
960-
-> Option<Spanned<(Symbol, ast::StrStyle)>> {
959+
pub fn expr_to_spanned_string<'a>(
960+
cx: &'a mut ExtCtxt,
961+
expr: P<ast::Expr>,
962+
err_msg: &str,
963+
) -> Result<Spanned<(Symbol, ast::StrStyle)>, DiagnosticBuilder<'a>> {
961964
// Update `expr.span`'s ctxt now in case expr is an `include!` macro invocation.
962965
let expr = expr.map(|mut expr| {
963966
expr.span = expr.span.apply_mark(cx.current_expansion.mark);
964967
expr
965968
});
966969

967-
// we want to be able to handle e.g. concat("foo", "bar")
970+
// we want to be able to handle e.g. `concat!("foo", "bar")`
968971
let expr = cx.expander().fold_expr(expr);
969-
match expr.node {
972+
Err(match expr.node {
970973
ast::ExprKind::Lit(ref l) => match l.node {
971-
ast::LitKind::Str(s, style) => return Some(respan(expr.span, (s, style))),
972-
_ => cx.span_err(l.span, err_msg)
974+
ast::LitKind::Str(s, style) => return Ok(respan(expr.span, (s, style))),
975+
_ => cx.struct_span_err(l.span, err_msg)
973976
},
974-
_ => cx.span_err(expr.span, err_msg)
975-
}
976-
None
977+
_ => cx.struct_span_err(expr.span, err_msg)
978+
})
977979
}
978980

979981
pub fn expr_to_string(cx: &mut ExtCtxt, expr: P<ast::Expr>, err_msg: &str)
980982
-> Option<(Symbol, ast::StrStyle)> {
981-
expr_to_spanned_string(cx, expr, err_msg).map(|s| s.node)
983+
expr_to_spanned_string(cx, expr, err_msg)
984+
.map_err(|mut err| err.emit())
985+
.ok()
986+
.map(|s| s.node)
982987
}
983988

984989
/// Non-fatally assert that `tts` is empty. Note that this function

Diff for: ‎src/libsyntax_ext/concat.rs

+7-8
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ pub fn expand_syntax_ext(
2727
None => return base::DummyResult::expr(sp),
2828
};
2929
let mut accumulator = String::new();
30+
let mut missing_literal = vec![];
3031
for e in es {
3132
match e.node {
3233
ast::ExprKind::Lit(ref lit) => match lit.node {
@@ -51,17 +52,15 @@ pub fn expand_syntax_ext(
5152
}
5253
},
5354
_ => {
54-
let mut err = cx.struct_span_err(e.span, "expected a literal");
55-
let snippet = cx.codemap().span_to_snippet(e.span).unwrap();
56-
err.span_suggestion(
57-
e.span,
58-
"you might be missing a string literal to format with",
59-
format!("\"{{}}\", {}", snippet),
60-
);
61-
err.emit();
55+
missing_literal.push(e.span);
6256
}
6357
}
6458
}
59+
if missing_literal.len() > 0 {
60+
let mut err = cx.struct_span_err(missing_literal, "expected a literal");
61+
err.note("only `&str` literals can be passed to `concat!()`");
62+
err.emit();
63+
}
6564
let sp = sp.apply_mark(cx.current_expansion.mark);
6665
base::MacEager::expr(cx.expr_str(sp, Symbol::intern(&accumulator)))
6766
}

Diff for: ‎src/libsyntax_ext/format.rs

+22-6
Original file line numberDiff line numberDiff line change
@@ -703,10 +703,24 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
703703
let arg_unique_types: Vec<_> = (0..args.len()).map(|_| Vec::new()).collect();
704704
let mut macsp = ecx.call_site();
705705
macsp = macsp.apply_mark(ecx.current_expansion.mark);
706-
let msg = "format argument must be a string literal.";
706+
let msg = "format argument must be a string literal";
707+
let fmt_sp = efmt.span;
707708
let fmt = match expr_to_spanned_string(ecx, efmt, msg) {
708-
Some(fmt) => fmt,
709-
None => return DummyResult::raw_expr(sp),
709+
Ok(fmt) => fmt,
710+
Err(mut err) => {
711+
let sugg_fmt = match args.len() {
712+
0 => "{}".to_string(),
713+
_ => format!("{}{{}}", "{}, ".repeat(args.len())),
714+
715+
};
716+
err.span_suggestion(
717+
fmt_sp.shrink_to_lo(),
718+
"you might be missing a string literal to format with",
719+
format!("\"{}\", ", sugg_fmt),
720+
);
721+
err.emit();
722+
return DummyResult::raw_expr(sp);
723+
},
710724
};
711725

712726
let mut cx = Context {
@@ -818,7 +832,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
818832
errs.iter().map(|&(sp, _)| sp).collect::<Vec<Span>>(),
819833
"multiple unused formatting arguments"
820834
);
821-
diag.span_label(cx.fmtsp, "multiple unused arguments in this statement");
835+
diag.span_label(cx.fmtsp, "multiple missing formatting arguments");
822836
diag
823837
}
824838
};
@@ -861,8 +875,10 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
861875
}
862876

863877
if show_doc_note {
864-
diag.note(concat!(stringify!($kind), " formatting not supported; see \
865-
the documentation for `std::fmt`"));
878+
diag.note(concat!(
879+
stringify!($kind),
880+
" formatting not supported; see the documentation for `std::fmt`",
881+
));
866882
}
867883
}};
868884
}

Diff for: ‎src/test/ui/const-eval/conditional_array_execution.nll.stderr

+4-10
Original file line numberDiff line numberDiff line change
@@ -28,25 +28,19 @@ LL | println!("{}", FOO);
2828
| ^^^ referenced constant has errors
2929

3030
error[E0080]: referenced constant has errors
31-
--> $DIR/conditional_array_execution.rs:19:5
31+
--> $DIR/conditional_array_execution.rs:19:14
3232
|
3333
LL | const FOO: u32 = [X - Y, Y - X][(X < Y) as usize];
3434
| ----- attempt to subtract with overflow
3535
...
3636
LL | println!("{}", FOO);
37-
| ^^^^^^^^^^^^^^^^^^^^
38-
|
39-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
37+
| ^^^^
4038

4139
error[E0080]: erroneous constant used
42-
--> $DIR/conditional_array_execution.rs:19:5
40+
--> $DIR/conditional_array_execution.rs:19:14
4341
|
4442
LL | println!("{}", FOO);
45-
| ^^^^^^^^^^^^^^^---^^
46-
| |
47-
| referenced constant has errors
48-
|
49-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
43+
| ^^^^ --- referenced constant has errors
5044

5145
error[E0080]: referenced constant has errors
5246
--> $DIR/conditional_array_execution.rs:19:20

Diff for: ‎src/test/ui/const-eval/issue-43197.nll.stderr

+4-10
Original file line numberDiff line numberDiff line change
@@ -51,25 +51,19 @@ LL | println!("{} {}", X, Y);
5151
| ^ referenced constant has errors
5252

5353
error[E0080]: referenced constant has errors
54-
--> $DIR/issue-43197.rs:24:5
54+
--> $DIR/issue-43197.rs:24:14
5555
|
5656
LL | const X: u32 = 0-1;
5757
| --- attempt to subtract with overflow
5858
...
5959
LL | println!("{} {}", X, Y);
60-
| ^^^^^^^^^^^^^^^^^^^^^^^^
61-
|
62-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
60+
| ^^^^^^^
6361

6462
error[E0080]: erroneous constant used
65-
--> $DIR/issue-43197.rs:24:5
63+
--> $DIR/issue-43197.rs:24:14
6664
|
6765
LL | println!("{} {}", X, Y);
68-
| ^^^^^^^^^^^^^^^^^^-^^^^^
69-
| |
70-
| referenced constant has errors
71-
|
72-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
66+
| ^^^^^^^ - referenced constant has errors
7367

7468
error[E0080]: referenced constant has errors
7569
--> $DIR/issue-43197.rs:24:26

Diff for: ‎src/test/ui/const-eval/issue-44578.nll.stderr

+4-10
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,17 @@
11
error[E0080]: referenced constant has errors
2-
--> $DIR/issue-44578.rs:35:5
2+
--> $DIR/issue-44578.rs:35:14
33
|
44
LL | const AMT: usize = [A::AMT][(A::AMT > B::AMT) as usize];
55
| ------------------------------------ index out of bounds: the len is 1 but the index is 1
66
...
77
LL | println!("{}", <Bar<u16, u8> as Foo>::AMT);
8-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
9-
|
10-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
8+
| ^^^^
119

1210
error[E0080]: erroneous constant used
13-
--> $DIR/issue-44578.rs:35:5
11+
--> $DIR/issue-44578.rs:35:14
1412
|
1513
LL | println!("{}", <Bar<u16, u8> as Foo>::AMT);
16-
| ^^^^^^^^^^^^^^^--------------------------^^
17-
| |
18-
| referenced constant has errors
19-
|
20-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
14+
| ^^^^ -------------------------- referenced constant has errors
2115

2216
error[E0080]: referenced constant has errors
2317
--> $DIR/issue-44578.rs:35:20

Diff for: ‎src/test/ui/fmt/format-string-error.rs

+2
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@
1010

1111
fn main() {
1212
println!("{");
13+
//~^ ERROR invalid format string: expected `'}'` but string was terminated
1314
println!("{{}}");
1415
println!("}");
16+
//~^ ERROR invalid format string: unmatched `}` found
1517
let _ = format!("{_foo}", _foo = 6usize);
1618
//~^ ERROR invalid format string: invalid argument name `_foo`
1719
let _ = format!("{_}", _ = 6usize);

Diff for: ‎src/test/ui/fmt/format-string-error.stderr

+9-11
Original file line numberDiff line numberDiff line change
@@ -1,55 +1,53 @@
11
error: invalid format string: expected `'}'` but string was terminated
2-
--> $DIR/format-string-error.rs:12:5
2+
--> $DIR/format-string-error.rs:12:16
33
|
44
LL | println!("{");
5-
| ^^^^^^^^^^^^^^ expected `'}'` in format string
5+
| ^ expected `'}'` in format string
66
|
77
= note: if you intended to print `{`, you can escape it using `{{`
8-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
98

109
error: invalid format string: unmatched `}` found
11-
--> $DIR/format-string-error.rs:14:5
10+
--> $DIR/format-string-error.rs:15:15
1211
|
1312
LL | println!("}");
14-
| ^^^^^^^^^^^^^^ unmatched `}` in format string
13+
| ^ unmatched `}` in format string
1514
|
1615
= note: if you intended to print `}`, you can escape it using `}}`
17-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
1816

1917
error: invalid format string: invalid argument name `_foo`
20-
--> $DIR/format-string-error.rs:15:23
18+
--> $DIR/format-string-error.rs:17:23
2119
|
2220
LL | let _ = format!("{_foo}", _foo = 6usize);
2321
| ^^^^ invalid argument name in format string
2422
|
2523
= note: argument names cannot start with an underscore
2624

2725
error: invalid format string: invalid argument name `_`
28-
--> $DIR/format-string-error.rs:17:23
26+
--> $DIR/format-string-error.rs:19:23
2927
|
3028
LL | let _ = format!("{_}", _ = 6usize);
3129
| ^ invalid argument name in format string
3230
|
3331
= note: argument names cannot start with an underscore
3432

3533
error: invalid format string: expected `'}'` but string was terminated
36-
--> $DIR/format-string-error.rs:19:23
34+
--> $DIR/format-string-error.rs:21:23
3735
|
3836
LL | let _ = format!("{");
3937
| ^ expected `'}'` in format string
4038
|
4139
= note: if you intended to print `{`, you can escape it using `{{`
4240

4341
error: invalid format string: unmatched `}` found
44-
--> $DIR/format-string-error.rs:21:22
42+
--> $DIR/format-string-error.rs:23:22
4543
|
4644
LL | let _ = format!("}");
4745
| ^ unmatched `}` in format string
4846
|
4947
= note: if you intended to print `}`, you can escape it using `}}`
5048

5149
error: invalid format string: expected `'}'`, found `'/'`
52-
--> $DIR/format-string-error.rs:23:23
50+
--> $DIR/format-string-error.rs:25:23
5351
|
5452
LL | let _ = format!("{/}");
5553
| ^ expected `}` in format string

Diff for: ‎src/test/ui/macros/bad_hello.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,8 @@
99
// except according to those terms.
1010

1111
fn main() {
12-
println!(3 + 4); //~ ERROR expected a literal
12+
println!(3 + 4);
13+
//~^ ERROR format argument must be a string literal
14+
println!(3, 4);
15+
//~^ ERROR format argument must be a string literal
1316
}

Diff for: ‎src/test/ui/macros/bad_hello.stderr

+15-5
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,22 @@
1-
error: expected a literal
1+
error: format argument must be a string literal
22
--> $DIR/bad_hello.rs:12:14
33
|
4-
LL | println!(3 + 4); //~ ERROR expected a literal
4+
LL | println!(3 + 4);
55
| ^^^^^
66
help: you might be missing a string literal to format with
77
|
8-
LL | println!("{}", 3 + 4); //~ ERROR expected a literal
9-
| ^^^^^^^^^^^
8+
LL | println!("{}", 3 + 4);
9+
| ^^^^^
10+
11+
error: format argument must be a string literal
12+
--> $DIR/bad_hello.rs:14:14
13+
|
14+
LL | println!(3, 4);
15+
| ^
16+
help: you might be missing a string literal to format with
17+
|
18+
LL | println!("{}, {}", 3, 4);
19+
| ^^^^^^^^^
1020

11-
error: aborting due to previous error
21+
error: aborting due to 2 previous errors
1222

Diff for: ‎src/test/ui/macros/format-foreign.stderr

+3-2
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@ error: multiple unused formatting arguments
22
--> $DIR/format-foreign.rs:12:30
33
|
44
LL | println!("%.*3$s %s!/n", "Hello,", "World", 4); //~ ERROR multiple unused formatting arguments
5-
| -------------------------^^^^^^^^--^^^^^^^--^-- multiple unused arguments in this statement
5+
| -------------- ^^^^^^^^ ^^^^^^^ ^
6+
| |
7+
| multiple missing formatting arguments
68
|
79
= help: `%.*3$s` should be written as `{:.2$}`
810
= help: `%s` should be written as `{}`
911
= note: printf formatting not supported; see the documentation for `std::fmt`
10-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
1112

1213
error: argument never used
1314
--> $DIR/format-foreign.rs:13:29

0 commit comments

Comments
 (0)