Skip to content

Commit

Permalink
Point at internal span in format string
Browse files Browse the repository at this point in the history
  • Loading branch information
Esteban Küber committed Jul 23, 2018
1 parent d3b3bc5 commit 38abca8
Show file tree
Hide file tree
Showing 5 changed files with 224 additions and 18 deletions.
17 changes: 13 additions & 4 deletions src/libfmt_macros/lib.rs
View file
Expand Up @@ -154,6 +154,7 @@ pub struct Parser<'a> {
style: Option<usize>,
/// How many newlines have been seen in the string so far, to adjust the error spans
seen_newlines: usize,
pub arg_places: Vec<(usize, usize)>,
}

impl<'a> Iterator for Parser<'a> {
Expand All @@ -168,9 +169,13 @@ impl<'a> Iterator for Parser<'a> {
if self.consume('{') {
Some(String(self.string(pos + 1)))
} else {
let ret = Some(NextArgument(self.argument()));
self.must_consume('}');
ret
let mut arg = self.argument();
if let Some(arg_pos) = self.must_consume('}').map(|end| {
(pos + raw + 1, end + raw + 2)
}) {
self.arg_places.push(arg_pos);
}
Some(NextArgument(arg))
}
}
'}' => {
Expand Down Expand Up @@ -211,6 +216,7 @@ impl<'a> Parser<'a> {
curarg: 0,
style,
seen_newlines: 0,
arg_places: vec![],
}
}

Expand Down Expand Up @@ -271,20 +277,22 @@ impl<'a> Parser<'a> {

/// Forces consumption of the specified character. If the character is not
/// found, an error is emitted.
fn must_consume(&mut self, c: char) {
fn must_consume(&mut self, c: char) -> Option<usize> {
self.ws();
let raw = self.style.unwrap_or(0);

let padding = raw + self.seen_newlines;
if let Some(&(pos, maybe)) = self.cur.peek() {
if c == maybe {
self.cur.next();
Some(pos)
} else {
let pos = pos + padding + 1;
self.err(format!("expected `{:?}`, found `{:?}`", c, maybe),
format!("expected `{}`", c),
pos,
pos);
None
}
} else {
let msg = format!("expected `{:?}` but string was terminated", c);
Expand All @@ -302,6 +310,7 @@ impl<'a> Parser<'a> {
} else {
self.err(msg, format!("expected `{:?}`", c), pos, pos);
}
None
}
}

Expand Down
34 changes: 22 additions & 12 deletions src/libsyntax_ext/format.rs
View file
Expand Up @@ -21,7 +21,7 @@ use syntax::feature_gate;
use syntax::parse::token;
use syntax::ptr::P;
use syntax::symbol::Symbol;
use syntax_pos::{Span, DUMMY_SP};
use syntax_pos::{Span, MultiSpan, DUMMY_SP};
use syntax::tokenstream;

use std::collections::{HashMap, HashSet};
Expand Down Expand Up @@ -264,28 +264,38 @@ impl<'a, 'b> Context<'a, 'b> {
/// errors for the case where all arguments are positional and for when
/// there are named arguments or numbered positional arguments in the
/// format string.
fn report_invalid_references(&self, numbered_position_args: bool) {
fn report_invalid_references(&self, numbered_position_args: bool, arg_places: &[(usize, usize)]) {
let mut e;
let mut refs: Vec<String> = self.invalid_refs
.iter()
.map(|r| r.to_string())
.collect();
let sps = arg_places.iter()
.map(|&(start, end)| self.fmtsp.from_inner_byte_pos(start, end))
.collect::<Vec<_>>();
let sp = MultiSpan::from_spans(sps);
let mut refs: Vec<_> = self.invalid_refs
.iter()
.map(|r| r.to_string())
.collect();

if self.names.is_empty() && !numbered_position_args {
e = self.ecx.mut_span_err(self.fmtsp,
e = self.ecx.mut_span_err(sp,
&format!("{} positional argument{} in format string, but {}",
self.pieces.len(),
if self.pieces.len() > 1 { "s" } else { "" },
self.describe_num_args()));
} else {
let arg_list = match refs.len() {
1 => format!("argument {}", refs.pop().unwrap()),
_ => format!("arguments {head} and {tail}",
tail=refs.pop().unwrap(),
1 => {
let reg = refs.pop().unwrap();
format!("argument {}", reg)
},
_ => {
let reg = refs.pop().unwrap();
format!("arguments {head} and {tail}",
tail=reg,
head=refs.join(", "))
}
};

e = self.ecx.mut_span_err(self.fmtsp,
e = self.ecx.mut_span_err(sp,
&format!("invalid reference to positional {} ({})",
arg_list,
self.describe_num_args()));
Expand Down Expand Up @@ -835,7 +845,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
}

if cx.invalid_refs.len() >= 1 {
cx.report_invalid_references(numbered_position_args);
cx.report_invalid_references(numbered_position_args, &parser.arg_places);
}

// Make sure that all arguments were used and all arguments have types.
Expand Down
0 src/test/compile-fail/ifmt-bad-arg.rs → src/test/ui/ifmt-bad-arg.rs
View file
File renamed without changes.
187 changes: 187 additions & 0 deletions src/test/ui/ifmt-bad-arg.stderr
View file
@@ -0,0 +1,187 @@
error: 1 positional argument in format string, but no arguments were given
--> $DIR/ifmt-bad-arg.rs:16:14
|
LL | format!("{}");
| ^^

error: invalid reference to positional argument 1 (there is 1 argument)
--> $DIR/ifmt-bad-arg.rs:19:14
|
LL | format!("{1}", 1);
| ^^^
|
= note: positional arguments are zero-based

error: argument never used
--> $DIR/ifmt-bad-arg.rs:19:20
|
LL | format!("{1}", 1);
| ^

error: 2 positional arguments in format string, but no arguments were given
--> $DIR/ifmt-bad-arg.rs:23:14
|
LL | format!("{} {}");
| ^^ ^^

error: invalid reference to positional argument 1 (there is 1 argument)
--> $DIR/ifmt-bad-arg.rs:26:14
|
LL | format!("{0} {1}", 1);
| ^^^ ^^^
|
= note: positional arguments are zero-based

error: invalid reference to positional argument 2 (there are 2 arguments)
--> $DIR/ifmt-bad-arg.rs:29:14
|
LL | format!("{0} {1} {2}", 1, 2);
| ^^^ ^^^ ^^^
|
= note: positional arguments are zero-based

error: invalid reference to positional argument 2 (there are 2 arguments)
--> $DIR/ifmt-bad-arg.rs:32:14
|
LL | format!("{} {value} {} {}", 1, value=2);
| ^^ ^^^^^^^ ^^ ^^
|
= note: positional arguments are zero-based

error: invalid reference to positional arguments 3, 4 and 5 (there are 3 arguments)
--> $DIR/ifmt-bad-arg.rs:34:14
|
LL | format!("{name} {value} {} {} {} {} {} {}", 0, name=1, value=2);
| ^^^^^^ ^^^^^^^ ^^ ^^ ^^ ^^ ^^ ^^
|
= note: positional arguments are zero-based

error: there is no argument named `foo`
--> $DIR/ifmt-bad-arg.rs:37:13
|
LL | format!("{} {foo} {} {bar} {}", 1, 2, 3);
| ^^^^^^^^^^^^^^^^^^^^^^

error: there is no argument named `bar`
--> $DIR/ifmt-bad-arg.rs:37:13
|
LL | format!("{} {foo} {} {bar} {}", 1, 2, 3);
| ^^^^^^^^^^^^^^^^^^^^^^

error: there is no argument named `foo`
--> $DIR/ifmt-bad-arg.rs:41:13
|
LL | format!("{foo}"); //~ ERROR: no argument named `foo`
| ^^^^^^^

error: multiple unused formatting arguments
--> $DIR/ifmt-bad-arg.rs:42:17
|
LL | format!("", 1, 2); //~ ERROR: multiple unused formatting arguments
| -- ^ ^
| |
| multiple missing formatting arguments

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

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

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

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

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

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

error: duplicate argument named `foo`
--> $DIR/ifmt-bad-arg.rs:50:33
|
LL | format!("{foo}", foo=1, foo=2); //~ ERROR: duplicate argument
| ^
|
note: previously here
--> $DIR/ifmt-bad-arg.rs:50:26
|
LL | format!("{foo}", foo=1, foo=2); //~ ERROR: duplicate argument
| ^

error: expected ident, positional arguments cannot follow named arguments
--> $DIR/ifmt-bad-arg.rs:51:24
|
LL | format!("", foo=1, 2); //~ ERROR: positional arguments cannot follow
| ^

error: there is no argument named `valueb`
--> $DIR/ifmt-bad-arg.rs:55:13
|
LL | format!("{valuea} {valueb}", valuea=5, valuec=7);
| ^^^^^^^^^^^^^^^^^^^

error: named argument never used
--> $DIR/ifmt-bad-arg.rs:55:51
|
LL | format!("{valuea} {valueb}", valuea=5, valuec=7);
| ^

error: invalid format string: expected `'}'` but string was terminated
--> $DIR/ifmt-bad-arg.rs:61:15
|
LL | format!("{"); //~ ERROR: expected `'}'` but string was terminated
| ^ expected `'}'` in format string
|
= note: if you intended to print `{`, you can escape it using `{{`

error: invalid format string: unmatched `}` found
--> $DIR/ifmt-bad-arg.rs:63:18
|
LL | format!("foo } bar"); //~ ERROR: unmatched `}` found
| ^ unmatched `}` in format string
|
= note: if you intended to print `}`, you can escape it using `}}`

error: invalid format string: unmatched `}` found
--> $DIR/ifmt-bad-arg.rs:64:18
|
LL | format!("foo }"); //~ ERROR: unmatched `}` found
| ^ unmatched `}` in format string
|
= note: if you intended to print `}`, you can escape it using `}}`

error: argument never used
--> $DIR/ifmt-bad-arg.rs:66:27
|
LL | format!("foo %s baz", "bar"); //~ ERROR: argument never used
| ^^^^^
|
= help: `%s` should be written as `{}`
= note: printf formatting not supported; see the documentation for `std::fmt`

error: aborting due to 26 previous errors

4 changes: 2 additions & 2 deletions src/test/ui/macros/macro-backtrace-println.stderr
View file
@@ -1,8 +1,8 @@
error: 1 positional argument in format string, but no arguments were given
--> $DIR/macro-backtrace-println.rs:24:30
--> $DIR/macro-backtrace-println.rs:24:31
|
LL | ($fmt:expr) => (myprint!(concat!($fmt, "/n"))); //~ ERROR no arguments were given
| ^^^^^^^^^^^^^^^^^^^
| ^^
...
LL | myprintln!("{}");
| ----------------- in this macro invocation
Expand Down

0 comments on commit 38abca8

Please sign in to comment.