Skip to content

Commit

Permalink
Improve format string errors
Browse files Browse the repository at this point in the history 
 - Point at format string position inside the formatting string
 - Explain that argument names can't start with an underscore
  • Loading branch information
@estebank
estebank committed May 10, 2018
1 parent e5f80f2 commit 3f6b3bb
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 21 deletions.
82 changes: 68 additions & 14 deletions src/libfmt_macros/lib.rs
View file
Expand Up @@ -127,6 +127,14 @@ pub enum Count<'a> {
CountImplied,
}

pub struct ParseError {
pub description: string::String,
pub note: Option<string::String>,
pub label: string::String,
pub start: usize,
pub end: usize,
}

/// The parser structure for interpreting the input format string. This is
/// modeled as an iterator over `Piece` structures to form a stream of tokens
/// being output.
Expand All @@ -137,7 +145,7 @@ pub struct Parser<'a> {
input: &'a str,
cur: iter::Peekable<str::CharIndices<'a>>,
/// Error messages accumulated during parsing
pub errors: Vec<(string::String, Option<string::String>)>,
pub errors: Vec<ParseError>,
/// Current position of implicit positional argument pointer
curarg: usize,
}
Expand All @@ -160,12 +168,17 @@ impl<'a> Iterator for Parser<'a> {
}
'}' => {
self.cur.next();
let pos = pos + 1;
if self.consume('}') {
Some(String(self.string(pos + 1)))
Some(String(self.string(pos)))
} else {
self.err_with_note("unmatched `}` found",
"if you intended to print `}`, \
you can escape it using `}}`");
self.err_with_note(
"unmatched `}` found",
"unmatched `}`",
"if you intended to print `}`, you can escape it using `}}`",
pos,
pos,
);
None
}
}
Expand All @@ -191,15 +204,40 @@ impl<'a> Parser<'a> {
/// Notifies of an error. The message doesn't actually need to be of type
/// String, but I think it does when this eventually uses conditions so it
/// might as well start using it now.
fn err(&mut self, msg: &str) {
self.errors.push((msg.to_owned(), None));
fn err<S1: Into<string::String>, S2: Into<string::String>>(
&mut self,
description: S1,
label: S2,
start: usize,
end: usize,
) {
self.errors.push(ParseError {
description: description.into(),
note: None,
label: label.into(),
start,
end,
});
}

/// Notifies of an error. The message doesn't actually need to be of type
/// String, but I think it does when this eventually uses conditions so it
/// might as well start using it now.
fn err_with_note(&mut self, msg: &str, note: &str) {
self.errors.push((msg.to_owned(), Some(note.to_owned())));
fn err_with_note<S1: Into<string::String>, S2: Into<string::String>, S3: Into<string::String>>(
&mut self,
description: S1,
label: S2,
note: S3,
start: usize,
end: usize,
) {
self.errors.push(ParseError {
description: description.into(),
note: Some(note.into()),
label: label.into(),
start,
end,
});
}

/// Optionally consumes the specified character. If the character is not at
Expand All @@ -222,19 +260,26 @@ impl<'a> Parser<'a> {
/// found, an error is emitted.
fn must_consume(&mut self, c: char) {
self.ws();
if let Some(&(_, maybe)) = self.cur.peek() {
if let Some(&(pos, maybe)) = self.cur.peek() {
if c == maybe {
self.cur.next();
} else {
self.err(&format!("expected `{:?}`, found `{:?}`", c, maybe));
self.err(format!("expected `{:?}`, found `{:?}`", c, maybe),
format!("expected `{}`", c),
pos + 1,
pos + 1);
}
} else {
let msg = &format!("expected `{:?}` but string was terminated", c);
let msg = format!("expected `{:?}` but string was terminated", c);
let pos = self.input.len() + 1; // point at closing `"`
if c == '}' {
self.err_with_note(msg,
"if you intended to print `{`, you can escape it using `{{`");
format!("expected `{:?}`", c),
"if you intended to print `{`, you can escape it using `{{`",
pos,
pos);
} else {
self.err(msg);
self.err(msg, format!("expected `{:?}`", c), pos, pos);
}
}
}
Expand Down Expand Up @@ -300,6 +345,15 @@ impl<'a> Parser<'a> {
} else {
match self.cur.peek() {
Some(&(_, c)) if c.is_alphabetic() => Some(ArgumentNamed(self.word())),
Some(&(pos, c)) if c == '_' => {
let invalid_name = self.string(pos);
self.err_with_note(format!("invalid argument name `{}`", invalid_name),
"invalid argument name",
"argument names cannot start with an underscore",
pos + 1, // add 1 to account for leading `{`
pos + 1 + invalid_name.len());
Some(ArgumentNamed(invalid_name))
},

// This is an `ArgumentNext`.
// Record the fact and do the resolution after parsing the
Expand Down
9 changes: 6 additions & 3 deletions src/libsyntax_ext/format.rs
View file
Expand Up @@ -767,9 +767,12 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt,
}

if !parser.errors.is_empty() {
let (err, note) = parser.errors.remove(0);
let mut e = cx.ecx.struct_span_err(cx.fmtsp, &format!("invalid format string: {}", err));
if let Some(note) = note {
let err = parser.errors.remove(0);
let sp = cx.fmtsp.from_inner_byte_pos(err.start, err.end);
let mut e = cx.ecx.struct_span_err(sp, &format!("invalid format string: {}",
err.description));
e.span_label(sp, err.label + " in format string");
if let Some(note) = err.note {
e.note(&note);
}
e.emit();
Expand Down
7 changes: 7 additions & 0 deletions src/libsyntax_pos/lib.rs
View file
Expand Up @@ -427,6 +427,13 @@ impl Span {
)
}

pub fn from_inner_byte_pos(self, start: usize, end: usize) -> Span {
let span = self.data();
Span::new(span.lo + BytePos::from_usize(start),
span.lo + BytePos::from_usize(end),
span.ctxt)
}

#[inline]
pub fn apply_mark(self, mark: Mark) -> Span {
let span = self.data();
Expand Down
11 changes: 10 additions & 1 deletion src/test/ui/fmt/format-string-error.rs
View file
Expand Up @@ -12,5 +12,14 @@ fn main() {
println!("{");
println!("{{}}");
println!("}");
let _ = format!("{_foo}", _foo = 6usize);
//~^ ERROR invalid format string: invalid argument name `_foo`
let _ = format!("{_}", _ = 6usize);
//~^ ERROR invalid format string: invalid argument name `_`
let _ = format!("{");
//~^ ERROR invalid format string: expected `'}'` but string was terminated
let _ = format!("}");
//~^ ERROR invalid format string: unmatched `}` found
let _ = format!("{\\}");
//~^ ERROR invalid format string: expected `'}'`, found `'\\'`
}

44 changes: 41 additions & 3 deletions src/test/ui/fmt/format-string-error.stderr
View file
Expand Up @@ -2,7 +2,7 @@ error: invalid format string: expected `'}'` but string was terminated
--> $DIR/format-string-error.rs:12:5
|
LL | println!("{");
| ^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^ 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)
Expand All @@ -11,10 +11,48 @@ error: invalid format string: unmatched `}` found
--> $DIR/format-string-error.rs:14:5
|
LL | println!("}");
| ^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^ 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: aborting due to 2 previous errors
error: invalid format string: invalid argument name `_foo`
--> $DIR/format-string-error.rs:15: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
|
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
|
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
|
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
|
LL | let _ = format!("{/}");
| ^ expected `}` in format string

error: aborting due to 7 previous errors

0 comments on commit 3f6b3bb

Please sign in to comment.