From 5d2a2a1caa1a12a5c445fcc99e2dee42e5a5f3d7 Mon Sep 17 00:00:00 2001 From: Camelid Date: Sat, 5 Dec 2020 13:12:39 -0800 Subject: [PATCH] Add explanations and suggestions to `irrefutable_let_patterns` lint --- compiler/rustc_lint_defs/src/builtin.rs | 8 ++--- .../src/thir/pattern/check_match.rs | 35 ++++++++++++++----- ...e-origin-single-variant-diagnostics.stderr | 2 ++ src/test/ui/expr/if/if-let.stderr | 16 +++++++++ .../deny-irrefutable-let-patterns.rs | 8 +++++ .../deny-irrefutable-let-patterns.stderr | 22 +++++++++--- .../ui/rfc-2294-if-let-guard/warns.stderr | 4 ++- src/test/ui/while-let.stderr | 7 ++++ 8 files changed, 83 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 8eeee19cc29..686d09dd7fc 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -1814,14 +1814,12 @@ declare_lint! { } declare_lint! { - /// The `irrefutable_let_patterns` lint detects detects [irrefutable - /// patterns] in [`if let`] and [`while let`] statements. - /// - /// + /// The `irrefutable_let_patterns` lint detects [irrefutable patterns] + /// in [`if let`]s, [`while let`]s, and `if let` guards. /// /// ### Example /// - /// ```rust + /// ``` /// if let _ = 123 { /// println!("always runs!"); /// } 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 e928f3c5d4d..fdecbb94788 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -366,14 +366,31 @@ fn unreachable_pattern(tcx: TyCtxt<'_>, span: Span, id: HirId, catchall: Option< } fn irrefutable_let_pattern(tcx: TyCtxt<'_>, span: Span, id: HirId, source: hir::MatchSource) { - tcx.struct_span_lint_hir(IRREFUTABLE_LET_PATTERNS, id, span, |lint| { - let msg = match source { - hir::MatchSource::IfLetDesugar { .. } => "irrefutable `if let` pattern", - hir::MatchSource::WhileLetDesugar => "irrefutable `while let` pattern", - hir::MatchSource::IfLetGuardDesugar => "irrefutable `if let` guard", - _ => bug!(), - }; - lint.build(msg).emit() + tcx.struct_span_lint_hir(IRREFUTABLE_LET_PATTERNS, id, span, |lint| match source { + hir::MatchSource::IfLetDesugar { .. } => { + let mut diag = lint.build("irrefutable `if let` pattern"); + diag.note("this pattern will always match, so the `if let` is useless"); + diag.help("consider replacing the `if let` with a `let`"); + diag.emit() + } + hir::MatchSource::WhileLetDesugar => { + let mut diag = lint.build("irrefutable `while let` pattern"); + diag.note("this pattern will always match, so the loop will never exit"); + diag.help("consider instead using a `loop { ... }` with a `let` inside it"); + diag.emit() + } + hir::MatchSource::IfLetGuardDesugar => { + let mut diag = lint.build("irrefutable `if let` guard pattern"); + diag.note("this pattern will always match, so the guard is useless"); + diag.help("consider removing the guard and adding a `let` inside the match arm"); + diag.emit() + } + _ => { + bug!( + "expected `if let`, `while let`, or `if let` guard HIR match source, found {:?}", + source, + ) + } }); } @@ -387,7 +404,7 @@ fn check_if_let_guard<'p, 'tcx>( report_arm_reachability(&cx, &report, hir::MatchSource::IfLetGuardDesugar); if report.non_exhaustiveness_witnesses.is_empty() { - // The match is exhaustive, i.e. the if let pattern is irrefutable. + // The match is exhaustive, i.e. the `if let` pattern is irrefutable. irrefutable_let_pattern(cx.tcx, pat.span, pat_id, hir::MatchSource::IfLetGuardDesugar) } } diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr index 5c7a56c7ced..8586dfd9186 100644 --- a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr @@ -17,6 +17,8 @@ LL | | } | |_________^ | = note: `#[warn(irrefutable_let_patterns)]` on by default + = note: this pattern will always match, so the `if let` is useless + = help: consider replacing the `if let` with a `let` error[E0382]: use of moved value: `c` --> $DIR/closure-origin-single-variant-diagnostics.rs:25:13 diff --git a/src/test/ui/expr/if/if-let.stderr b/src/test/ui/expr/if/if-let.stderr index 468e913a773..c64c9093ee5 100644 --- a/src/test/ui/expr/if/if-let.stderr +++ b/src/test/ui/expr/if/if-let.stderr @@ -10,6 +10,8 @@ LL | | }); | |_______- in this macro invocation | = note: `#[warn(irrefutable_let_patterns)]` on by default + = note: this pattern will always match, so the `if let` is useless + = help: consider replacing the `if let` with a `let` = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) warning: irrefutable `if let` pattern @@ -23,6 +25,8 @@ LL | | println!("irrefutable pattern"); LL | | }); | |_______- in this macro invocation | + = note: this pattern will always match, so the `if let` is useless + = help: consider replacing the `if let` with a `let` = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) warning: irrefutable `if let` pattern @@ -32,6 +36,9 @@ LL | / if let a = 1 { LL | | println!("irrefutable pattern"); LL | | } | |_____^ + | + = 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:5 @@ -44,6 +51,9 @@ LL | | } else { LL | | println!("else in irrefutable `if let`"); LL | | } | |_____^ + | + = 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:12 @@ -53,6 +63,9 @@ LL | } else if let a = 1 { LL | | println!("irrefutable pattern"); LL | | } | |_____^ + | + = 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:12 @@ -62,6 +75,9 @@ LL | } else if let a = 1 { LL | | println!("irrefutable pattern"); LL | | } | |_____^ + | + = note: this pattern will always match, so the `if let` is useless + = help: consider replacing the `if let` with a `let` warning: 6 warnings emitted diff --git a/src/test/ui/pattern/usefulness/deny-irrefutable-let-patterns.rs b/src/test/ui/pattern/usefulness/deny-irrefutable-let-patterns.rs index c1cfa4695c9..7549eae7016 100644 --- a/src/test/ui/pattern/usefulness/deny-irrefutable-let-patterns.rs +++ b/src/test/ui/pattern/usefulness/deny-irrefutable-let-patterns.rs @@ -1,3 +1,6 @@ +#![feature(if_let_guard)] +#![allow(incomplete_features)] + #![deny(irrefutable_let_patterns)] fn main() { @@ -6,4 +9,9 @@ fn main() { while let _ = 5 { //~ ERROR irrefutable `while let` pattern break; } + + match 5 { + _ if let _ = 2 => {} //~ ERROR irrefutable `if let` guard pattern + _ => {} + } } diff --git a/src/test/ui/pattern/usefulness/deny-irrefutable-let-patterns.stderr b/src/test/ui/pattern/usefulness/deny-irrefutable-let-patterns.stderr index 1de30f7db06..d6926ee12ee 100644 --- a/src/test/ui/pattern/usefulness/deny-irrefutable-let-patterns.stderr +++ b/src/test/ui/pattern/usefulness/deny-irrefutable-let-patterns.stderr @@ -1,22 +1,36 @@ error: irrefutable `if let` pattern - --> $DIR/deny-irrefutable-let-patterns.rs:4:5 + --> $DIR/deny-irrefutable-let-patterns.rs:7:5 | LL | if let _ = 5 {} | ^^^^^^^^^^^^^^^ | note: the lint level is defined here - --> $DIR/deny-irrefutable-let-patterns.rs:1:9 + --> $DIR/deny-irrefutable-let-patterns.rs:4:9 | LL | #![deny(irrefutable_let_patterns)] | ^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this pattern will always match, so the `if let` is useless + = help: consider replacing the `if let` with a `let` error: irrefutable `while let` pattern - --> $DIR/deny-irrefutable-let-patterns.rs:6:5 + --> $DIR/deny-irrefutable-let-patterns.rs:9:5 | LL | / while let _ = 5 { LL | | break; LL | | } | |_____^ + | + = note: this pattern will always match, so the loop will never exit + = help: consider instead using a `loop { ... }` with a `let` inside it -error: aborting due to 2 previous errors +error: irrefutable `if let` guard pattern + --> $DIR/deny-irrefutable-let-patterns.rs:14:18 + | +LL | _ if let _ = 2 => {} + | ^ + | + = note: this pattern will always match, so the guard is useless + = help: consider removing the guard and adding a `let` inside the match arm + +error: aborting due to 3 previous errors diff --git a/src/test/ui/rfc-2294-if-let-guard/warns.stderr b/src/test/ui/rfc-2294-if-let-guard/warns.stderr index 33fa25d32fb..c7627f1c3c5 100644 --- a/src/test/ui/rfc-2294-if-let-guard/warns.stderr +++ b/src/test/ui/rfc-2294-if-let-guard/warns.stderr @@ -1,4 +1,4 @@ -error: irrefutable `if let` guard +error: irrefutable `if let` guard pattern --> $DIR/warns.rs:7:24 | LL | Some(x) if let () = x => {} @@ -9,6 +9,8 @@ note: the lint level is defined here | LL | #[deny(irrefutable_let_patterns)] | ^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this pattern will always match, so the guard is useless + = help: consider removing the guard and adding a `let` inside the match arm error: unreachable pattern --> $DIR/warns.rs:16:25 diff --git a/src/test/ui/while-let.stderr b/src/test/ui/while-let.stderr index 6538b9fbe6f..04e77bf9470 100644 --- a/src/test/ui/while-let.stderr +++ b/src/test/ui/while-let.stderr @@ -10,6 +10,8 @@ LL | | }); | |_______- in this macro invocation | = note: `#[warn(irrefutable_let_patterns)]` on by default + = note: this pattern will always match, so the loop will never exit + = help: consider instead using a `loop { ... }` with a `let` inside it = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) warning: irrefutable `while let` pattern @@ -23,6 +25,8 @@ LL | | println!("irrefutable pattern"); LL | | }); | |_______- in this macro invocation | + = note: this pattern will always match, so the loop will never exit + = help: consider instead using a `loop { ... }` with a `let` inside it = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) warning: irrefutable `while let` pattern @@ -33,6 +37,9 @@ LL | | println!("irrefutable pattern"); LL | | break; LL | | } | |_____^ + | + = note: this pattern will always match, so the loop will never exit + = help: consider instead using a `loop { ... }` with a `let` inside it warning: 3 warnings emitted