From d216ca0506d6a70ca70fc29974ed9155beaabf7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 18 Aug 2025 16:52:02 +0000 Subject: [PATCH 1/2] Detect missing `if let` or `let-else` During `let` binding parse error and encountering a block, detect if there is a likely missing `if` or `else`: ``` error: expected one of `.`, `;`, `?`, `else`, or an operator, found `{` --> $DIR/missing-if-let-or-let-else.rs:14:25 | LL | let Some(x) = foo() { | ^ expected one of `.`, `;`, `?`, `else`, or an operator | help: you might have meant to use `if let` | LL | if let Some(x) = foo() { | ++ help: alternatively, you might have meant to use `let else` | LL | let Some(x) = foo() else { | ++++ ``` --- compiler/rustc_parse/src/lib.rs | 1 + compiler/rustc_parse/src/parser/stmt.rs | 94 +++++++++++++++++++ .../uninhabited/missing-if-let-or-let-else.rs | 24 +++++ .../missing-if-let-or-let-else.stderr | 39 ++++++++ 4 files changed, 158 insertions(+) create mode 100644 tests/ui/uninhabited/missing-if-let-or-let-else.rs create mode 100644 tests/ui/uninhabited/missing-if-let-or-let-else.stderr diff --git a/compiler/rustc_parse/src/lib.rs b/compiler/rustc_parse/src/lib.rs index 2050c5f960877..0dd5d89491b40 100644 --- a/compiler/rustc_parse/src/lib.rs +++ b/compiler/rustc_parse/src/lib.rs @@ -6,6 +6,7 @@ #![feature(assert_matches)] #![feature(box_patterns)] #![feature(debug_closure_helpers)] +#![feature(default_field_values)] #![feature(if_let_guard)] #![feature(iter_intersperse)] #![recursion_limit = "256"] diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index b4943ff7de64f..732c653e4bc8e 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -6,6 +6,7 @@ use ast::Label; use rustc_ast as ast; use rustc_ast::token::{self, Delimiter, InvisibleOrigin, MetaVarKind, TokenKind}; 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, @@ -783,6 +784,71 @@ impl<'a> Parser<'a> { Ok(self.mk_block(stmts, s, lo.to(self.prev_token.span))) } + fn recover_missing_let_else(&mut self, err: &mut Diag<'_>, pat: &ast::Pat, stmt_span: Span) { + if self.token.kind != token::OpenBrace { + return; + } + match pat.kind { + ast::PatKind::Ident(..) | ast::PatKind::Missing | ast::PatKind::Wild => { + // Not if let or let else + return; + } + _ => {} + } + let snapshot = self.create_snapshot_for_diagnostic(); + let block_span = self.token.span; + let (if_let, let_else) = match self.parse_block() { + Ok(block) => { + let mut idents = vec![]; + pat.walk(&mut |pat: &ast::Pat| { + if let ast::PatKind::Ident(_, ident, _) = pat.kind { + idents.push(ident); + } + true + }); + // Collect all bindings in pattern and see if they appear in the block. Likely meant + // to write `if let`. See if the block has a return. Likely meant to write + // `let else`. + let mut visitor = IdentFinder { idents, .. }; + visitor.visit_block(&block); + + (visitor.references_ident, visitor.has_return) + } + Err(e) => { + e.cancel(); + self.restore_snapshot(snapshot); + (false, false) + } + }; + + let mut alternatively = ""; + if if_let || !let_else { + alternatively = "alternatively, "; + err.span_suggestion_verbose( + stmt_span.shrink_to_lo(), + "you might have meant to use `if let`", + "if ".to_string(), + if if_let { + Applicability::MachineApplicable + } else { + Applicability::MaybeIncorrect + }, + ); + } + if let_else || !if_let { + err.span_suggestion_verbose( + block_span.shrink_to_lo(), + format!("{alternatively}you might have meant to use `let else`"), + "else ".to_string(), + if let_else { + Applicability::MachineApplicable + } else { + Applicability::MaybeIncorrect + }, + ); + } + } + fn recover_missing_dot(&mut self, err: &mut Diag<'_>) { let Some((ident, _)) = self.token.ident() else { return; @@ -977,6 +1043,7 @@ impl<'a> Parser<'a> { self.check_mistyped_turbofish_with_multiple_type_params(e, expr).map_err( |mut e| { self.recover_missing_dot(&mut e); + self.recover_missing_let_else(&mut e, &local.pat, stmt.span); e }, )?; @@ -1065,3 +1132,30 @@ impl<'a> Parser<'a> { self.mk_block(thin_vec![self.mk_stmt_err(span, guar)], BlockCheckMode::Default, span) } } + +struct IdentFinder { + idents: Vec, + /// If a block references one of the bindings introduced by the let pattern, we likely meant to + /// use `if let`. + /// This is pre-expansion, so if we encounter `let Some(x) = foo() { println!("{x}") }` we won't + /// find it. + references_ident: bool = false, + /// If a block has a `return`, then we know with high certainty that the + has_return: bool = false, +} + +impl<'a> Visitor<'a> for IdentFinder { + fn visit_ident(&mut self, ident: &Ident) { + for i in &self.idents { + if ident.name == i.name { + self.references_ident = true; + } + } + } + fn visit_expr(&mut self, node: &'a Expr) { + if let ExprKind::Ret(..) = node.kind { + self.has_return = true; + } + walk_expr(self, node); + } +} diff --git a/tests/ui/uninhabited/missing-if-let-or-let-else.rs b/tests/ui/uninhabited/missing-if-let-or-let-else.rs new file mode 100644 index 0000000000000..51fedb797562c --- /dev/null +++ b/tests/ui/uninhabited/missing-if-let-or-let-else.rs @@ -0,0 +1,24 @@ +fn a() { + let Some(x) = foo() { //~ ERROR expected one of + //~^ HELP you might have meant to use `if let` + let y = x; + } +} +fn b() { + let Some(x) = foo() { //~ ERROR expected one of + //~^ HELP you might have meant to use `let else` + return; + } +} +fn c() { + let Some(x) = foo() { //~ ERROR expected one of + //~^ HELP you might have meant to use `if let` + //~| HELP alternatively, you might have meant to use `let else` + // The parser check happens pre-macro-expansion, so we don't know for sure. + println!("{x}"); + } +} +fn foo() -> Option { + Some(42) +} +fn main() {} diff --git a/tests/ui/uninhabited/missing-if-let-or-let-else.stderr b/tests/ui/uninhabited/missing-if-let-or-let-else.stderr new file mode 100644 index 0000000000000..4b78a0fa16e8d --- /dev/null +++ b/tests/ui/uninhabited/missing-if-let-or-let-else.stderr @@ -0,0 +1,39 @@ +error: expected one of `.`, `;`, `?`, `else`, or an operator, found `{` + --> $DIR/missing-if-let-or-let-else.rs:2:25 + | +LL | let Some(x) = foo() { + | ^ expected one of `.`, `;`, `?`, `else`, or an operator + | +help: you might have meant to use `if let` + | +LL | if let Some(x) = foo() { + | ++ + +error: expected one of `.`, `;`, `?`, `else`, or an operator, found `{` + --> $DIR/missing-if-let-or-let-else.rs:8:25 + | +LL | let Some(x) = foo() { + | ^ expected one of `.`, `;`, `?`, `else`, or an operator + | +help: you might have meant to use `let else` + | +LL | let Some(x) = foo() else { + | ++++ + +error: expected one of `.`, `;`, `?`, `else`, or an operator, found `{` + --> $DIR/missing-if-let-or-let-else.rs:14:25 + | +LL | let Some(x) = foo() { + | ^ expected one of `.`, `;`, `?`, `else`, or an operator + | +help: you might have meant to use `if let` + | +LL | if let Some(x) = foo() { + | ++ +help: alternatively, you might have meant to use `let else` + | +LL | let Some(x) = foo() else { + | ++++ + +error: aborting due to 3 previous errors + From 3af81cf0b7bd394dac89cbacec303b5937b5519a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 30 Aug 2025 18:42:07 +0000 Subject: [PATCH 2/2] review comment: move `Visitor` --- compiler/rustc_parse/src/parser/stmt.rs | 56 +++++++++++++------------ 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index 732c653e4bc8e..ad5ab6e6b7794 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -806,6 +806,35 @@ impl<'a> Parser<'a> { } true }); + + struct IdentFinder { + idents: Vec, + /// If a block references one of the bindings introduced by the let pattern, + /// we likely meant to use `if let`. + /// This is pre-expansion, so if we encounter + /// `let Some(x) = foo() { println!("{x}") }` we won't find it. + references_ident: bool = false, + /// If a block has a `return`, then we know with high certainty that it was + /// meant to be let-else. + has_return: bool = false, + } + + impl<'a> Visitor<'a> for IdentFinder { + fn visit_ident(&mut self, ident: &Ident) { + for i in &self.idents { + if ident.name == i.name { + self.references_ident = true; + } + } + } + fn visit_expr(&mut self, node: &'a Expr) { + if let ExprKind::Ret(..) = node.kind { + self.has_return = true; + } + walk_expr(self, node); + } + } + // Collect all bindings in pattern and see if they appear in the block. Likely meant // to write `if let`. See if the block has a return. Likely meant to write // `let else`. @@ -1132,30 +1161,3 @@ impl<'a> Parser<'a> { self.mk_block(thin_vec![self.mk_stmt_err(span, guar)], BlockCheckMode::Default, span) } } - -struct IdentFinder { - idents: Vec, - /// If a block references one of the bindings introduced by the let pattern, we likely meant to - /// use `if let`. - /// This is pre-expansion, so if we encounter `let Some(x) = foo() { println!("{x}") }` we won't - /// find it. - references_ident: bool = false, - /// If a block has a `return`, then we know with high certainty that the - has_return: bool = false, -} - -impl<'a> Visitor<'a> for IdentFinder { - fn visit_ident(&mut self, ident: &Ident) { - for i in &self.idents { - if ident.name == i.name { - self.references_ident = true; - } - } - } - fn visit_expr(&mut self, node: &'a Expr) { - if let ExprKind::Ret(..) = node.kind { - self.has_return = true; - } - walk_expr(self, node); - } -}