Supress suggest let else when no let in refutable bindings

Signed-off-by: xizheyin <[email protected]>

Author: xizheyin

compiler/rustc_middle/src/thir.rs

@@ -241,6 +241,8 @@ pub enum StmtKind<'tcx> {
 
         /// Span of the `let <PAT> = <INIT>` part.
         span: Span,
+
+        pat_hir_id: HirId,
     },
 }
 
@@ -396,6 +398,7 @@ pub enum ExprKind<'tcx> {
     Let {
         expr: ExprId,
         pat: Box<Pat<'tcx>>,
+        pat_hir_id: HirId,
     },
     /// A `match` expression.
     Match {
@@ -1129,7 +1132,7 @@ mod size_asserts {
     static_assert_size!(ExprKind<'_>, 40);
     static_assert_size!(Pat<'_>, 64);
     static_assert_size!(PatKind<'_>, 48);
-    static_assert_size!(Stmt<'_>, 48);
-    static_assert_size!(StmtKind<'_>, 48);
+    static_assert_size!(Stmt<'_>, 56);
+    static_assert_size!(StmtKind<'_>, 56);
     // tidy-alphabetical-end
 }

compiler/rustc_middle/src/thir/visit.rs

@@ -79,7 +79,7 @@ pub fn walk_expr<'thir, 'tcx: 'thir, V: Visitor<'thir, 'tcx>>(
         PointerCoercion { source, cast: _, is_from_as_cast: _ } => {
             visitor.visit_expr(&visitor.thir()[source])
         }
-        Let { expr, ref pat } => {
+        Let { expr, ref pat, pat_hir_id: _ } => {
             visitor.visit_expr(&visitor.thir()[expr]);
             visitor.visit_pat(pat);
         }
@@ -209,6 +209,7 @@ pub fn walk_stmt<'thir, 'tcx: 'thir, V: Visitor<'thir, 'tcx>>(
             lint_level: _,
             else_block,
             span: _,
+            pat_hir_id: _,
         } => {
             if let Some(init) = initializer {
                 visitor.visit_expr(&visitor.thir()[*init]);

compiler/rustc_mir_build/src/builder/block.rs

@@ -86,6 +86,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     lint_level,
                     else_block: Some(else_block),
                     span: _,
+                    pat_hir_id: _,
                 } => {
                     // When lowering the statement `let <pat> = <expr> else { <else> };`,
                     // the `<else>` block is nested in the parent scope enclosing this statement.
@@ -235,6 +236,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     lint_level,
                     else_block: None,
                     span: _,
+                    pat_hir_id: _,
                 } => {
                     let ignores_expr_result = matches!(pattern.kind, PatKind::Wild);
                     this.block_context.push(BlockFrame::Statement { ignores_expr_result });

compiler/rustc_mir_build/src/builder/matches/mod.rs

@@ -186,7 +186,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 })
             }
             ExprKind::Use { source } => this.then_else_break_inner(block, source, args),
-            ExprKind::Let { expr, ref pat } => this.lower_let_expr(
+            ExprKind::Let { expr, ref pat, pat_hir_id: _ } => this.lower_let_expr(
                 block,
                 expr,
                 pat,
@@ -774,7 +774,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         visibility_scope: Option<SourceScope>,
     ) {
         match self.thir.exprs[guard_expr].kind {
-            ExprKind::Let { expr: _, pat: ref guard_pat } => {
+            ExprKind::Let { expr: _, pat: ref guard_pat, pat_hir_id: _ } => {
                 // FIXME: pass a proper `opt_match_place`
                 self.declare_bindings(visibility_scope, scope_span, guard_pat, None, None);
             }

compiler/rustc_mir_build/src/thir/cx/block.rs

@@ -112,6 +112,7 @@ impl<'tcx> ThirBuildCx<'tcx> {
                                 else_block,
                                 lint_level: LintLevel::Explicit(local.hir_id),
                                 span,
+                                pat_hir_id: local.pat.hir_id,
                             },
                         };
                         Some(self.thir.stmts.push(stmt))

compiler/rustc_mir_build/src/thir/cx/expr.rs

@@ -892,6 +892,7 @@ impl<'tcx> ThirBuildCx<'tcx> {
             hir::ExprKind::Let(let_expr) => ExprKind::Let {
                 expr: self.mirror_expr(let_expr.init),
                 pat: self.pattern_from_hir(let_expr.pat),
+                pat_hir_id: let_expr.pat.hir_id,
             },
             hir::ExprKind::If(cond, then, else_opt) => ExprKind::If {
                 if_then_scope: region::Scope {

compiler/rustc_mir_build/src/thir/pattern/check_match.rs

@@ -6,7 +6,7 @@ use rustc_errors::codes::*;
 use rustc_errors::{Applicability, ErrorGuaranteed, MultiSpan, struct_span_code_err};
 use rustc_hir::def::*;
 use rustc_hir::def_id::LocalDefId;
-use rustc_hir::{self as hir, BindingMode, ByRef, HirId, MatchSource};
+use rustc_hir::{self as hir, BindingMode, ByRef, HirId, LocalSource, MatchSource, Node};
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_lint::Level;
 use rustc_middle::bug;
@@ -32,6 +32,7 @@ use crate::errors::*;
 use crate::fluent_generated as fluent;
 
 pub(crate) fn check_match(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Result<(), ErrorGuaranteed> {
+    //println!("check_match: {:?}, span: {:?}", def_id, tcx.def_span(def_id));
     let typeck_results = tcx.typeck(def_id);
     let (thir, expr) = tcx.thir_body(def_id)?;
     let thir = thir.borrow();
@@ -62,7 +63,7 @@ pub(crate) fn check_match(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Result<(), Err
 
     for param in thir.params.iter() {
         if let Some(box ref pattern) = param.pat {
-            visitor.check_binding_is_irrefutable(pattern, origin, None, None);
+            visitor.check_binding_is_irrefutable(pattern, origin, None, None, None);
         }
     }
     visitor.error
@@ -160,8 +161,8 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> {
             } => {
                 self.check_match(scrutinee, arms, MatchSource::Normal, span);
             }
-            ExprKind::Let { box ref pat, expr } => {
-                self.check_let(pat, Some(expr), ex.span);
+            ExprKind::Let { box ref pat, expr, pat_hir_id } => {
+                self.check_let(pat, Some(expr), ex.span, pat_hir_id);
             }
             ExprKind::LogicalOp { op: LogicalOp::And, .. }
                 if !matches!(self.let_source, LetSource::None) =>
@@ -182,13 +183,19 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> {
     fn visit_stmt(&mut self, stmt: &'p Stmt<'tcx>) {
         match stmt.kind {
             StmtKind::Let {
-                box ref pattern, initializer, else_block, lint_level, span, ..
+                box ref pattern,
+                initializer,
+                else_block,
+                lint_level,
+                span,
+                pat_hir_id,
+                ..
             } => {
                 self.with_lint_level(lint_level, |this| {
                     let let_source =
                         if else_block.is_some() { LetSource::LetElse } else { LetSource::PlainLet };
                     this.with_let_source(let_source, |this| {
-                        this.check_let(pattern, initializer, span)
+                        this.check_let(pattern, initializer, span, pat_hir_id)
                     });
                     visit::walk_stmt(this, stmt);
                 });
@@ -262,7 +269,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
             ExprKind::Scope { value, lint_level, .. } => {
                 self.with_lint_level(lint_level, |this| this.visit_land_rhs(&this.thir[value]))
             }
-            ExprKind::Let { box ref pat, expr } => {
+            ExprKind::Let { box ref pat, expr, pat_hir_id: _ } => {
                 let expr = &self.thir()[expr];
                 self.with_let_source(LetSource::None, |this| {
                     this.visit_expr(expr);
@@ -439,11 +446,23 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
     }
 
     #[instrument(level = "trace", skip(self))]
-    fn check_let(&mut self, pat: &'p Pat<'tcx>, scrutinee: Option<ExprId>, span: Span) {
+    fn check_let(
+        &mut self,
+        pat: &'p Pat<'tcx>,
+        scrutinee: Option<ExprId>,
+        span: Span,
+        pat_hir_id: HirId,
+    ) {
         assert!(self.let_source != LetSource::None);
         let scrut = scrutinee.map(|id| &self.thir[id]);
         if let LetSource::PlainLet = self.let_source {
-            self.check_binding_is_irrefutable(pat, "local binding", scrut, Some(span))
+            self.check_binding_is_irrefutable(
+                pat,
+                "local binding",
+                scrut,
+                Some(span),
+                Some(pat_hir_id),
+            )
         } else {
             let Ok(refutability) = self.is_let_irrefutable(pat, scrut) else { return };
             if matches!(refutability, Irrefutable) {
@@ -519,6 +538,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
                     "`for` loop binding",
                     None,
                     None,
+                    None,
                 );
             } else {
                 // span after scrutinee, or after `.match`. That is, the braces, arms,
@@ -665,6 +685,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
         origin: &str,
         scrut: Option<&Expr<'tcx>>,
         sp: Option<Span>,
+        pat_hir_id: Option<HirId>,
     ) {
         let pattern_ty = pat.ty;
 
@@ -715,6 +736,9 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
             && self.tcx.sess.source_map().is_span_accessible(span)
             && interpreted_as_const.is_none()
             && scrut.is_some()
+            // we only suggest `let else` when the pattern is not desugared from an assignment
+            && let Some(pat_hir_id) = pat_hir_id
+            && !self.is_from_destructing_assignment(pat_hir_id)
         {
             let mut bindings = vec![];
             pat.each_binding(|name, _, _, _| bindings.push(name));
@@ -768,6 +792,20 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
             adt_defined_here,
         }));
     }
+
+    /// Check if `pat` is desugared into a `let`, i.e., `LocalSource::AssignDesugar`
+    /// rather than a real `let` the user wrote.
+    /// This helps suppress suggestions for `let` statements/exprs when
+    /// we're dealing with an assignment like `Some(x) = rhs`.
+    fn is_from_destructing_assignment(&self, pat_hir_id: HirId) -> bool {
+        // pat_hir_id is the hir_id of the pattern in the `let` statement/expr.
+        for (_, node) in self.tcx.hir_parent_iter(pat_hir_id) {
+            if let Node::LetStmt(let_stmt) = node {
+                return matches!(let_stmt.source, LocalSource::AssignDesugar(_));
+            }
+        }
+        false
+    }
 }
 
 /// Check if a by-value binding is by-value. That is, check if the binding's type is not `Copy`.

compiler/rustc_mir_build/src/thir/print.rs

@@ -144,6 +144,7 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> {
                 else_block,
                 lint_level,
                 span,
+                pat_hir_id,
             } => {
                 print_indented!(self, "kind: Let {", depth_lvl + 1);
                 print_indented!(
@@ -175,6 +176,7 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> {
 
                 print_indented!(self, format!("lint_level: {:?}", lint_level), depth_lvl + 2);
                 print_indented!(self, format!("span: {:?}", span), depth_lvl + 2);
+                print_indented!(self, format!("pat_hir_id: {:?}", pat_hir_id), depth_lvl + 2);
                 print_indented!(self, "}", depth_lvl + 1);
             }
         }
@@ -337,11 +339,12 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> {
                 print_indented!(self, "}", depth_lvl + 2);
                 print_indented!(self, "}", depth_lvl);
             }
-            Let { expr, pat } => {
+            Let { expr, pat, pat_hir_id } => {
                 print_indented!(self, "Let {", depth_lvl);
                 print_indented!(self, "expr:", depth_lvl + 1);
                 self.print_expr(*expr, depth_lvl + 2);
                 print_indented!(self, format!("pat: {:?}", pat), depth_lvl + 1);
+                print_indented!(self, format!("pat_hir_id: {:?}", pat_hir_id), depth_lvl + 1);
                 print_indented!(self, "}", depth_lvl);
             }
             Match { scrutinee, arms, .. } => {

tests/ui/destructuring-assignment/non-exhaustive-destructure.stderr

@@ -7,10 +7,6 @@ LL |     None = Some(3);
    = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
    = note: for more information, visit https://doc.rust-lang.org/book/ch19-02-refutability.html
    = note: the matched value is of type `Option<i32>`
-help: you might want to use `if let` to ignore the variant that isn't matched
-   |
-LL |     if None = Some(3) { todo!() };
-   |     ++                +++++++++++
 
 error: aborting due to 1 previous error
 

tests/ui/let-else/suggest-with-let-issue-145548.stderr

@@ -7,10 +7,6 @@ LL |     Some(x) = Some(1);
    = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
    = note: for more information, visit https://doc.rust-lang.org/book/ch19-02-refutability.html
    = note: the matched value is of type `Option<i32>`
-help: you might want to use `let else` to handle the variant that isn't matched
-   |
-LL |     Some(x) = Some(1) else { todo!() };
-   |                       ++++++++++++++++
 
 error[E0005]: refutable pattern in local binding
   --> $DIR/suggest-with-let-issue-145548.rs:11:9
@@ -35,10 +31,6 @@ LL |     Some(()) = Some(());
    = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
    = note: for more information, visit https://doc.rust-lang.org/book/ch19-02-refutability.html
    = note: the matched value is of type `Option<()>`
-help: you might want to use `if let` to ignore the variant that isn't matched
-   |
-LL |     if Some(()) = Some(()) { todo!() };
-   |     ++                     +++++++++++
 
 error: aborting due to 3 previous errors