Skip to content
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

Emit proper errors when on missing closure braces #88546

Merged
merged 1 commit into from
Sep 11, 2021
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
30 changes: 26 additions & 4 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
use super::pat::{RecoverColon, RecoverComma, PARAM_EXPECTED};
use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign};
use super::{AttrWrapper, BlockMode, ForceCollect, Parser, PathStyle, Restrictions, TokenType};
use super::{
AttrWrapper, BlockMode, ClosureSpans, ForceCollect, Parser, PathStyle, Restrictions, TokenType,
};
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 +94,8 @@ impl<'a> Parser<'a> {
/// Parses an expression.
#[inline]
pub fn parse_expr(&mut self) -> PResult<'a, P<Expr>> {
self.current_closure.take();

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

Expand Down Expand Up @@ -1736,7 +1741,7 @@ impl<'a> Parser<'a> {
let capture_clause = self.parse_capture_clause()?;
let decl = self.parse_fn_block_decl()?;
let decl_hi = self.prev_token.span;
let body = match decl.output {
let mut body = match decl.output {
FnRetTy::Default(_) => {
let restrictions = self.restrictions - Restrictions::STMT_EXPR;
self.parse_expr_res(restrictions, None)?
Expand All @@ -1753,11 +1758,28 @@ impl<'a> Parser<'a> {
self.sess.gated_spans.gate(sym::async_closure, span);
}

Ok(self.mk_expr(
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.
body = self.mk_expr_err(body.span);
}
Comment on lines +1761 to +1767
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm somewhat uneasy with this check, but having both Semi and Paren be true should only happen on malformed code.


let body_span = body.span;

let closure = self.mk_expr(
lo.to(body.span),
ExprKind::Closure(capture_clause, asyncness, movability, decl, body, lo.to(decl_hi)),
attrs,
))
);

// Disable recovery for closure body
let spans =
ClosureSpans { whole_closure: closure.span, closing_pipe: decl_hi, body: body_span };
self.current_closure = Some(spans);

Ok(closure)
}

/// Parses an optional `move` prefix to a closure-like construct.
Expand Down
103 changes: 98 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,17 @@ 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 current_closure: Option<ClosureSpans>,
}

/// Stores span informations about a closure.
#[derive(Clone)]
pub struct ClosureSpans {
pub whole_closure: Span,
pub closing_pipe: Span,
pub body: Span,
}

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

// Make parser point to the first token.
Expand Down Expand Up @@ -761,19 +773,41 @@ impl<'a> Parser<'a> {
first = false;
} else {
match self.expect(t) {
Ok(false) => {}
Ok(false) => {
self.current_closure.take();
}
Ok(true) => {
self.current_closure.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.current_closure.take() {
Some(closure_spans) 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(
closure_spans,
expect_err,
)?;

continue;
}

_ => {
// 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();
}
}
}
}

Expand Down Expand Up @@ -839,6 +873,65 @@ impl<'a> Parser<'a> {
Ok((v, trailing, recovered))
}

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

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

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

let preceding_pipe_span = closure_spans.closing_pipe;
let following_token_span = self.token.span;

let mut first_note = MultiSpan::from(vec![initial_semicolon]);
first_note.push_span_label(
initial_semicolon,
"this `;` turns the preceding closure into a statement".to_string(),
);
first_note.push_span_label(
closure_spans.body,
"this expression is a statement because of the trailing semicolon".to_string(),
);
expect_err.span_note(first_note, "statement found outside of a block");

let mut second_note = MultiSpan::from(vec![closure_spans.whole_closure]);
second_note.push_span_label(
closure_spans.whole_closure,
"this is the parsed closure...".to_string(),
);
second_note.push_span_label(
following_token_span,
"...but likely you meant the closure to end here".to_string(),
);
expect_err.span_note(second_note, "the closure body may be incorrectly delimited");

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 bodies 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 bodies 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,38 @@
error: closure bodies that contain statements must be surrounded by braces
--> $DIR/missing_braces_around_block.rs:14:26
|
LL | (1..num).reduce(|a, b|
| ^
...
LL | ).unwrap();
| ^
|
note: statement found outside of a block
--> $DIR/missing_braces_around_block.rs:16:26
|
LL | println!("{}", a);
| -----------------^ this `;` turns the preceding closure into a statement
| |
| this expression is a statement because of the trailing semicolon
note: the closure body may be incorrectly delimited
--> $DIR/missing_braces_around_block.rs:14:21
|
LL | (1..num).reduce(|a, b|
| _____________________^
LL | |
LL | | println!("{}", a);
| |_________________________^ this is the parsed closure...
LL | a * b
LL | ).unwrap();
| - ...but likely you meant the closure to end here
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

16 changes: 16 additions & 0 deletions src/test/ui/expr/malformed_closure/ruby_style_closure.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Part of issue #27300.
// The problem here is that ruby-style closures are parsed as blocks whose
// first statement is a closure. See the issue for more details:
// https://github.com/rust-lang/rust/issues/27300

// Note: this test represents what the compiler currently emits. The error
// message will be improved later.

fn main() {
let p = Some(45).and_then({
//~^ expected a `FnOnce<({integer},)>` closure, found `Option<_>`
|x| println!("doubling {}", x);
Some(x * 2)
//~^ ERROR: cannot find value `x` in this scope
});
}
18 changes: 18 additions & 0 deletions src/test/ui/expr/malformed_closure/ruby_style_closure.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error[E0425]: cannot find value `x` in this scope
--> $DIR/ruby_style_closure.rs:13:14
|
LL | Some(x * 2)
| ^ not found in this scope

error[E0277]: expected a `FnOnce<({integer},)>` closure, found `Option<_>`
--> $DIR/ruby_style_closure.rs:10:22
|
LL | let p = Some(45).and_then({
| ^^^^^^^^ expected an `FnOnce<({integer},)>` closure, found `Option<_>`
|
= help: the trait `FnOnce<({integer},)>` is not implemented for `Option<_>`

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0277, E0425.
For more information about an error, try `rustc --explain E0277`.