Skip to content

Commit

Permalink
Emit proper errors when on missing closure braces
Browse files Browse the repository at this point in the history
This commit focuses on emitting clean errors for the following syntax
error:

```
Some(42).map(|a|
    dbg!(a);
    a
);
```

Previous implementation tried to recover after parsing the closure body
(the `dbg` expression) by replacing the next `;` with a `,`, which made
the next expression belong to the next function argument. As such, the
following errors were emitted (among others):
  - the semicolon token was not expected,
  - a is not in scope,
  - Option::map is supposed to take one argument, not two.

This commit allows us to gracefully handle this situation by adding
giving the parser the ability to remember when it has just parsed a
closure body inside a function call. When this happens, we can treat the
unexpected `;` specifically and try to parse as much statements as
possible in order to eat the whole block. When we can't parse statements
anymore, we generate a clean error indicating that the braces are
missing, and return an ExprKind::Err.
  • Loading branch information
Sasha Pourcelot committed Aug 31, 2021
1 parent 0a84708 commit 1f5568d
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 5 deletions.
15 changes: 15 additions & 0 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use super::{AttrWrapper, BlockMode, ForceCollect, Parser, PathStyle, Restriction
use super::{SemiColonMode, SeqSep, TokenExpectType, TrailingToken};
use crate::maybe_recover_from_interpolated_ty_qpath;

use ast::token::DelimToken;
use rustc_ast::ptr::P;
use rustc_ast::token::{self, Token, TokenKind};
use rustc_ast::tokenstream::Spacing;
Expand Down Expand Up @@ -91,6 +92,8 @@ impl<'a> Parser<'a> {
/// Parses an expression.
#[inline]
pub fn parse_expr(&mut self) -> PResult<'a, P<Expr>> {
self.last_closure_body.take();

self.parse_expr_res(Restrictions::empty(), None)
}

Expand Down Expand Up @@ -1733,6 +1736,18 @@ impl<'a> Parser<'a> {
self.sess.gated_spans.gate(sym::async_closure, span);
}

// Disable recovery for closure body
self.last_closure_body = Some(decl_hi);

if self.token.kind == TokenKind::Semi && self.token_cursor.frame.delim == DelimToken::Paren
{
// It is likely that the closure body is a block but where the
// braces have been removed. We will recover and eat the next
// statements later in the parsing process.

return Ok(self.mk_expr_err(lo.to(body.span)));
}

Ok(self.mk_expr(
lo.to(body.span),
ExprKind::Closure(capture_clause, asyncness, movability, decl, body, lo.to(decl_hi)),
Expand Down
81 changes: 76 additions & 5 deletions compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ pub struct Parser<'a> {
/// If present, this `Parser` is not parsing Rust code but rather a macro call.
subparser_name: Option<&'static str>,
capture_state: CaptureState,

/// This allows us to recover when the user forget to add braces around
/// multiple statements in the closure body.
pub last_closure_body: Option<Span /* The closing `|` of the closure declarator. */>,
}

/// Indicates a range of tokens that should be replaced by
Expand Down Expand Up @@ -440,6 +444,7 @@ impl<'a> Parser<'a> {
replace_ranges: Vec::new(),
inner_attr_ranges: Default::default(),
},
last_closure_body: None,
};

// Make parser point to the first token.
Expand Down Expand Up @@ -761,20 +766,44 @@ impl<'a> Parser<'a> {
first = false;
} else {
match self.expect(t) {
Ok(false) => {}
Ok(false) => {
self.last_closure_body.take();
}
Ok(true) => {
self.last_closure_body.take();
recovered = true;
break;
}
Err(mut expect_err) => {
let sp = self.prev_token.span.shrink_to_hi();
let token_str = pprust::token_kind_to_string(t);

// Attempt to keep parsing if it was a similar separator.
if let Some(ref tokens) = t.similar_tokens() {
if tokens.contains(&self.token.kind) && !unclosed_delims {
self.bump();
match self.last_closure_body.take() {
None => {
// Attempt to keep parsing if it was a similar separator.
if let Some(ref tokens) = t.similar_tokens() {
if tokens.contains(&self.token.kind) && !unclosed_delims {
self.bump();
}
}
}

Some(right_pipe_span) if self.token.kind == TokenKind::Semi => {
// Finding a semicolon instead of a comma
// after a closure body indicates that the
// closure body may be a block but the user
// forgot to put braces around its
// statements.

self.recover_missing_braces_around_closure_body(
right_pipe_span,
expect_err,
)?;

continue;
}

_ => {}
}

// If this was a missing `@` in a binding pattern
Expand Down Expand Up @@ -839,6 +868,48 @@ impl<'a> Parser<'a> {
Ok((v, trailing, recovered))
}

fn recover_missing_braces_around_closure_body(
&mut self,
right_pipe_span: Span,
mut expect_err: DiagnosticBuilder<'_>,
) -> PResult<'a, ()> {
let initial_semicolon = self.token.span;

expect_err.span_help(
initial_semicolon,
"This `;` turns the expression into a statement, which must be placed in a block",
);

while self.eat(&TokenKind::Semi) {
let _ = self.parse_stmt(ForceCollect::Yes)?;
}

expect_err.set_primary_message(
"closure body that contain statements must be surrounded by braces",
);

let preceding_pipe_span = right_pipe_span;
let following_token_span = self.token.span;

expect_err.set_span(vec![preceding_pipe_span, following_token_span]);

let opening_suggestion_str = " {".to_string();
let closing_suggestion_str = "}".to_string();

expect_err.multipart_suggestion(
"try adding braces",
vec![
(preceding_pipe_span.shrink_to_hi(), opening_suggestion_str),
(following_token_span.shrink_to_lo(), closing_suggestion_str),
],
Applicability::MaybeIncorrect,
);

expect_err.emit();

Ok(())
}

/// Parses a sequence, not including the closing delimiter. The function
/// `f` must consume tokens until reaching the next separator or
/// closing bracket.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// This snippet ensures that no attempt to recover on a semicolon instead of
// comma is made next to a closure body.
//
// If this recovery happens, then plenty of errors are emitted. Here, we expect
// only one error.
//
// This is part of issue #88065:
// https://github.com/rust-lang/rust/issues/88065

// run-rustfix

fn main() {
let num = 5;
(1..num).reduce(|a, b| {
//~^ ERROR: closure body that contain statements must be surrounded by braces
println!("{}", a);
a * b
}).unwrap();
}
19 changes: 19 additions & 0 deletions src/test/ui/expr/malformed_closure/missing_braces_around_block.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// This snippet ensures that no attempt to recover on a semicolon instead of
// comma is made next to a closure body.
//
// If this recovery happens, then plenty of errors are emitted. Here, we expect
// only one error.
//
// This is part of issue #88065:
// https://github.com/rust-lang/rust/issues/88065

// run-rustfix

fn main() {
let num = 5;
(1..num).reduce(|a, b|
//~^ ERROR: closure body that contain statements must be surrounded by braces
println!("{}", a);
a * b
).unwrap();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
error: closure body that contain statements must be surrounded by braces
--> $DIR/missing_braces_around_block.rs:14:26
|
LL | (1..num).reduce(|a, b|
| ^
...
LL | ).unwrap();
| ^
|
help: This `;` turns the expression into a statement, which must be placed in a block
--> $DIR/missing_braces_around_block.rs:16:26
|
LL | println!("{}", a);
| ^
help: try adding braces
|
LL ~ (1..num).reduce(|a, b| {
LL |
LL | println!("{}", a);
LL | a * b
LL ~ }).unwrap();
|

error: aborting due to previous error

0 comments on commit 1f5568d

Please sign in to comment.