Skip to content

Commit

Permalink
Auto merge of #57140 - estebank:str-err, r=varkor
Browse files Browse the repository at this point in the history
Tweaks to format string diagnostics

Add label spans and fix incorrect spans.

Fix #55155, fix #55350.
  • Loading branch information
bors committed Dec 29, 2018
2 parents 43d26b1 + 5e952e3 commit 35a64f8
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 37 deletions.
7 changes: 3 additions & 4 deletions src/libfmt_macros/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ pub struct Parser<'a> {
/// `Some(raw count)` when the string is "raw", used to position spans correctly
style: Option<usize>,
/// Start and end byte offset of every successfully parsed argument
pub arg_places: Vec<(usize, usize)>,
pub arg_places: Vec<(SpanIndex, SpanIndex)>,
/// Characters that need to be shifted
skips: Vec<usize>,
/// Span offset of the last opening brace seen, used for error reporting
Expand All @@ -154,7 +154,7 @@ pub struct Parser<'a> {
}

#[derive(Clone, Copy, Debug)]
pub struct SpanIndex(usize);
pub struct SpanIndex(pub usize);

impl SpanIndex {
pub fn unwrap(self) -> usize {
Expand All @@ -166,7 +166,6 @@ impl<'a> Iterator for Parser<'a> {
type Item = Piece<'a>;

fn next(&mut self) -> Option<Piece<'a>> {
let raw = self.raw();
if let Some(&(pos, c)) = self.cur.peek() {
match c {
'{' => {
Expand All @@ -180,7 +179,7 @@ impl<'a> Iterator for Parser<'a> {
} else {
let arg = self.argument();
if let Some(arg_pos) = self.must_consume('}').map(|end| {
(pos + raw + 1, end + raw + 2)
(self.to_span_index(pos), self.to_span_index(end + 1))
}) {
self.arg_places.push(arg_pos);
}
Expand Down
11 changes: 9 additions & 2 deletions src/libsyntax_ext/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,9 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
}

let arg_spans = parser.arg_places.iter()
.map(|&(start, end)| fmt.span.from_inner_byte_pos(start, end))
.map(|&(parse::SpanIndex(start), parse::SpanIndex(end))| {
fmt.span.from_inner_byte_pos(start, end)
})
.collect();

let mut cx = Context {
Expand Down Expand Up @@ -954,13 +956,18 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
let mut diag = {
if errs_len == 1 {
let (sp, msg) = errs.into_iter().next().unwrap();
cx.ecx.struct_span_err(sp, msg)
let mut diag = cx.ecx.struct_span_err(sp, msg);
diag.span_label(sp, msg);
diag
} else {
let mut diag = cx.ecx.struct_span_err(
errs.iter().map(|&(sp, _)| sp).collect::<Vec<Span>>(),
"multiple unused formatting arguments",
);
diag.span_label(cx.fmtsp, "multiple missing formatting specifiers");
for (sp, msg) in errs {
diag.span_label(sp, msg);
}
diag
}
};
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/fmt/format-string-error-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,6 @@ raw { \n
asdf}
", asdf=1);
//~^^ ERROR invalid format string
println!("\t{}");
//~^ ERROR 1 positional argument in format string
}
8 changes: 7 additions & 1 deletion src/test/ui/fmt/format-string-error-2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -133,5 +133,11 @@ LL | asdf}
|
= note: if you intended to print `{`, you can escape it using `{{`

error: aborting due to 13 previous errors
error: 1 positional argument in format string, but no arguments were given
--> $DIR/format-string-error-2.rs:70:17
|
LL | println!("/t{}");
| ^^

error: aborting due to 14 previous errors

26 changes: 14 additions & 12 deletions src/test/ui/if/ifmt-bad-arg.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ error: argument never used
--> $DIR/ifmt-bad-arg.rs:9:20
|
LL | format!("{1}", 1);
| ----- ^
| ----- ^ argument never used
| |
| formatting specifier missing

Expand Down Expand Up @@ -80,56 +80,58 @@ error: multiple unused formatting arguments
--> $DIR/ifmt-bad-arg.rs:32:17
|
LL | format!("", 1, 2); //~ ERROR: multiple unused formatting arguments
| -- ^ ^
| |
| -- ^ ^ argument never used
| | |
| | argument never used
| multiple missing formatting specifiers

error: argument never used
--> $DIR/ifmt-bad-arg.rs:33:22
|
LL | format!("{}", 1, 2); //~ ERROR: argument never used
| ---- ^
| ---- ^ argument never used
| |
| formatting specifier missing

error: argument never used
--> $DIR/ifmt-bad-arg.rs:34:20
|
LL | format!("{1}", 1, 2); //~ ERROR: argument never used
| ----- ^
| ----- ^ argument never used
| |
| formatting specifier missing

error: named argument never used
--> $DIR/ifmt-bad-arg.rs:35:26
|
LL | format!("{}", 1, foo=2); //~ ERROR: named argument never used
| ---- ^
| ---- ^ named argument never used
| |
| formatting specifier missing

error: argument never used
--> $DIR/ifmt-bad-arg.rs:36:22
|
LL | format!("{foo}", 1, foo=2); //~ ERROR: argument never used
| ------- ^
| ------- ^ argument never used
| |
| formatting specifier missing

error: named argument never used
--> $DIR/ifmt-bad-arg.rs:37:21
|
LL | format!("", foo=2); //~ ERROR: named argument never used
| -- ^
| -- ^ named argument never used
| |
| formatting specifier missing

error: multiple unused formatting arguments
--> $DIR/ifmt-bad-arg.rs:38:32
|
LL | format!("{} {}", 1, 2, foo=1, bar=2); //~ ERROR: multiple unused formatting arguments
| ------- ^ ^
| |
| ------- ^ ^ named argument never used
| | |
| | named argument never used
| multiple missing formatting specifiers

error: duplicate argument named `foo`
Expand Down Expand Up @@ -160,7 +162,7 @@ error: named argument never used
--> $DIR/ifmt-bad-arg.rs:45:51
|
LL | format!("{valuea} {valueb}", valuea=5, valuec=7);
| ------------------- ^
| ------------------- ^ named argument never used
| |
| formatting specifier missing

Expand Down Expand Up @@ -194,7 +196,7 @@ error: argument never used
--> $DIR/ifmt-bad-arg.rs:56:27
|
LL | format!("foo %s baz", "bar"); //~ ERROR: argument never used
| -- ^^^^^
| -- ^^^^^ argument never used
| |
| help: format specifiers use curly braces: `{}`
|
Expand Down
24 changes: 15 additions & 9 deletions src/test/ui/macros/format-foreign.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ error: multiple unused formatting arguments
--> $DIR/format-foreign.rs:2:30
|
LL | println!("%.*3$s %s!/n", "Hello,", "World", 4); //~ ERROR multiple unused formatting arguments
| -------------- ^^^^^^^^ ^^^^^^^ ^
| |
| -------------- ^^^^^^^^ ^^^^^^^ ^ argument never used
| | | |
| | | argument never used
| | argument never used
| multiple missing formatting specifiers
|
= note: printf formatting not supported; see the documentation for `std::fmt`
Expand All @@ -16,7 +18,7 @@ error: argument never used
--> $DIR/format-foreign.rs:3:29
|
LL | println!("%1$*2$.*3$f", 123.456); //~ ERROR never used
| ----------- ^^^^^^^
| ----------- ^^^^^^^ argument never used
| |
| help: format specifiers use curly braces: `{0:1$.2$}`
|
Expand All @@ -29,8 +31,10 @@ LL | println!(r###"%.*3$s
| ______________-
LL | | %s!/n
LL | | "###, "Hello,", "World", 4);
| | - ^^^^^^^^ ^^^^^^^ ^
| |____|
| | - ^^^^^^^^ ^^^^^^^ ^ argument never used
| | | | |
| | | | argument never used
| |____| argument never used
| multiple missing formatting specifiers
|
= note: printf formatting not supported; see the documentation for `std::fmt`
Expand All @@ -44,15 +48,15 @@ error: argument never used
--> $DIR/format-foreign.rs:12:30
|
LL | println!("{} %f", "one", 2.0); //~ ERROR never used
| ------- ^^^
| ------- ^^^ argument never used
| |
| formatting specifier missing

error: named argument never used
--> $DIR/format-foreign.rs:14:39
|
LL | println!("Hi there, $NAME.", NAME="Tim"); //~ ERROR never used
| ----- ^^^^^
| ----- ^^^^^ named argument never used
| |
| help: format specifiers use curly braces: `{NAME}`
|
Expand All @@ -62,8 +66,10 @@ error: multiple unused formatting arguments
--> $DIR/format-foreign.rs:15:32
|
LL | println!("$1 $0 $$ $NAME", 1, 2, NAME=3);
| ---------------- ^ ^ ^
| |
| ---------------- ^ ^ ^ named argument never used
| | | |
| | | argument never used
| | argument never used
| multiple missing formatting specifiers
|
= note: shell formatting not supported; see the documentation for `std::fmt`
Expand Down
20 changes: 11 additions & 9 deletions src/test/ui/macros/format-unused-lables.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ error: multiple unused formatting arguments
--> $DIR/format-unused-lables.rs:2:22
|
LL | println!("Test", 123, 456, 789);
| ------ ^^^ ^^^ ^^^
| |
| ------ ^^^ ^^^ ^^^ argument never used
| | | |
| | | argument never used
| | argument never used
| multiple missing formatting specifiers

error: multiple unused formatting arguments
Expand All @@ -12,17 +14,17 @@ error: multiple unused formatting arguments
LL | println!("Test2",
| ------- multiple missing formatting specifiers
LL | 123, //~ ERROR multiple unused formatting arguments
| ^^^
| ^^^ argument never used
LL | 456,
| ^^^
| ^^^ argument never used
LL | 789
| ^^^
| ^^^ argument never used

error: named argument never used
--> $DIR/format-unused-lables.rs:11:35
|
LL | println!("Some stuff", UNUSED="args"); //~ ERROR named argument never used
| ------------ ^^^^^^
| ------------ ^^^^^^ named argument never used
| |
| formatting specifier missing

Expand All @@ -35,12 +37,12 @@ LL | println!("Some more $STUFF",
| | help: format specifiers use curly braces: `{STUFF}`
| multiple missing formatting specifiers
LL | "woo!", //~ ERROR multiple unused formatting arguments
| ^^^^^^
| ^^^^^^ argument never used
LL | STUFF=
LL | "things"
| ^^^^^^^^
| ^^^^^^^^ named argument never used
LL | , UNUSED="args");
| ^^^^^^
| ^^^^^^ named argument never used
|
= note: shell formatting not supported; see the documentation for `std::fmt`

Expand Down

0 comments on commit 35a64f8

Please sign in to comment.