Skip to content

Commit

Permalink
Use string literal directly when available in format
Browse files Browse the repository at this point in the history
Previous implementation used the `Parser::parse_expr` function in order
to extract the format expression. If the first comma following the
format expression was mistakenly replaced with a dot, then the next
format expression was eaten by the function, because it looked as a
syntactically valid expression, which resulted in incorrectly spanned
error messages.

The way the format expression is exctracted is changed: we first look at
the first available token in the first argument supplied to the
`format!` macro call. If it is a string literal, then it is promoted as
a format expression immediatly, otherwise we fall back to the original
`parse_expr`-related method.

This allows us to ensure that the parser won't consume too much tokens
when a typo is made.

A test has been created so that it is ensured that the issue is properly
fixed.
  • Loading branch information
scrabsha committed Aug 30, 2020
1 parent 85fbf49 commit f6d18db
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 2 deletions.
21 changes: 20 additions & 1 deletion compiler/rustc_builtin_macros/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,26 @@ fn parse_args<'a>(
return Err(ecx.struct_span_err(sp, "requires at least a format string argument"));
}

let fmtstr = p.parse_expr()?;
let first_token = &p.token;
let fmtstr = match first_token.kind {
token::TokenKind::Literal(token::Lit {
kind: token::LitKind::Str | token::LitKind::StrRaw(_),
..
}) => {
// If the first token is a string literal, then a format expression
// is constructed from it.
//
// This allows us to properly handle cases when the first comma
// after the format string is mistakenly replaced with any operator,
// which cause the expression parser to eat too much tokens.
p.parse_literal_maybe_minus()?
}
_ => {
// Otherwise, we fall back to the expression parser.
p.parse_expr()?
}
};

let mut first = true;
let mut named = false;

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1480,7 +1480,7 @@ impl<'a> Parser<'a> {

/// Matches `'-' lit | lit` (cf. `ast_validation::AstValidator::check_expr_within_pat`).
/// Keep this in sync with `Token::can_begin_literal_maybe_minus`.
pub(super) fn parse_literal_maybe_minus(&mut self) -> PResult<'a, P<Expr>> {
pub fn parse_literal_maybe_minus(&mut self) -> PResult<'a, P<Expr>> {
maybe_whole_expr!(self);

let lo = self.token.span;
Expand Down
22 changes: 22 additions & 0 deletions src/test/ui/fmt/incorrect-first-separator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Allows to track issue #75492:
// https://github.com/rust-lang/rust/issues/75492

use std::iter;

fn main() {
format!("A number: {}". iter::once(42).next().unwrap());
//~^ ERROR expected token: `,`

// Other kind of types are also checked:

format!("A number: {}" / iter::once(42).next().unwrap());
//~^ ERROR expected token: `,`

format!("A number: {}"; iter::once(42).next().unwrap());
//~^ ERROR expected token: `,`

// Note: this character is an COMBINING COMMA BELOW unicode char
format!("A number: {}" ̦ iter::once(42).next().unwrap());
//~^ ERROR expected token: `,`
//~^^ ERROR unknown start of token: \u{326}
}
32 changes: 32 additions & 0 deletions src/test/ui/fmt/incorrect-first-separator.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
error: unknown start of token: \u{326}
--> $DIR/incorrect-first-separator.rs:19:28
|
LL | format!("A number: {}" ̦ iter::once(42).next().unwrap());
| ^

error: expected token: `,`
--> $DIR/incorrect-first-separator.rs:7:27
|
LL | format!("A number: {}". iter::once(42).next().unwrap());
| ^ expected `,`

error: expected token: `,`
--> $DIR/incorrect-first-separator.rs:12:28
|
LL | format!("A number: {}" / iter::once(42).next().unwrap());
| ^ expected `,`

error: expected token: `,`
--> $DIR/incorrect-first-separator.rs:15:27
|
LL | format!("A number: {}"; iter::once(42).next().unwrap());
| ^ expected `,`

error: expected token: `,`
--> $DIR/incorrect-first-separator.rs:19:30
|
LL | format!("A number: {}" ̦ iter::once(42).next().unwrap());
| ^^^^ expected `,`

error: aborting due to 5 previous errors

0 comments on commit f6d18db

Please sign in to comment.