Improve irrefutable let-else lint wording

compiler/rustc_mir_build/src/errors.rs

@@ -932,13 +932,14 @@ pub(crate) struct IrrefutableLetPatternsIfLetGuard {
 )]
 #[note(
     "{$count ->
-    [one] this pattern
-    *[other] these patterns
-} will always match, so the `else` clause is useless"
+    [one] this pattern always matches, so the else clause is unreachable
+    *[other] these patterns always match, so the else clause is unreachable
+}"
 )]
-#[help("consider removing the `else` clause")]
 pub(crate) struct IrrefutableLetPatternsLetElse {
     pub(crate) count: usize,
+    #[help("remove this `else` block")]
+    pub(crate) else_span: Option<Span>,
 }
 
 #[derive(Diagnostic)]

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

@@ -160,7 +160,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) =>
@@ -169,7 +169,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);
+                    self.lint_single_let(ex.span, None);
                 }
                 return;
             }
@@ -184,8 +184,9 @@ impl<'p, 'tcx> Visitor<'p, 'tcx> for MatchVisitor<'p, 'tcx> {
                 self.with_hir_source(hir_id, |this| {
                     let let_source =
                         if else_block.is_some() { LetSource::LetElse } else { LetSource::PlainLet };
+                    let else_span = else_block.map(|bid| this.thir.blocks[bid].span);
                     this.with_let_source(let_source, |this| {
-                        this.check_let(pattern, initializer, span)
+                        this.check_let(pattern, initializer, span, else_span)
                     });
                     visit::walk_stmt(this, stmt);
                 });
@@ -426,13 +427,19 @@ 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,
+        else_span: Option<Span>,
+    ) {
         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));
         } else if let Ok(Irrefutable) = self.is_let_irrefutable(pat, scrut) {
-            self.lint_single_let(span);
+            self.lint_single_let(span, else_span);
         }
     }
 
@@ -540,8 +547,15 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
     }
 
     #[instrument(level = "trace", skip(self))]
