Tweak irrefutable let else warning output

compiler/rustc_mir_build/src/errors.rs

@@ -924,22 +924,24 @@ pub(crate) struct IrrefutableLetPatternsIfLetGuard {
 }
 
 #[derive(Diagnostic)]
-#[diag(
-    "irrefutable `let...else` {$count ->
-    [one] pattern
-    *[other] patterns
-}"
-)]
-#[note(
-    "{$count ->
-    [one] this pattern always matches, so the else clause is unreachable
-    *[other] these patterns always match, so the else clause is unreachable
-}"
-)]
+#[diag("unreachable `else` clause")]
+#[note("this pattern always matches, so the else clause is unreachable")]
 pub(crate) struct IrrefutableLetPatternsLetElse {
-    pub(crate) count: usize,
-    #[help("remove this `else` block")]
-    pub(crate) else_span: Option<Span>,
+    #[subdiagnostic]
+    pub(crate) be_replaced: Option<BeReplacedLetElse>,
+}
+
+#[derive(Subdiagnostic, Debug)]
+#[suggestion(
+    "consider using `let {$lhs} = {$rhs}` to match on a specific variant",
+    code = "let {lhs} = {rhs}",
+    applicability = "machine-applicable"
+)]
+pub(crate) struct BeReplacedLetElse {
+    #[primary_span]
+    pub(crate) span: Span,
+    pub(crate) lhs: String,
+    pub(crate) rhs: String,
 }
 
 #[derive(Diagnostic)]

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

@@ -168,7 +168,7 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> {
                 let Ok(()) = self.visit_land(ex, &mut chain_refutabilities) else { return };
                 // Lint only single irrefutable let binding.
                 if let [Some((_, Irrefutable))] = chain_refutabilities[..] {
-                    self.lint_single_let(ex.span, None);
+                    self.lint_single_let(ex.span, None, None);
                 }
                 return;
             }
@@ -438,7 +438,45 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
         if let LetSource::PlainLet = self.let_source {
             self.check_binding_is_irrefutable(pat, "local binding", scrut, Some(span));
         } else if let Ok(Irrefutable) = self.is_let_irrefutable(pat, scrut) {
-            self.lint_single_let(span, else_span);
+            if span.from_expansion() {
+                self.lint_single_let(span, None, None);
+                return;
+            }
+            let let_else_span = self.check_irrefutable_option_some(pat, scrut, span);
+
+            let sm = self.tcx.sess.source_map();
+            let next_token_start = sm.span_extend_while_whitespace(span.clone()).hi();
+            let line_span = sm.span_extend_to_line(span.clone()).with_lo(next_token_start);
+            let else_keyword_span = sm.span_until_whitespace(line_span);
+            self.lint_single_let(pat.span, Some(else_keyword_span), let_else_span);
+        }
+    }
+
+    /// Check case `let x = Some(y);`, user likely intended to destructure `Option`
+    fn check_irrefutable_option_some(
+        &self,
+        pat: &'p Pat<'tcx>,
+        initializer: Option<&Expr<'tcx>>,
+        span: Span,
+    ) -> Option<BeReplacedLetElse> {
+        if let sm = self.tcx.sess.source_map()
+            && let Some(initializer) = initializer
+            && let Some(s_ty) = initializer.ty.ty_adt_def()
+            && self.tcx.is_diagnostic_item(rustc_span::sym::Option, s_ty.did())
+            && let ExprKind::Scope { value, .. } = initializer.kind
+            && let initializer_expr = &self.thir[value]
+            && let ExprKind::Adt(box AdtExpr { fields, .. }) = &initializer_expr.kind
+            && let Some(field) = fields.first()
+            && let inner = &self.thir[field.expr]
+            && let Some(inner_ty) = inner.ty.ty_adt_def()
+            && self.tcx.is_diagnostic_item(rustc_span::sym::Option, inner_ty.did())
+            && let Ok(rhs) = sm.span_to_snippet(inner.span)
+            && let Ok(lhs) = sm.span_to_snippet(pat.span)
+        {
+            let lhs = format!("Some({})", lhs);
+            Some(BeReplacedLetElse { span, lhs, rhs })
+        } else {
+            None
         }
     }
 
@@ -546,14 +584,20 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
     }
 
     #[instrument(level = "trace", skip(self))]
