Supress suggest let else when no let in refutable bindings

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;
@@ -62,7 +62,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
@@ -161,7 +161,7 @@ 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);
+                self.check_let(pat, Some(expr), ex.span, None);
             }
             ExprKind::LogicalOp { op: LogicalOp::And, .. }
                 if !matches!(self.let_source, LetSource::None) =>
@@ -188,7 +188,12 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> {
                     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)
+                        // FIXME(#145548): We get hir_id from the lint_level, replace it if thir has a better way to get it.
+                        let hir_id = match lint_level {
+                            LintLevel::Explicit(hir_id) => Some(hir_id),
+                            _ => None,
+                        };
+                        this.check_let(pattern, initializer, span, hir_id)
                     });
                     visit::walk_stmt(this, stmt);
                 });
@@ -439,11 +444,17 @@ 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,
+        hir_id: Option<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), hir_id)
         } else {
             let Ok(refutability) = self.is_let_irrefutable(pat, scrut) else { return };
             if matches!(refutability, Irrefutable) {
@@ -519,6 +530,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 +677,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
         origin: &str,
         scrut: Option<&Expr<'tcx>>,
         sp: Option<Span>,
+        hir_id: Option<HirId>,
     ) {
         let pattern_ty = pat.ty;
 
@@ -715,6 +728,8 @@ 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
+            && !self.is_from_destructing_assignment(hir_id)
         {
             let mut bindings = vec![];
             pat.each_binding(|name, _, _, _| bindings.push(name));
@@ -768,6 +783,19 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
             adt_defined_here,
         }));
     }
+
+    /// Check if hir_id of LetStmt is desugared into a `let`, i.e., `LocalSource::AssignDesugar`
+    /// rather than a real `let` the user wrote. This helps suppress suggestions for `let` statements
+    /// when we're dealing with an assignment like `Some(x) = rhs`. See #145548.
+    fn is_from_destructing_assignment(&self, hir_id: Option<HirId>) -> bool {
+        if let Some(hir_id) = hir_id {
+            // pat_hir_id is the hir_id of the pattern in the `let` statement/expr.
+            if let Node::LetStmt(let_stmt) = self.tcx.hir_node(hir_id) {
+                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`.

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.rs

@@ -0,0 +1,18 @@
+// `Some(x) = Some(1)` is treated as a let statement/expr,
+// so we should not suggest to add `let ... else{...}`
+
+fn foo1(){
+    let x = 2;
+    Some(x) = Some(1); //~ ERROR refutable pattern in local binding [E0005]
+}
+
+fn foo2(){
+    let x = 2;
+    let Some(x) = Some(1); //~ ERROR refutable pattern in local binding [E0005]
+}
+
+fn foo3(){
+    Some(()) = Some(()); //~ ERROR refutable pattern in local binding [E0005]
+}
+
+fn main() {}

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

@@ -0,0 +1,37 @@
+error[E0005]: refutable pattern in local binding
+  --> $DIR/suggest-with-let-issue-145548.rs:6:5
+   |
+LL |     Some(x) = Some(1);
+   |     ^^^^^^^ pattern `None` not covered
+   |
+   = 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>`
+
+error[E0005]: refutable pattern in local binding
+  --> $DIR/suggest-with-let-issue-145548.rs:11:9
+   |
+LL |     let Some(x) = Some(1);
+   |         ^^^^^^^ pattern `None` not covered
+   |
+   = 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 |     let Some(x) = Some(1) else { todo!() };
+   |                           ++++++++++++++++
+
+error[E0005]: refutable pattern in local binding
+  --> $DIR/suggest-with-let-issue-145548.rs:15:5
+   |
+LL |     Some(()) = Some(());
+   |     ^^^^^^^^ pattern `None` not covered
+   |
+   = 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<()>`
+
+error: aborting due to 3 previous errors
+
+For more information about this error, try `rustc --explain E0005`.