-    fn lint_single_let(&mut self, let_span: Span) {
-        report_irrefutable_let_patterns(self.tcx, self.hir_source, self.let_source, 1, let_span);
+    fn lint_single_let(&mut self, let_span: Span, else_span: Option<Span>) {
+        report_irrefutable_let_patterns(
+            self.tcx,
+            self.hir_source,
+            self.let_source,
+            1,
+            let_span,
+            else_span,
+        );
     }
 
     fn analyze_binding(
@@ -836,6 +850,7 @@ fn report_irrefutable_let_patterns(
     source: LetSource,
     count: usize,
     span: Span,
+    else_span: Option<Span>,
 ) {
     macro_rules! emit_diag {
         ($lint:tt) => {{
@@ -847,7 +862,14 @@ fn report_irrefutable_let_patterns(
         LetSource::None | LetSource::PlainLet | LetSource::Else => bug!(),
         LetSource::IfLet | LetSource::ElseIfLet => emit_diag!(IrrefutableLetPatternsIfLet),
         LetSource::IfLetGuard => emit_diag!(IrrefutableLetPatternsIfLetGuard),
-        LetSource::LetElse => emit_diag!(IrrefutableLetPatternsLetElse),
+        LetSource::LetElse => {
+            tcx.emit_node_span_lint(
+                IRREFUTABLE_LET_PATTERNS,
+                id,
+                span,
+                IrrefutableLetPatternsLetElse { count, else_span },
+            );
+        }
         LetSource::WhileLet => emit_diag!(IrrefutableLetPatternsWhileLet),
     }
 }

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

@@ -0,0 +1,13 @@
+//@ check-pass
+
+// Regression test for https://github.com/rust-lang/rust/issues/152938
+// The irrefutable `let...else` diagnostic should explain that the pattern
+// always matches and point at the `else` block for removal.
+
+pub fn say_hello(name: Option<String>) {
+    let name_str = Some(name) else { return; };
+    //~^ WARN irrefutable `let...else` pattern
+    drop(name_str);
+}
+
+fn main() {}

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

@@ -0,0 +1,16 @@
+warning: irrefutable `let...else` pattern
+  --> $DIR/let-else-irrefutable-152938.rs:8:5
+   |
+LL |     let name_str = Some(name) else { return; };
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = 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
+
+warning: 1 warning emitted
+

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

@@ -4,8 +4,12 @@ warning: irrefutable `let...else` pattern
 LL |     let x = 1 else { return };
    |     ^^^^^^^^^
    |
-   = note: this pattern will always match, so the `else` clause is useless
-   = help: consider removing the `else` clause
+   = note: this pattern always matches, so the else clause is unreachable
+help: remove this `else` block
+  --> $DIR/let-else-irrefutable.rs:4:20
+   |
+LL |     let x = 1 else { return };
+   |                    ^^^^^^^^^^
    = note: `#[warn(irrefutable_let_patterns)]` on by default
 
 warning: irrefutable `let...else` pattern
@@ -14,8 +18,16 @@ warning: irrefutable `let...else` pattern
 LL |     let x = 1 else {
    |     ^^^^^^^^^
    |
-   = note: this pattern will always match, so the `else` clause is useless
-   = help: consider removing the `else` clause
+   = note: this pattern always matches, so the else clause is unreachable
+help: remove this `else` block
+  --> $DIR/let-else-irrefutable.rs:7:20
+   |
+LL |       let x = 1 else {
+   |  ____________________^
+LL | |         eprintln!("problem case encountered");
+LL | |         return
+LL | |     };
+   | |_____^
 
 warning: 2 warnings emitted
 

tests/ui/parser/bad-let-else-statement.stderr

@@ -268,8 +268,16 @@ LL | |         1
 LL | |     } else {
    | |_____^
    |
-   = note: this pattern will always match, so the `else` clause is useless
-   = help: consider removing the `else` clause
+   = note: this pattern always matches, so the else clause is unreachable
+help: remove this `else` block
+  --> $DIR/bad-let-else-statement.rs:98:12
+   |
+LL |       } else {
+   |  ____________^
+LL | |
+LL | |         return;
+LL | |     };
+   | |_____^
    = note: `#[warn(irrefutable_let_patterns)]` on by default
 
 warning: irrefutable `let...else` pattern
@@ -281,26 +289,42 @@ LL | |         x
 LL | |     } else {
    | |_____^
    |
-   = note: this pattern will always match, so the `else` clause is useless
-   = help: consider removing the `else` clause
+   = note: this pattern always matches, so the else clause is unreachable
+help: remove this `else` block
+  --> $DIR/bad-let-else-statement.rs:163:12
+   |
+LL |       } else {
+   |  ____________^
+LL | |
+LL | |         return;
+LL | |     };
+   | |_____^
 
 warning: irrefutable `let...else` pattern
   --> $DIR/bad-let-else-statement.rs:170:5
    |
 LL |     let ok = format_args!("") else { return; };
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: this pattern will always match, so the `else` clause is useless
-   = help: consider removing the `else` clause
+   = note: this pattern always matches, so the else clause is unreachable
+help: remove this `else` block
+  --> $DIR/bad-let-else-statement.rs:170:36
+   |
+LL |     let ok = format_args!("") else { return; };
+   |                                    ^^^^^^^^^^^
 
 warning: irrefutable `let...else` pattern
   --> $DIR/bad-let-else-statement.rs:173:5
    |
 LL |     let bad = format_args! {""} else { return; };
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: this pattern will always match, so the `else` clause is useless
-   = help: consider removing the `else` clause
+   = note: this pattern always matches, so the else clause is unreachable
+help: remove this `else` block
+  --> $DIR/bad-let-else-statement.rs:173:38
+   |
+LL |     let bad = format_args! {""} else { return; };
+   |                                      ^^^^^^^^^^^
 
 warning: irrefutable `let...else` pattern
   --> $DIR/bad-let-else-statement.rs:204:5
@@ -311,8 +335,16 @@ LL | |         8
 LL | |     } else {
    | |_____^
    |
-   = note: this pattern will always match, so the `else` clause is useless
-   = help: consider removing the `else` clause
+   = note: this pattern always matches, so the else clause is unreachable
+help: remove this `else` block
+  --> $DIR/bad-let-else-statement.rs:207:12
+   |
+LL |       } else {
+   |  ____________^
+LL | |
+LL | |         return;
+LL | |     };
+   | |_____^
 
 error: aborting due to 19 previous errors; 5 warnings emitted
 

tests/ui/span/let-offset-of.stderr

@@ -14,8 +14,12 @@ warning: irrefutable `let...else` pattern
 LL |     let x = offset_of!(Foo, field) else { return; };
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: this pattern will always match, so the `else` clause is useless
-   = help: consider removing the `else` clause
+   = note: this pattern always matches, so the else clause is unreachable
+help: remove this `else` block
+  --> $DIR/let-offset-of.rs:17:41
+   |
+LL |     let x = offset_of!(Foo, field) else { return; };
+   |                                         ^^^^^^^^^^^
 
 warning: 2 warnings emitted