Auto merge of #9092 - tamaroning:fix-needless-match, r=llogiq

Fix false positives of needless_match

closes: #9084
made needless_match take into account arm in the form of `_ if => ...`

changelog: none

Author: bors

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;
         }

tests/ui/needless_match.fixed

@@ -207,4 +207,43 @@ impl Tr for Result<i32, i32> {
     }
 }
 
+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() {}

tests/ui/needless_match.rs

@@ -244,4 +244,50 @@ impl Tr for Result<i32, i32> {
     }
 }
 
+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() {}

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