-    fn lint_single_let(&mut self, let_span: Span, else_span: Option<Span>) {
+    fn lint_single_let(
+        &mut self,
+        let_span: Span,
+        else_keyword_span: Option<Span>,
+        let_else_span: Option<BeReplacedLetElse>,
+    ) {
         report_irrefutable_let_patterns(
             self.tcx,
             self.hir_source,
             self.let_source,
             1,
             let_span,
-            else_span,
+            else_keyword_span,
+            let_else_span,
         );
     }
 
@@ -849,7 +893,8 @@ fn report_irrefutable_let_patterns(
     source: LetSource,
     count: usize,
     span: Span,
-    else_span: Option<Span>,
+    else_keyword_span: Option<Span>,
+    let_else_span: Option<BeReplacedLetElse>,
 ) {
     macro_rules! emit_diag {
         ($lint:tt) => {{
@@ -862,11 +907,23 @@ fn report_irrefutable_let_patterns(
         LetSource::IfLet | LetSource::ElseIfLet => emit_diag!(IrrefutableLetPatternsIfLet),
         LetSource::IfLetGuard => emit_diag!(IrrefutableLetPatternsIfLetGuard),
         LetSource::LetElse => {
+            let spans = match else_keyword_span {
+                Some(else_keyword_span) => {
+                    let mut spans = MultiSpan::from_span(else_keyword_span);
+                    spans.push_span_label(
+                        span,
+                        msg!("assigning to binding pattern will always succeed"),
+                    );
+                    spans
+                }
+                None => span.into(),
+            };
+
             tcx.emit_node_span_lint(
                 IRREFUTABLE_LET_PATTERNS,
                 id,
-                span,
-                IrrefutableLetPatternsLetElse { count, else_span },
+                spans,
+                IrrefutableLetPatternsLetElse { be_replaced: let_else_span },
             );
         }
         LetSource::WhileLet => emit_diag!(IrrefutableLetPatternsWhileLet),

tests/ui/binding/irrefutable-in-let-chains.stderr

@@ -1,54 +1,54 @@
 warning: irrefutable `if let` pattern
-  --> $DIR/irrefutable-in-let-chains.rs:13:8
+  --> $DIR/irrefutable-in-let-chains.rs:13:12
    |
 LL |     if let first = &opt {}
-   |        ^^^^^^^^^^^^^^^^
+   |            ^^^^^
    |
    = note: this pattern will always match, so the `if let` is useless
    = help: consider replacing the `if let` with a `let`
    = note: `#[warn(irrefutable_let_patterns)]` on by default
 
 warning: irrefutable `if let` pattern
-  --> $DIR/irrefutable-in-let-chains.rs:30:8
+  --> $DIR/irrefutable-in-let-chains.rs:30:12
    |
 LL |     if let Range { start: local_start, end: _ } = (None..Some(1)) {}
-   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: this pattern will always match, so the `if let` is useless
    = help: consider replacing the `if let` with a `let`
 
 warning: irrefutable `if let` pattern
-  --> $DIR/irrefutable-in-let-chains.rs:37:8
+  --> $DIR/irrefutable-in-let-chains.rs:37:12
    |
 LL |     if let (a, b, c) = (Some(1), Some(1), Some(1)) {}
-   |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |            ^^^^^^^^^
    |
    = note: this pattern will always match, so the `if let` is useless
    = help: consider replacing the `if let` with a `let`
 
 warning: irrefutable `if let` pattern
-  --> $DIR/irrefutable-in-let-chains.rs:49:15
+  --> $DIR/irrefutable-in-let-chains.rs:49:19
    |
 LL |     } else if let x = opt.clone().map(|_| 1) {
-   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                   ^
    |
    = note: this pattern will always match, so the `if let` is useless
    = help: consider replacing the `if let` with a `let`
 
 warning: irrefutable `if let` guard pattern
-  --> $DIR/irrefutable-in-let-chains.rs:70:28
+  --> $DIR/irrefutable-in-let-chains.rs:70:32
    |
 LL |         Some(ref first) if let second = first => {}
-   |                            ^^^^^^^^^^^^^^^^^^
+   |                                ^^^^^^
    |
    = note: this pattern will always match, so the guard is useless
    = help: consider removing the guard and adding a `let` inside the match arm
 
 warning: irrefutable `while let` pattern
-  --> $DIR/irrefutable-in-let-chains.rs:99:11
+  --> $DIR/irrefutable-in-let-chains.rs:99:15
    |
 LL |     while let first = &opt {}
-   |           ^^^^^^^^^^^^^^^^
+   |               ^^^^^
    |
    = note: this pattern will always match, so the loop will never exit
    = help: consider instead using a `loop { ... }` with a `let` inside it

tests/ui/closures/2229_closure_analysis/issue-88118-2.stderr

@@ -1,8 +1,8 @@
 warning: irrefutable `if let` guard pattern
-  --> $DIR/issue-88118-2.rs:9:25
+  --> $DIR/issue-88118-2.rs:9:29
    |
 LL |             Registry if let _ = registry.try_find_description() => { }
-   |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                             ^
    |
    = note: this pattern will always match, so the guard is useless
    = help: consider removing the guard and adding a `let` inside the match arm

tests/ui/closures/2229_closure_analysis/migrations/issue-78720.stderr

@@ -1,8 +1,8 @@
 warning: irrefutable `if let` pattern
-  --> $DIR/issue-78720.rs:7:8
+  --> $DIR/issue-78720.rs:7:12
    |
 LL |     if let a = "" {
-   |        ^^^^^^^^^^
+   |            ^
    |
    = note: this pattern will always match, so the `if let` is useless
    = help: consider replacing the `if let` with a `let`

tests/ui/expr/if/if-let.stderr

@@ -30,37 +30,37 @@ LL | |     });
    = note: this warning originates in the macro `foo` which comes from the expansion of the macro `bar` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 warning: irrefutable `if let` pattern
-  --> $DIR/if-let.rs:26:8
+  --> $DIR/if-let.rs:26:12
    |
 LL |     if let a = 1 {
-   |        ^^^^^^^^^
+   |            ^
    |
    = note: this pattern will always match, so the `if let` is useless
    = help: consider replacing the `if let` with a `let`
 
 warning: irrefutable `if let` pattern
-  --> $DIR/if-let.rs:30:8
+  --> $DIR/if-let.rs:30:12
    |
 LL |     if let a = 1 {
-   |        ^^^^^^^^^
+   |            ^
    |
    = note: this pattern will always match, so the `if let` is useless
    = help: consider replacing the `if let` with a `let`
 
 warning: irrefutable `if let` pattern
-  --> $DIR/if-let.rs:40:15
+  --> $DIR/if-let.rs:40:19
    |
 LL |     } else if let a = 1 {
-   |               ^^^^^^^^^
+   |                   ^
    |
    = note: this pattern will always match, so the `if let` is useless
    = help: consider replacing the `if let` with a `let`
 
 warning: irrefutable `if let` pattern
-  --> $DIR/if-let.rs:46:15
+  --> $DIR/if-let.rs:46:19
    |
 LL |     } else if let a = 1 {
-   |               ^^^^^^^^^
+   |                   ^
    |
    = note: this pattern will always match, so the `if let` is useless
    = help: consider replacing the `if let` with a `let`

tests/ui/for-loop-while/while-let-2.stderr

@@ -30,10 +30,10 @@ LL | |     });
    = note: this warning originates in the macro `foo` which comes from the expansion of the macro `bar` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 warning: irrefutable `while let` pattern
-  --> $DIR/while-let-2.rs:27:11
+  --> $DIR/while-let-2.rs:27:15
    |
 LL |     while let _a = 1 {
-   |           ^^^^^^^^^^
+   |               ^^
    |
    = note: this pattern will always match, so the loop will never exit
    = help: consider instead using a `loop { ... }` with a `let` inside it

tests/ui/let-else/let-else-irrefutable-152938.rs

@@ -6,7 +6,7 @@
 
 pub fn say_hello(name: Option<String>) {
     let name_str = Some(name) else { return; };
-    //~^ WARN irrefutable `let...else` pattern
+    //~^ WARN unreachable `else` clause
     drop(name_str);
 }
 

tests/ui/let-else/let-else-irrefutable-152938.stderr

@@ -1,16 +1,18 @@
-warning: irrefutable `let...else` pattern
-  --> $DIR/let-else-irrefutable-152938.rs:8:5
+warning: unreachable `else` clause
+  --> $DIR/let-else-irrefutable-152938.rs:8:31
    |
 LL |     let name_str = Some(name) else { return; };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |         --------              ^^^^
+   |         |
+   |         assigning to binding pattern will always succeed
    |
    = note: this pattern always matches, so the else clause is unreachable
-help: remove this `else` block
-  --> $DIR/let-else-irrefutable-152938.rs:8:36
-   |
-LL |     let name_str = Some(name) else { return; };
-   |                                    ^^^^^^^^^^^
    = note: `#[warn(irrefutable_let_patterns)]` on by default
+help: consider using `let Some(name_str) = name` to match on a specific variant
+   |
+LL -     let name_str = Some(name) else { return; };
+LL +     let Some(name_str) = name else { return; };
+   |
 
 warning: 1 warning emitted
 

tests/ui/let-else/let-else-irrefutable.rs

@@ -1,10 +1,18 @@
 //@ check-pass
 
 fn main() {
-    let x = 1 else { return }; //~ WARN irrefutable `let...else` pattern
+    let x = 1 else { return }; //~ WARN unreachable `else` clause
 
     // Multiline else blocks should not get printed
-    let x = 1 else { //~ WARN irrefutable `let...else` pattern
+    let x = 1 else { //~ WARN unreachable `else` clause
+        eprintln!("problem case encountered");
+        return
+    };
+
+    let case = Some("a");
+    let name = Some(case) else {
+        //~^ WARN unreachable `else` clause
+        //~| HELP consider using `let Some(name) = case` to match on a specific variant
         eprintln!("problem case encountered");
         return
     };