From b21425de3c6be4264b875e68c8a28b424c27b8c4 Mon Sep 17 00:00:00 2001 From: Sasha Pourcelot Date: Mon, 16 Aug 2021 15:22:36 +0200 Subject: [PATCH] Emit proper errors on missing closure braces 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. --- compiler/rustc_parse/src/parser/expr.rs | 30 ++++- compiler/rustc_parse/src/parser/mod.rs | 103 +++++++++++++++++- .../missing_braces_around_block.fixed | 19 ++++ .../missing_braces_around_block.rs | 19 ++++ .../missing_braces_around_block.stderr | 38 +++++++ .../malformed_closure/ruby_style_closure.rs | 16 +++ .../ruby_style_closure.stderr | 18 +++ 7 files changed, 234 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/expr/malformed_closure/missing_braces_around_block.fixed create mode 100644 src/test/ui/expr/malformed_closure/missing_braces_around_block.rs create mode 100644 src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr create mode 100644 src/test/ui/expr/malformed_closure/ruby_style_closure.rs create mode 100644 src/test/ui/expr/malformed_closure/ruby_style_closure.stderr diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 05156745105a1..dc80dab8c6c5f 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -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; @@ -91,6 +94,8 @@ impl<'a> Parser<'a> { /// Parses an expression. #[inline] pub fn parse_expr(&mut self) -> PResult<'a, P> { + self.current_closure.take(); + self.parse_expr_res(Restrictions::empty(), None) } @@ -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)? @@ -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); + } + + 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. diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index c4419e995edac..5c701fefd17de 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -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, +} + +/// 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 @@ -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. @@ -761,8 +773,11 @@ 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; } @@ -770,10 +785,29 @@ impl<'a> Parser<'a> { 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(); + } + } } } @@ -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. diff --git a/src/test/ui/expr/malformed_closure/missing_braces_around_block.fixed b/src/test/ui/expr/malformed_closure/missing_braces_around_block.fixed new file mode 100644 index 0000000000000..c50b9a12b6d44 --- /dev/null +++ b/src/test/ui/expr/malformed_closure/missing_braces_around_block.fixed @@ -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(); +} diff --git a/src/test/ui/expr/malformed_closure/missing_braces_around_block.rs b/src/test/ui/expr/malformed_closure/missing_braces_around_block.rs new file mode 100644 index 0000000000000..58c81f3a6e2a9 --- /dev/null +++ b/src/test/ui/expr/malformed_closure/missing_braces_around_block.rs @@ -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(); +} diff --git a/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr b/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr new file mode 100644 index 0000000000000..dac9a8cfc69d4 --- /dev/null +++ b/src/test/ui/expr/malformed_closure/missing_braces_around_block.stderr @@ -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 + diff --git a/src/test/ui/expr/malformed_closure/ruby_style_closure.rs b/src/test/ui/expr/malformed_closure/ruby_style_closure.rs new file mode 100644 index 0000000000000..e4341e196877b --- /dev/null +++ b/src/test/ui/expr/malformed_closure/ruby_style_closure.rs @@ -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 + }); +} diff --git a/src/test/ui/expr/malformed_closure/ruby_style_closure.stderr b/src/test/ui/expr/malformed_closure/ruby_style_closure.stderr new file mode 100644 index 0000000000000..99df0632b4c33 --- /dev/null +++ b/src/test/ui/expr/malformed_closure/ruby_style_closure.stderr @@ -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`.