From 459821b1914248180cac691128dd35d945bfde88 Mon Sep 17 00:00:00 2001 From: tamaron Date: Sat, 2 Jul 2022 12:57:57 +0900 Subject: [PATCH 1/3] fix --- clippy_lints/src/matches/needless_match.rs | 4 ++- tests/ui/needless_match.fixed | 26 +++++++++++++++++ tests/ui/needless_match.rs | 33 ++++++++++++++++++++++ tests/ui/needless_match.stderr | 23 ++++++++++++++- 4 files changed, 84 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/matches/needless_match.rs b/clippy_lints/src/matches/needless_match.rs index fa19cddd35e..53a4a91d0e7 100644 --- a/clippy_lints/src/matches/needless_match.rs +++ b/clippy_lints/src/matches/needless_match.rs @@ -66,7 +66,9 @@ fn check_all_arms(cx: &LateContext<'_>, match_expr: &Expr<'_>, arms: &[Arm<'_>]) for arm in arms { let arm_expr = peel_blocks_with_stmt(arm.body); 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..9d4427f1df2 100644 --- a/tests/ui/needless_match.fixed +++ b/tests/ui/needless_match.fixed @@ -207,4 +207,30 @@ impl Tr for Result { } } +mod issue9084 { + fn wildcard_if() { + let 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, + }; + } +} + fn main() {} diff --git a/tests/ui/needless_match.rs b/tests/ui/needless_match.rs index f66f01d7cca..cae850fb059 100644 --- a/tests/ui/needless_match.rs +++ b/tests/ui/needless_match.rs @@ -244,4 +244,37 @@ fn as_mut(&mut self) -> Result<&mut i32, &mut i32> { } } +mod issue9084 { + fn wildcard_if() { + let 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, + }; + } +} + fn main() {} diff --git a/tests/ui/needless_match.stderr b/tests/ui/needless_match.stderr index 5bc79800a1a..a173bbe4d77 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:252: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:258: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 From 45084eeefb4edb981ce437c7fee6a07e61ff224b Mon Sep 17 00:00:00 2001 From: tamaron Date: Thu, 11 Aug 2022 12:00:46 +0900 Subject: [PATCH 2/3] give up when gurad has side effects --- clippy_lints/src/matches/needless_match.rs | 18 +++++++++++++++++- tests/ui/needless_match.fixed | 15 ++++++++++++++- tests/ui/needless_match.rs | 15 ++++++++++++++- 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/matches/needless_match.rs b/clippy_lints/src/matches/needless_match.rs index 53a4a91d0e7..6f037339ec7 100644 --- a/clippy_lints/src/matches/needless_match.rs +++ b/clippy_lints/src/matches/needless_match.rs @@ -8,7 +8,7 @@ }; 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,6 +65,22 @@ 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 { if !eq_expr_value(cx, match_expr, strip_return(arm_expr)) { return false; diff --git a/tests/ui/needless_match.fixed b/tests/ui/needless_match.fixed index 9d4427f1df2..7e47406798c 100644 --- a/tests/ui/needless_match.fixed +++ b/tests/ui/needless_match.fixed @@ -209,7 +209,7 @@ impl Tr for Result { mod issue9084 { fn wildcard_if() { - let some_bool = true; + let mut some_bool = true; let e = Some(1); // should lint @@ -230,6 +230,19 @@ mod issue9084 { _ 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, + }; } } diff --git a/tests/ui/needless_match.rs b/tests/ui/needless_match.rs index cae850fb059..809c694bf40 100644 --- a/tests/ui/needless_match.rs +++ b/tests/ui/needless_match.rs @@ -246,7 +246,7 @@ fn as_mut(&mut self) -> Result<&mut i32, &mut i32> { mod issue9084 { fn wildcard_if() { - let some_bool = true; + let mut some_bool = true; let e = Some(1); // should lint @@ -274,6 +274,19 @@ fn wildcard_if() { _ 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, + }; } } From f7a376e4fce60826582efa91c3cfdce80158399f Mon Sep 17 00:00:00 2001 From: tamaron Date: Sun, 21 Aug 2022 17:26:39 +0900 Subject: [PATCH 3/3] Update needless_match.stderr --- tests/ui/needless_match.stderr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ui/needless_match.stderr b/tests/ui/needless_match.stderr index a173bbe4d77..28e78441c25 100644 --- a/tests/ui/needless_match.stderr +++ b/tests/ui/needless_match.stderr @@ -110,7 +110,7 @@ LL | | }; | |_________^ help: replace it with: `ce` error: this match expression is unnecessary - --> $DIR/needless_match.rs:252:17 + --> $DIR/needless_match.rs:253:17 | LL | let _ = match e { | _________________^ @@ -120,7 +120,7 @@ LL | | }; | |_________^ help: replace it with: `e` error: this match expression is unnecessary - --> $DIR/needless_match.rs:258:17 + --> $DIR/needless_match.rs:259:17 | LL | let _ = match e { | _________________^