Skip to content
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
12 changes: 12 additions & 0 deletions compiler/rustc_parse/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4651,3 +4651,15 @@ pub(crate) struct SuggestIntroduceTypeParameter {
pub span: Span,
pub parameters: String,
}

#[derive(Subdiagnostic)]
#[suggestion(
"you might have meant to write a diverging block on a refutable `let` statement by using `let-else`
for more information, visit <https://doc.rust-lang.org/beta/rust-by-example/flow_control/let_else.html>",
code = " else ",
applicability = "maybe-incorrect"
)]
pub(crate) struct MissingElseInLet {
#[primary_span]
pub span: Span,
}
31 changes: 29 additions & 2 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ impl<'a> Parser<'a> {
self.current_closure.take();
self.parse_expr_res(Restrictions::empty())
}

#[inline]
pub fn parse_expr_in_let(&mut self) -> PResult<'a, Box<Expr>> {
self.current_closure.take();
self.parse_expr_res(Restrictions::IN_LET)
}
/// Parses an expression, forcing tokens to be collected.
pub fn parse_expr_force_collect(&mut self) -> PResult<'a, Box<Expr>> {
self.current_closure.take();
Expand Down Expand Up @@ -3680,11 +3684,12 @@ impl<'a> Parser<'a> {
Option<ErrorGuaranteed>, /* async blocks are forbidden in Rust 2015 */
),
> {
let open_span = self.prev_token.span; //{
let mut fields = ThinVec::new();
let mut base = ast::StructRest::None;
let mut recovered_async = None;
let in_if_guard = self.restrictions.contains(Restrictions::IN_IF_GUARD);

let in_let = self.restrictions.contains(Restrictions::IN_LET);
let async_block_err = |e: &mut Diag<'_>, span: Span| {
crate::diagnostics::AsyncBlockIn2015 { span }.add_to_diag(e);
crate::diagnostics::HelpUseLatestEdition::new().add_to_diag(e);
Expand Down Expand Up @@ -3769,6 +3774,28 @@ impl<'a> Parser<'a> {
return Err(e);
}

if in_let {
// Better diagnostic for `foo { return 42; };`
// We've consumed `foo {` already
let mut snapshot = self.create_snapshot_for_diagnostic();
let might_be_stmt = snapshot.token.is_keyword(kw::Return);
snapshot.consume_block(
exp!(OpenBrace),
exp!(CloseBrace),
super::diagnostics::ConsumeClosingDelim::Yes,
); //consume to the end of the block, including `}`

// make sure the block is at the end by eating a `;`,
// we shouldn't report such diagnostic for `let a = foo{return 43;}+bar;`
// also skip the suggestion if the span cross macro boundaries
if might_be_stmt && snapshot.eat(exp!(Semi)) && pth.span.eq_ctxt(open_span)
{
let span = pth.span.between(open_span);
e.subdiagnostic(crate::diagnostics::MissingElseInLet { span });
self.restore_snapshot(snapshot);
return Err(e);
}
}
let guar = e.emit_err();
if pth == kw::Async {
recovered_async = Some(guar);
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ bitflags::bitflags! {
/// expression, but halts parsing the expression when reaching certain
/// tokens like `=`.
const IS_PAT = 1 << 5;
/// Used to detect a missing `else` in a let statement.
/// e.g. let Some(foo) = bar{return;};
const IN_LET = 1 << 6;
Comment on lines +128 to +130

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am concerned that we're starting to creep on the limits of a byte here, hope we don't have a need to increase the number of flags anytime soon.

}
}

Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_parse/src/parser/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@ use std::mem;
use std::ops::Bound;

use ast::Label;
use rustc_ast as ast;
use rustc_ast::token::{self, Delimiter, InvisibleOrigin, MetaVarKind, TokenKind};
use rustc_ast::tokenstream::TokenTree;
use rustc_ast::util::classify::{self, TrailingBrace};
use rustc_ast::visit::{Visitor, walk_expr};
use rustc_ast::{
AttrStyle, AttrVec, Block, BlockCheckMode, DUMMY_NODE_ID, Expr, ExprKind, HasAttrs, Local,
LocalKind, MacCall, MacCallStmt, MacStmtStyle, Recovered, Stmt, StmtKind,
self as ast, AttrStyle, AttrVec, Block, BlockCheckMode, DUMMY_NODE_ID, Expr, ExprKind,
HasAttrs, Local, LocalKind, MacCall, MacCallStmt, MacStmtStyle, Recovered, Stmt, StmtKind,
};
use rustc_errors::{Applicability, Diag, PResult};
use rustc_span::{ErrorGuaranteed, Ident, Span, kw, sym};
Expand Down Expand Up @@ -517,7 +516,7 @@ impl<'a> Parser<'a> {
_ => self.eat(exp!(Eq)),
};

Ok(if eq_consumed || eq_optional { Some(self.parse_expr()?) } else { None })
Ok(if eq_consumed || eq_optional { Some(self.parse_expr_in_let()?) } else { None })
}

/// Parses a block. No inner attributes are allowed.
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/parser/detect-missing-else-in-let-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// detect missing else in let statement. (issue #135857)
fn main() {
let foo = Some(1);
let Some(a) = foo{return;};
//~^ HELP you might have meant to write a diverging block
//~| HELP escape `return` to use it as an identifier
//~| ERROR expected identifier, found keyword `return`
}
20 changes: 20 additions & 0 deletions tests/ui/parser/detect-missing-else-in-let-1.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: expected identifier, found keyword `return`
--> $DIR/detect-missing-else-in-let-1.rs:4:23
|
LL | let Some(a) = foo{return;};
| --- ^^^^^^ expected identifier, found keyword
| |
| while parsing this struct
|
help: escape `return` to use it as an identifier
|
LL | let Some(a) = foo{r#return;};
| ++
Comment on lines +9 to +12

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Future improvement: we should likely keep a list of bindings introduced in the current item and only suggest using identifiers as values (either here for turning keywords into raw identifiers, or in other cases) when that would make sense. I've stopped myself from doing that because it is a worse version of name resolution purely for parsing that will introduce some additional memory pressure, but maybe we should experiment with it and see if it is worthwhile.

help: you might have meant to write a diverging block on a refutable `let` statement by using `let-else`
for more information, visit <https://doc.rust-lang.org/beta/rust-by-example/flow_control/let_else.html>
|
LL | let Some(a) = foo else {return;};
| ++++

error: aborting due to 1 previous error

10 changes: 10 additions & 0 deletions tests/ui/parser/detect-missing-else-in-let-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// the compiler currently will suggest the user to add `else` in this case
// it shouldn't because `bar` is an irrefutable pattern, the else block would never be valid/useful
// it is currently unmitigated
fn main(){
let foo = 12;
let bar = foo{return;};
//~^ HELP you might have meant to write a diverging block
//~| HELP escape `return` to use it as an identifier
//~| ERROR expected identifier, found keyword `return`
}
20 changes: 20 additions & 0 deletions tests/ui/parser/detect-missing-else-in-let-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: expected identifier, found keyword `return`
--> $DIR/detect-missing-else-in-let-2.rs:6:19
|
LL | let bar = foo{return;};
| --- ^^^^^^ expected identifier, found keyword
| |
| while parsing this struct
|
help: escape `return` to use it as an identifier
|
LL | let bar = foo{r#return;};
| ++
help: you might have meant to write a diverging block on a refutable `let` statement by using `let-else`
for more information, visit <https://doc.rust-lang.org/beta/rust-by-example/flow_control/let_else.html>
|
LL | let bar = foo else {return;};
| ++++

error: aborting due to 1 previous error

Loading