improve MATCH_LIKE_MATCHES_MACRO lint

rust-lang · Oct 24, 2020 · 4b0a88b · 4b0a88b
1 parent 5e1154e
commit 4b0a88b
Show file tree

Hide file tree

Showing 4 changed files with 140 additions and 13 deletions.
diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
@@ -1078,19 +1078,32 @@ fn check_match_like_matches<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>)
 /// Lint a `match` or desugared `if let` for replacement by `matches!`
 fn find_matches_sugg(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>, desugared: bool) {
     if_chain! {
-        if arms.len() == 2;
+        if arms.len() >= 2;
         if cx.typeck_results().expr_ty(expr).is_bool();
-        if is_wild(&arms[1].pat);
-        if let Some(first) = find_bool_lit(&arms[0].body.kind, desugared);
-        if let Some(second) = find_bool_lit(&arms[1].body.kind, desugared);
-        if first != second;
+        if let Some((b1_arm, b0_arms)) = arms.split_last();
+        if let Some(b0) = find_bool_lit(&b0_arms[0].body.kind, desugared);
+        if let Some(b1) = find_bool_lit(&b1_arm.body.kind, desugared);
+        if is_wild(&b1_arm.pat);
+        if b0 != b1;
+        let if_guard = &b0_arms[0].guard;
+        if if_guard.is_none() || b0_arms.len() == 1;
+        if b0_arms[1..].iter()
+            .all(|arm| {
+                find_bool_lit(&arm.body.kind, desugared).map_or(false, |b| b == b0) &&
+                arm.guard.is_none()
+            });
         then {
             let mut applicability = Applicability::MachineApplicable;
-
-            let pat_and_guard = if let Some(Guard::If(g)) = arms[0].guard {
-                format!("{} if {}", snippet_with_applicability(cx, arms[0].pat.span, "..", &mut applicability), snippet_with_applicability(cx, g.span, "..", &mut applicability))
+            let pat = {
+                use itertools::Itertools as _;
+                b0_arms.iter()
+                    .map(|arm| snippet_with_applicability(cx, arm.pat.span, "..", &mut applicability))
+                    .join(" | ")
+            };
+            let pat_and_guard = if let Some(Guard::If(g)) = if_guard {
+                format!("{} if {}", pat, snippet_with_applicability(cx, g.span, "..", &mut applicability))
             } else {
-                format!("{}", snippet_with_applicability(cx, arms[0].pat.span, "..", &mut applicability))
+                pat
             };
             span_lint_and_sugg(
                 cx,
@@ -1100,7 +1113,7 @@ fn find_matches_sugg(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr
                 "try this",
                 format!(
                     "{}matches!({}, {})",
-                    if first { "" } else { "!" },
+                    if b0 { "" } else { "!" },
                     snippet_with_applicability(cx, ex.span, "..", &mut applicability),
                     pat_and_guard,
                 ),

diff --git a/tests/ui/match_expr_like_matches_macro.fixed b/tests/ui/match_expr_like_matches_macro.fixed
@@ -1,7 +1,7 @@
 // run-rustfix
 
 #![warn(clippy::match_like_matches_macro)]
-#![allow(unreachable_patterns)]
+#![allow(unreachable_patterns, dead_code)]
 
 fn main() {
     let x = Some(5);
@@ -33,4 +33,46 @@ fn main() {
         _ => true,
         None => false,
     };
+
+    enum E {
+        A(u32),
+        B(i32),
+        C,
+        D,
+    };
+    let x = E::A(2);
+    {
+        // lint
+        let _ans = matches!(x, E::A(_) | E::B(_));
+    }
+    {
+        // lint
+        let _ans = !matches!(x, E::B(_) | E::C);
+    }
+    {
+        // no lint
+        let _ans = match x {
+            E::A(_) => false,
+            E::B(_) => false,
+            E::C => true,
+            _ => true,
+        };
+    }
+    {
+        // no lint
+        let _ans = match x {
+            E::A(_) => true,
+            E::B(_) => false,
+            E::C => false,
+            _ => true,
+        };
+    }
+    {
+        // no lint
+        let _ans = match x {
+            E::A(a) if a < 10 => false,
+            E::B(a) if a < 10 => false,
+            _ => true,
+        };
+    }
 }
diff --git a/tests/ui/match_expr_like_matches_macro.rs b/tests/ui/match_expr_like_matches_macro.rs
@@ -1,7 +1,7 @@
 // run-rustfix
 
 #![warn(clippy::match_like_matches_macro)]
-#![allow(unreachable_patterns)]
+#![allow(unreachable_patterns, dead_code)]
 
 fn main() {
     let x = Some(5);
@@ -45,4 +45,54 @@ fn main() {
         _ => true,
         None => false,
     };
+
+    enum E {
+        A(u32),
+        B(i32),
+        C,
+        D,
+    };
+    let x = E::A(2);
+    {
+        // lint
+        let _ans = match x {
+            E::A(_) => true,
+            E::B(_) => true,
+            _ => false,
+        };
+    }
+    {
+        // lint
+        let _ans = match x {
+            E::B(_) => false,
+            E::C => false,
+            _ => true,
+        };
+    }
+    {
+        // no lint
+        let _ans = match x {
+            E::A(_) => false,
+            E::B(_) => false,
+            E::C => true,
+            _ => true,
+        };
+    }
+    {
+        // no lint
+        let _ans = match x {
+            E::A(_) => true,
+            E::B(_) => false,
+            E::C => false,
+            _ => true,
+        };
+    }
+    {
+        // no lint
+        let _ans = match x {
+            E::A(a) if a < 10 => false,
+            E::B(a) if a < 10 => false,
+            _ => true,
+        };
+    }
 }
diff --git a/tests/ui/match_expr_like_matches_macro.stderr b/tests/ui/match_expr_like_matches_macro.stderr
@@ -48,5 +48,27 @@ error: if let .. else expression looks like `matches!` macro
 LL |     let _zzz = if let Some(5) = x { true } else { false };
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `matches!(x, Some(5))`
 
-error: aborting due to 5 previous errors
+error: match expression looks like `matches!` macro
+  --> $DIR/match_expr_like_matches_macro.rs:58:20
+   |
+LL |           let _ans = match x {
+   |  ____________________^
+LL | |             E::A(_) => true,
+LL | |             E::B(_) => true,
+LL | |             _ => false,
+LL | |         };
+   | |_________^ help: try this: `matches!(x, E::A(_) | E::B(_))`
+
+error: match expression looks like `matches!` macro
+  --> $DIR/match_expr_like_matches_macro.rs:66:20
+   |
+LL |           let _ans = match x {
+   |  ____________________^
+LL | |             E::B(_) => false,
+LL | |             E::C => false,
+LL | |             _ => true,
+LL | |         };
+   | |_________^ help: try this: `!matches!(x, E::B(_) | E::C)`
+
+error: aborting due to 7 previous errors