diff --git a/clippy_lints/src/matches/needless_match.rs b/clippy_lints/src/matches/needless_match.rs index fa19cddd35e..6f037339ec7 100644 --- a/clippy_lints/src/matches/needless_match.rs +++ b/clippy_lints/src/matches/needless_match.rs @@ -8,7 +8,7 @@ use clippy_utils::{ }; use rustc_errors::Applicability; use rustc_hir::LangItem::OptionNone; -use rustc_hir::{Arm, BindingAnnotation, Expr, ExprKind, FnRetTy, Node, Pat, PatKind, Path, QPath}; +use rustc_hir::{Arm, BindingAnnotation, Expr, ExprKind, FnRetTy, Guard, Node, Pat, PatKind, Path, QPath}; use rustc_lint::LateContext; use rustc_span::sym; use rustc_typeck::hir_ty_to_ty; @@ -65,8 +65,26 @@ pub(crate) fn check_if_let<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'_>, if_let: fn check_all_arms(cx: &LateContext<'_>, match_expr: &Expr<'_>, arms: &[Arm<'_>]) -> bool { for arm in arms { let arm_expr = peel_blocks_with_stmt(arm.body); + + if let Some(guard_expr) = &arm.guard { + match guard_expr { + // gives up if `pat if expr` can have side effects + Guard::If(if_cond) => { + if if_cond.can_have_side_effects() { + return false; + } + }, + // gives up `pat if let ...` arm + Guard::IfLet(_) => { + return false; + }, + }; + } + if let PatKind::Wild = arm.pat.kind { - return eq_expr_value(cx, match_expr, strip_return(arm_expr)); + if !eq_expr_value(cx, match_expr, strip_return(arm_expr)) { + return false; + } } else if !pat_same_as_expr(arm.pat, arm_expr) { return false; } diff --git a/tests/ui/needless_match.fixed b/tests/ui/needless_match.fixed index 0c9178fb85e..7e47406798c 100644 --- a/tests/ui/needless_match.fixed +++ b/tests/ui/needless_match.fixed @@ -207,4 +207,43 @@ impl Tr for Result { } } +mod issue9084 { + fn wildcard_if() { + let mut some_bool = true; + let e = Some(1); + + // should lint + let _ = e; + + // should lint + let _ = e; + + // should not lint + let _ = match e { + _ if some_bool => e, + _ => Some(2), + }; + + // should not lint + let _ = match e { + Some(i) => Some(i + 1), + _ if some_bool => e, + _ => e, + }; + + // should not lint (guard has side effects) + let _ = match e { + Some(i) => Some(i), + _ if { + some_bool = false; + some_bool + } => + { + e + }, + _ => e, + }; + } +} + fn main() {} diff --git a/tests/ui/needless_match.rs b/tests/ui/needless_match.rs index f66f01d7cca..809c694bf40 100644 --- a/tests/ui/needless_match.rs +++ b/tests/ui/needless_match.rs @@ -244,4 +244,50 @@ impl Tr for Result { } } +mod issue9084 { + fn wildcard_if() { + let mut some_bool = true; + let e = Some(1); + + // should lint + let _ = match e { + _ if some_bool => e, + _ => e, + }; + + // should lint + let _ = match e { + Some(i) => Some(i), + _ if some_bool => e, + _ => e, + }; + + // should not lint + let _ = match e { + _ if some_bool => e, + _ => Some(2), + }; + + // should not lint + let _ = match e { + Some(i) => Some(i + 1), + _ if some_bool => e, + _ => e, + }; + + // should not lint (guard has side effects) + let _ = match e { + Some(i) => Some(i), + _ if { + some_bool = false; + some_bool + } => + { + e + }, + _ => e, + }; + } +} + fn main() {} diff --git a/tests/ui/needless_match.stderr b/tests/ui/needless_match.stderr index 5bc79800a1a..28e78441c25 100644 --- a/tests/ui/needless_match.stderr +++ b/tests/ui/needless_match.stderr @@ -109,5 +109,26 @@ LL | | Complex::D(E::VariantB(ea, eb), b) => Complex::D(E::VariantB( LL | | }; | |_________^ help: replace it with: `ce` -error: aborting due to 11 previous errors +error: this match expression is unnecessary + --> $DIR/needless_match.rs:253:17 + | +LL | let _ = match e { + | _________________^ +LL | | _ if some_bool => e, +LL | | _ => e, +LL | | }; + | |_________^ help: replace it with: `e` + +error: this match expression is unnecessary + --> $DIR/needless_match.rs:259:17 + | +LL | let _ = match e { + | _________________^ +LL | | Some(i) => Some(i), +LL | | _ if some_bool => e, +LL | | _ => e, +LL | | }; + | |_________^ help: replace it with: `e` + +error: aborting due to 13 previous errors