Skip to content

[format_args!] Warn on whitespace before the } in a format specifier #71158

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

[format_args!] Warn on whitespace before the } in a format specifier #71158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f4a744
Try setting as error for a crater run
strega-nil Apr 26, 2020
0b1dad7
remove ws()
strega-nil Apr 27, 2020
9a7b12e
format
strega-nil Apr 27, 2020
db7c3d9
fix stderr output?
strega-nil Apr 28, 2020
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
95 changes: 23 additions & 72 deletions src/libfmt_macros/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ impl<'a> Iterator for Parser<'a> {
Some(String(self.string(pos + 1)))
} else {
let arg = self.argument();
if let Some(end) = self.must_consume('}') {
if let Some(end) = self.consume_close_brace() {
let start = self.to_span_index(pos);
let end = self.to_span_index(end + 1);
self.arg_places.push(start.to(end));
Expand Down Expand Up @@ -250,24 +250,6 @@ 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<S1: Into<string::String>, S2: Into<string::String>>(
&mut self,
description: S1,
label: S2,
span: InnerSpan,
) {
self.errors.push(ParseError {
description: description.into(),
note: None,
label: label.into(),
span,
secondary_label: None,
});
}

/// 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.
Expand Down Expand Up @@ -329,30 +311,21 @@ impl<'a> Parser<'a> {
InnerOffset(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();

/// Consume the final `}` in a format specifier. If it is not found, an error is emitted.
fn consume_close_brace(&mut self) -> Option<usize> {
if let Some(&(pos, maybe)) = self.cur.peek() {
if c == maybe {
if maybe == '}' {
self.cur.next();
Some(pos)
} else {
let pos = self.to_span_index(pos);
let description = format!("expected `'}}'`, found `{:?}`", maybe);
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
.map(|sp| ("because of this opening brace".to_owned(), sp)),
)
} else {
(None, None)
};
let note =
Some("if you intended to print `{`, you can escape it using `{{`".to_owned());
let secondary_label = self
.last_opening_brace
.map(|sp| ("because of this opening brace".to_owned(), sp));
self.errors.push(ParseError {
description,
note,
Expand All @@ -363,48 +336,26 @@ impl<'a> Parser<'a> {
None
}
} else {
let description = format!("expected `{:?}` but string was terminated", c);
let description = "expected `}` but string was terminated".to_owned();
// point at closing `"`
let pos = self.input.len() - if self.append_newline { 1 } else { 0 };
let pos = self.to_span_index(pos);
if c == '}' {
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
.map(|sp| ("because of this opening brace".to_owned(), sp)),
)
} else {
(None, None)
};
self.errors.push(ParseError {
description,
note,
label,
span: pos.to(pos),
secondary_label,
});
} else {
self.err(description, format!("expected `{:?}`", c), pos.to(pos));
}
let label = "expected `}`".to_owned();
let note =
Some("if you intended to print `{`, you can escape it using `{{`".to_owned());
let secondary_label =
self.last_opening_brace.map(|sp| ("because of this opening brace".to_owned(), sp));
self.errors.push(ParseError {
description,
note,
label,
span: pos.to(pos),
secondary_label,
});
None
}
}

/// 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() {
self.cur.next();
} else {
break;
}
}
}

/// Parses all of a string which is to be considered a "raw literal" in a
/// format string. This is everything outside of the braces.
fn string(&mut self, start: usize) -> &'a str {
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/issues/issue-51848.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: invalid format string: expected `'}'` but string was terminated
error: invalid format string: expected `}` but string was terminated
--> $DIR/issue-51848.rs:6:20
|
LL | println!("{");
| -^ expected `'}'` in format string
| -^ expected `}` in format string
| |
| because of this opening brace
...
Expand Down