Skip to content

Various changes to string format diagnostics #57069

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 104 additions & 40 deletions src/libfmt_macros/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,9 @@ pub struct ParseError {
pub description: string::String,
pub note: Option<string::String>,
pub label: string::String,
pub start: usize,
pub end: usize,
pub start: SpanIndex,
pub end: SpanIndex,
pub secondary_label: Option<(string::String, SpanIndex, SpanIndex)>,
}

/// The parser structure for interpreting the input format string. This is
Expand All @@ -142,22 +143,39 @@ pub struct Parser<'a> {
curarg: usize,
/// `Some(raw count)` when the string is "raw", used to position spans correctly
style: Option<usize>,
/// How many newlines have been seen in the string so far, to adjust the error spans
seen_newlines: usize,
/// Start and end byte offset of every successfully parsed argument
pub arg_places: Vec<(usize, usize)>,
/// Characters that need to be shifted
skips: Vec<usize>,
/// Span offset of the last opening brace seen, used for error reporting
last_opening_brace_pos: Option<SpanIndex>,
/// Wether the source string is comes from `println!` as opposed to `format!` or `print!`
append_newline: bool,
}

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

impl SpanIndex {
pub fn unwrap(self) -> usize {
self.0
}
}

impl<'a> Iterator for Parser<'a> {
type Item = Piece<'a>;

fn next(&mut self) -> Option<Piece<'a>> {
let raw = self.style.map(|raw| raw + self.seen_newlines).unwrap_or(0);
let raw = self.raw();
if let Some(&(pos, c)) = self.cur.peek() {
match c {
'{' => {
let curr_last_brace = self.last_opening_brace_pos;
self.last_opening_brace_pos = Some(self.to_span_index(pos));
self.cur.next();
if self.consume('{') {
self.last_opening_brace_pos = curr_last_brace;

Some(String(self.string(pos + 1)))
} else {
let arg = self.argument();
Expand All @@ -174,7 +192,7 @@ impl<'a> Iterator for Parser<'a> {
if self.consume('}') {
Some(String(self.string(pos + 1)))
} else {
let err_pos = pos + raw + 1;
let err_pos = self.to_span_index(pos);
self.err_with_note(
"unmatched `}` found",
"unmatched `}`",
Expand All @@ -186,7 +204,6 @@ impl<'a> Iterator for Parser<'a> {
}
}
'\n' => {
self.seen_newlines += 1;
Some(String(self.string(pos)))
}
_ => Some(String(self.string(pos))),
Expand All @@ -199,15 +216,22 @@ impl<'a> Iterator for Parser<'a> {

impl<'a> Parser<'a> {
/// Creates a new parser for the given format string
pub fn new(s: &'a str, style: Option<usize>) -> Parser<'a> {
pub fn new(
s: &'a str,
style: Option<usize>,
skips: Vec<usize>,
append_newline: bool,
) -> Parser<'a> {
Parser {
input: s,
cur: s.char_indices().peekable(),
errors: vec![],
curarg: 0,
style,
seen_newlines: 0,
arg_places: vec![],
skips,
last_opening_brace_pos: None,
append_newline,
}
}

Expand All @@ -218,15 +242,16 @@ impl<'a> Parser<'a> {
&mut self,
description: S1,
label: S2,
start: usize,
end: usize,
start: SpanIndex,
end: SpanIndex,
) {
self.errors.push(ParseError {
description: description.into(),
note: None,
label: label.into(),
start,
end,
secondary_label: None,
});
}

Expand All @@ -238,15 +263,16 @@ impl<'a> Parser<'a> {
description: S1,
label: S2,
note: S3,
start: usize,
end: usize,
start: SpanIndex,
end: SpanIndex,
) {
self.errors.push(ParseError {
description: description.into(),
note: Some(note.into()),
label: label.into(),
start,
end,
secondary_label: None,
});
}

Expand All @@ -266,47 +292,86 @@ impl<'a> Parser<'a> {
}
}

fn raw(&self) -> usize {
self.style.map(|raw| raw + 1).unwrap_or(0)
}

fn to_span_index(&self, pos: usize) -> SpanIndex {
let mut pos = pos;
for skip in &self.skips {
if pos > *skip {
pos += 1;
} else if pos == *skip && self.raw() == 0 {
pos += 1;
} else {
break;
}
}
SpanIndex(self.raw() + pos + 1)
}

/// Forces consumption of the specified character. If the character is not
/// found, an error is emitted.
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 + raw + 1;
self.err(format!("expected `{:?}`, found `{:?}`", c, maybe),
format!("expected `{}`", c),
pos,
pos);
let pos = self.to_span_index(pos);
let description = format!("expected `'}}'`, found `{:?}`", maybe);
let label = "expected `}`".to_owned();
let (note, secondary_label) = if c == '}' {
(Some("if you intended to print `{`, you can escape it using `{{`".to_owned()),
self.last_opening_brace_pos.map(|pos| {
("because of this opening brace".to_owned(), pos, pos)
}))
} else {
(None, None)
};
self.errors.push(ParseError {
description,
note,
label,
start: pos,
end: pos,
secondary_label,
});
None
}
} else {
let msg = format!("expected `{:?}` but string was terminated", c);
// point at closing `"`, unless the last char is `\n` to account for `println`
let pos = match self.input.chars().last() {
Some('\n') => self.input.len(),
_ => self.input.len() + 1,
};
let description = format!("expected `{:?}` but string was terminated", c);
// point at closing `"`
let pos = self.input.len() - if self.append_newline { 1 } else { 0 };
let pos = self.to_span_index(pos);
if c == '}' {
self.err_with_note(msg,
format!("expected `{:?}`", c),
"if you intended to print `{`, you can escape it using `{{`",
pos + padding,
pos + padding);
let label = format!("expected `{:?}`", c);
let (note, secondary_label) = if c == '}' {
(Some("if you intended to print `{`, you can escape it using `{{`".to_owned()),
self.last_opening_brace_pos.map(|pos| {
("because of this opening brace".to_owned(), pos, pos)
}))
} else {
(None, None)
};
self.errors.push(ParseError {
description,
note,
label,
start: pos,
end: pos,
secondary_label,
});
} else {
self.err(msg, format!("expected `{:?}`", c), pos, pos);
self.err(description, format!("expected `{:?}`", c), pos, pos);
}
None
}
}

/// Consumes all whitespace characters until the first non-whitespace
/// character
/// Consumes all whitespace characters until the first non-whitespace character
fn ws(&mut self) {
while let Some(&(_, c)) = self.cur.peek() {
if c.is_whitespace() {
Expand Down Expand Up @@ -334,8 +399,7 @@ impl<'a> Parser<'a> {
&self.input[start..self.input.len()]
}

/// Parses an Argument structure, or what's contained within braces inside
/// the format string
/// Parses an Argument structure, or what's contained within braces inside the format string
fn argument(&mut self) -> Argument<'a> {
let pos = self.position();
let format = self.format();
Expand Down Expand Up @@ -371,8 +435,8 @@ impl<'a> Parser<'a> {
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());
self.to_span_index(pos),
self.to_span_index(pos + invalid_name.len()));
Some(ArgumentNamed(invalid_name))
},

Expand Down Expand Up @@ -553,7 +617,7 @@ mod tests {
use super::*;

fn same(fmt: &'static str, p: &[Piece<'static>]) {
let parser = Parser::new(fmt, None);
let parser = Parser::new(fmt, None, vec![], false);
assert!(parser.collect::<Vec<Piece<'static>>>() == p);
}

Expand All @@ -569,7 +633,7 @@ mod tests {
}

fn musterr(s: &str) {
let mut p = Parser::new(s, None);
let mut p = Parser::new(s, None, vec![], false);
p.next();
assert!(!p.errors.is_empty());
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/traits/on_unimplemented.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedFormatString {
{
let name = tcx.item_name(trait_def_id);
let generics = tcx.generics_of(trait_def_id);
let parser = Parser::new(&self.0, None);
let parser = Parser::new(&self.0, None, vec![], false);
let mut result = Ok(());
for token in parser {
match token {
Expand Down Expand Up @@ -293,7 +293,7 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedFormatString {
}).collect::<FxHashMap<String, String>>();
let empty_string = String::new();

let parser = Parser::new(&self.0, None);
let parser = Parser::new(&self.0, None, vec![], false);
parser.map(|p|
match p {
Piece::String(s) => s,
Expand Down
Loading