diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs index 793d9ef18eb61..b7f4fc093b6ba 100644 --- a/compiler/rustc_mir_build/src/errors.rs +++ b/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, + #[subdiagnostic] + pub(crate) be_replaced: Option, +} + +#[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 LetElseReplacementSuggestion { + #[primary_span] + pub(crate) span: Span, + pub(crate) lhs: String, + pub(crate) rhs: String, } #[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 baabc1afe3fac..3888a297c891c 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/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(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 { + 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(LetElseReplacementSuggestion { 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) { + fn lint_single_let( + &mut self, + let_span: Span, + else_keyword_span: Option, + let_else_span: Option, + ) { 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, + else_keyword_span: Option, + let_else_span: Option, ) { 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), diff --git a/tests/ui/let-else/let-else-irrefutable-152938.rs b/tests/ui/let-else/let-else-irrefutable-152938.rs index 6e0ffe3fb8706..25e3e0f6e5708 100644 --- a/tests/ui/let-else/let-else-irrefutable-152938.rs +++ b/tests/ui/let-else/let-else-irrefutable-152938.rs @@ -6,7 +6,7 @@ pub fn say_hello(name: Option) { let name_str = Some(name) else { return; }; - //~^ WARN irrefutable `let...else` pattern + //~^ WARN unreachable `else` clause drop(name_str); } diff --git a/tests/ui/let-else/let-else-irrefutable-152938.stderr b/tests/ui/let-else/let-else-irrefutable-152938.stderr index 57632964be9b3..e32f4960a3350 100644 --- a/tests/ui/let-else/let-else-irrefutable-152938.stderr +++ b/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 diff --git a/tests/ui/let-else/let-else-irrefutable.rs b/tests/ui/let-else/let-else-irrefutable.rs index f9675ff5938eb..59de726d38ddf 100644 --- a/tests/ui/let-else/let-else-irrefutable.rs +++ b/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 }; diff --git a/tests/ui/let-else/let-else-irrefutable.stderr b/tests/ui/let-else/let-else-irrefutable.stderr index d36d227b4156a..12338789d2a03 100644 --- a/tests/ui/let-else/let-else-irrefutable.stderr +++ b/tests/ui/let-else/let-else-irrefutable.stderr @@ -1,33 +1,38 @@ -warning: irrefutable `let...else` pattern - --> $DIR/let-else-irrefutable.rs:4:5 +warning: unreachable `else` clause + --> $DIR/let-else-irrefutable.rs:4:15 | LL | let x = 1 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.rs:4:20 - | -LL | let x = 1 else { return }; - | ^^^^^^^^^^ = note: `#[warn(irrefutable_let_patterns)]` on by default -warning: irrefutable `let...else` pattern - --> $DIR/let-else-irrefutable.rs:7:5 +warning: unreachable `else` clause + --> $DIR/let-else-irrefutable.rs:7:15 | LL | let x = 1 else { - | ^^^^^^^^^ + | --------- ^^^^ + | | + | assigning to binding pattern will always succeed + | + = note: this pattern always matches, so the else clause is unreachable + +warning: unreachable `else` clause + --> $DIR/let-else-irrefutable.rs:13:27 + | +LL | let name = Some(case) else { + | --------------------- ^^^^ + | | + | 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.rs:7:20 - | -LL | let x = 1 else { - | ____________________^ -LL | | eprintln!("problem case encountered"); -LL | | return -LL | | }; - | |_____^ +help: consider using `let Some(name) = case` to match on a specific variant + | +LL - let name = Some(case) else { +LL + let Some(name) = case else { + | -warning: 2 warnings emitted +warning: 3 warnings emitted diff --git a/tests/ui/parser/bad-let-else-statement.rs b/tests/ui/parser/bad-let-else-statement.rs index 3ede26dbcd06c..a552fbcfc1fc3 100644 --- a/tests/ui/parser/bad-let-else-statement.rs +++ b/tests/ui/parser/bad-let-else-statement.rs @@ -93,10 +93,10 @@ fn i() { fn j() { let mut bar = 0; let foo = bar = { - //~^ WARN irrefutable `let...else` pattern 1 } else { - //~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed + //~^ WARN unreachable `else` clause + //~| ERROR right curly brace `}` before `else` in a `let...else` statement not allowed return; }; } @@ -158,21 +158,21 @@ fn o() -> Result<(), ()> { fn q() { let foo = |x: i32| { - //~^ WARN irrefutable `let...else` pattern x } else { - //~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed + //~^ WARN unreachable `else` clause + //~| ERROR right curly brace `}` before `else` in a `let...else` statement not allowed return; }; } fn r() { let ok = format_args!("") else { return; }; - //~^ WARN irrefutable `let...else` pattern + //~^ WARN unreachable `else` clause let bad = format_args! {""} else { return; }; //~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed - //~| WARN irrefutable `let...else` pattern + //~| WARN unreachable `else` clause } fn s() { @@ -202,10 +202,10 @@ fn t() { } let foo = &std::ptr::null as &'static dyn std::ops::Fn() -> *const primitive! { - //~^ WARN irrefutable `let...else` pattern 8 } else { - //~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed + //~^ WARN unreachable `else` clause + //~| ERROR right curly brace `}` before `else` in a `let...else` statement not allowed return; }; } diff --git a/tests/ui/parser/bad-let-else-statement.stderr b/tests/ui/parser/bad-let-else-statement.stderr index 8545d95f507f6..76fbbbb8c1e10 100644 --- a/tests/ui/parser/bad-let-else-statement.stderr +++ b/tests/ui/parser/bad-let-else-statement.stderr @@ -125,7 +125,7 @@ LL ~ }) else { | error: right curly brace `}` before `else` in a `let...else` statement not allowed - --> $DIR/bad-let-else-statement.rs:98:5 + --> $DIR/bad-let-else-statement.rs:97:5 | LL | } else { | ^ @@ -133,7 +133,6 @@ LL | } else { help: wrap the expression in parentheses | LL ~ let foo = bar = ({ -LL | LL | 1 LL ~ }) else { | @@ -204,7 +203,7 @@ LL ~ }) else { | error: right curly brace `}` before `else` in a `let...else` statement not allowed - --> $DIR/bad-let-else-statement.rs:163:5 + --> $DIR/bad-let-else-statement.rs:162:5 | LL | } else { | ^ @@ -212,7 +211,6 @@ LL | } else { help: wrap the expression in parentheses | LL ~ let foo = |x: i32| ({ -LL | LL | x LL ~ }) else { | @@ -230,7 +228,7 @@ LL + let bad = format_args! ("") else { return; }; | error: right curly brace `}` before `else` in a `let...else` statement not allowed - --> $DIR/bad-let-else-statement.rs:207:5 + --> $DIR/bad-let-else-statement.rs:206:5 | LL | } else { | ^ @@ -238,7 +236,6 @@ LL | } else { help: use parentheses instead of braces for this macro | LL ~ let foo = &std::ptr::null as &'static dyn std::ops::Fn() -> *const primitive! ( -LL | LL | 8 LL ~ ) else { | @@ -259,92 +256,62 @@ LL - let 0 = a! {} else { return; }; LL + let 0 = a! () else { return; }; | -warning: irrefutable `let...else` pattern - --> $DIR/bad-let-else-statement.rs:95:5 +warning: unreachable `else` clause + --> $DIR/bad-let-else-statement.rs:97:7 | LL | / let foo = bar = { -LL | | LL | | 1 LL | | } else { - | |_____^ + | | - ^^^^ + | |_____| + | assigning to binding pattern will always succeed | = 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 - --> $DIR/bad-let-else-statement.rs:160:5 +warning: unreachable `else` clause + --> $DIR/bad-let-else-statement.rs:162:7 | LL | / let foo = |x: i32| { -LL | | LL | | x LL | | } else { - | |_____^ + | | - ^^^^ + | |_____| + | assigning to binding pattern will always succeed | = 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 +warning: unreachable `else` clause + --> $DIR/bad-let-else-statement.rs:170:31 | LL | let ok = format_args!("") 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/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 +warning: unreachable `else` clause + --> $DIR/bad-let-else-statement.rs:173:33 | LL | let bad = format_args! {""} 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/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 +warning: unreachable `else` clause + --> $DIR/bad-let-else-statement.rs:206:7 | LL | / let foo = &std::ptr::null as &'static dyn std::ops::Fn() -> *const primitive! { -LL | | LL | | 8 LL | | } else { - | |_____^ + | | - ^^^^ + | |_____| + | assigning to binding pattern will always succeed | = 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.rs b/tests/ui/span/let-offset-of.rs index 99b34a1928470..eeecf38e6b6a9 100644 --- a/tests/ui/span/let-offset-of.rs +++ b/tests/ui/span/let-offset-of.rs @@ -15,5 +15,5 @@ fn init_to_offset_of() { //~^ WARN irrefutable `if let` pattern let x = offset_of!(Foo, field) else { return; }; - //~^ WARN irrefutable `let...else` pattern + //~^ WARN unreachable `else` clause } diff --git a/tests/ui/span/let-offset-of.stderr b/tests/ui/span/let-offset-of.stderr index df9b1e695b1e4..afcf8a8103d9d 100644 --- a/tests/ui/span/let-offset-of.stderr +++ b/tests/ui/span/let-offset-of.stderr @@ -8,18 +8,15 @@ LL | if let x = offset_of!(Foo, field) {} = help: consider replacing the `if let` with a `let` = note: `#[warn(irrefutable_let_patterns)]` on by default -warning: irrefutable `let...else` pattern - --> $DIR/let-offset-of.rs:17:5 +warning: unreachable `else` clause + --> $DIR/let-offset-of.rs:17:36 | LL | let x = offset_of!(Foo, field) 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-offset-of.rs:17:41 - | -LL | let x = offset_of!(Foo, field) else { return; }; - | ^^^^^^^^^^^ warning: 2 warnings emitted