diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index ccd0fdfcd1e7a..c362badd033ca 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/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, } #[derive(Diagnostic)] diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index f4645604e6efd..e844f1114d61e 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/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, span: Span) { + fn check_let( + &mut self, + pat: &'p Pat<'tcx>, + scrutinee: Option, + span: Span, + else_span: Option, + ) { 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) { + 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, ) { 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), } } diff --git a/tests/ui/let-else/let-else-irrefutable-152938.rs b/tests/ui/let-else/let-else-irrefutable-152938.rs new file mode 100644 index 0000000000000..6e0ffe3fb8706 --- /dev/null +++ b/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) { + let name_str = Some(name) else { return; }; + //~^ WARN irrefutable `let...else` pattern + drop(name_str); +} + +fn main() {} diff --git a/tests/ui/let-else/let-else-irrefutable-152938.stderr b/tests/ui/let-else/let-else-irrefutable-152938.stderr new file mode 100644 index 0000000000000..57632964be9b3 --- /dev/null +++ b/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 + diff --git a/tests/ui/let-else/let-else-irrefutable.stderr b/tests/ui/let-else/let-else-irrefutable.stderr index 73d4e5f34831d..d36d227b4156a 100644 --- a/tests/ui/let-else/let-else-irrefutable.stderr +++ b/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 diff --git a/tests/ui/parser/bad-let-else-statement.stderr b/tests/ui/parser/bad-let-else-statement.stderr index ba56452998312..8545d95f507f6 100644 --- a/tests/ui/parser/bad-let-else-statement.stderr +++ b/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,8 +289,16 @@ 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 @@ -290,8 +306,12 @@ warning: irrefutable `let...else` pattern 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 @@ -299,8 +319,12 @@ warning: irrefutable `let...else` pattern 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 diff --git a/tests/ui/span/let-offset-of.stderr b/tests/ui/span/let-offset-of.stderr index 6ea9b2194a36a..df9b1e695b1e4 100644 --- a/tests/ui/span/let-offset-of.stderr +++ b/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