redundant_pattern_matching

clippy_lints/src/matches/match_like_matches.rs

@@ -1,10 +1,12 @@
+use super::REDUNDANT_PATTERN_MATCHING;
 use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::is_lint_allowed;
 use clippy_utils::is_wild;
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::span_contains_comment;
 use rustc_ast::{Attribute, LitKind};
 use rustc_errors::Applicability;
-use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Guard, Pat};
+use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Guard, Pat, PatKind, QPath};
 use rustc_lint::{LateContext, LintContext};
 use rustc_middle::ty;
 use rustc_span::source_map::Spanned;
@@ -99,6 +101,14 @@ where
                 }
             }
 
+            for arm in iter_without_last.clone() {
+                if let Some(pat) = arm.1 {
+                    if !is_lint_allowed(cx, REDUNDANT_PATTERN_MATCHING, pat.hir_id) && is_some(pat.kind) {
+                        return false;
+                    }
+                }
+            }
+
             // The suggestion may be incorrect, because some arms can have `cfg` attributes
             // evaluated into `false` and so such arms will be stripped before.
             let mut applicability = Applicability::MaybeIncorrect;
@@ -170,3 +180,13 @@ fn find_bool_lit(ex: &ExprKind<'_>) -> Option<bool> {
         _ => None,
     }
 }
+
+fn is_some(path_kind: PatKind<'_>) -> bool {
+    match path_kind {
+        PatKind::TupleStruct(QPath::Resolved(_, path), [first, ..], _) if is_wild(first) => {
+            let name = path.segments[0].ident;
+            name.name == rustc_span::sym::Some
+        },
+        _ => false,
+    }
+}

clippy_lints/src/matches/redundant_pattern_match.rs

@@ -189,73 +189,7 @@ pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op
     if arms.len() == 2 {
         let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind);
 
-        let found_good_method = match node_pair {
-            (
-                PatKind::TupleStruct(ref path_left, patterns_left, _),
-                PatKind::TupleStruct(ref path_right, patterns_right, _),
-            ) if patterns_left.len() == 1 && patterns_right.len() == 1 => {
-                if let (PatKind::Wild, PatKind::Wild) = (&patterns_left[0].kind, &patterns_right[0].kind) {
-                    find_good_method_for_match(
-                        cx,
-                        arms,
-                        path_left,
-                        path_right,
-                        Item::Lang(ResultOk),
-                        Item::Lang(ResultErr),
-                        "is_ok()",
-                        "is_err()",
-                    )
-                    .or_else(|| {
-                        find_good_method_for_match(
-                            cx,
-                            arms,
-                            path_left,
-                            path_right,
-                            Item::Diag(sym::IpAddr, sym!(V4)),
-                            Item::Diag(sym::IpAddr, sym!(V6)),
-                            "is_ipv4()",
-                            "is_ipv6()",
-                        )
-                    })
-                } else {
-                    None
-                }
-            },
-            (PatKind::TupleStruct(ref path_left, patterns, _), PatKind::Path(ref path_right))
-            | (PatKind::Path(ref path_left), PatKind::TupleStruct(ref path_right, patterns, _))
-                if patterns.len() == 1 =>
-            {
-                if let PatKind::Wild = patterns[0].kind {
-                    find_good_method_for_match(
-                        cx,
-                        arms,
-                        path_left,
-                        path_right,
-                        Item::Lang(OptionSome),
-                        Item::Lang(OptionNone),
-                        "is_some()",
-                        "is_none()",
-                    )
-                    .or_else(|| {
-                        find_good_method_for_match(
-                            cx,
-                            arms,
-                            path_left,
-                            path_right,
-                            Item::Lang(PollReady),
-                            Item::Lang(PollPending),
-                            "is_ready()",
-                            "is_pending()",
-                        )
-                    })
-                } else {
-                    None
-                }
-            },
-            _ => None,
-        };
-
-        if let Some(good_method) = found_good_method {
+        if let Some(good_method) = found_good_method(cx, arms, node_pair) {
             let span = expr.span.to(op.span);
             let result_expr = match &op.kind {
                 ExprKind::AddrOf(_, _, borrowed) => borrowed,
@@ -279,6 +213,127 @@ pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op
     }
 }
 
+fn found_good_method<'a>(
+    cx: &LateContext<'_>,
+    arms: &[Arm<'_>],
+    node: (&PatKind<'_>, &PatKind<'_>),
+) -> Option<&'a str> {
+    match node {
+        (
+            PatKind::TupleStruct(ref path_left, patterns_left, _),
+            PatKind::TupleStruct(ref path_right, patterns_right, _),
+        ) if patterns_left.len() == 1 && patterns_right.len() == 1 => {
+            if let (PatKind::Wild, PatKind::Wild) = (&patterns_left[0].kind, &patterns_right[0].kind) {
+                find_good_method_for_match(
+                    cx,
+                    arms,
+                    path_left,
+                    path_right,
+                    Item::Lang(ResultOk),
+                    Item::Lang(ResultErr),
+                    "is_ok()",
+                    "is_err()",
+                )
+                .or_else(|| {
+                    find_good_method_for_match(
+                        cx,
+                        arms,
+                        path_left,
+                        path_right,
+                        Item::Diag(sym::IpAddr, sym!(V4)),
+                        Item::Diag(sym::IpAddr, sym!(V6)),
+                        "is_ipv4()",
+                        "is_ipv6()",
+                    )
+                })
+            } else {
+                None
+            }
+        },
+        (PatKind::TupleStruct(ref path_left, patterns, _), PatKind::Path(ref path_right))
+        | (PatKind::Path(ref path_left), PatKind::TupleStruct(ref path_right, patterns, _))
+            if patterns.len() == 1 =>
+        {
+            if let PatKind::Wild = patterns[0].kind {
+                find_good_method_for_match(
+                    cx,
+                    arms,
+                    path_left,
+                    path_right,
+                    Item::Lang(OptionSome),
+                    Item::Lang(OptionNone),
+                    "is_some()",
+                    "is_none()",
+                )
+                .or_else(|| {
+                    find_good_method_for_match(
+                        cx,
+                        arms,
+                        path_left,
+                        path_right,
+                        Item::Lang(PollReady),
+                        Item::Lang(PollPending),
+                        "is_ready()",
+                        "is_pending()",
+                    )
+                })
+            } else {
+                None
+            }
+        },
+        (PatKind::TupleStruct(ref path_left, patterns, _), PatKind::Wild) if patterns.len() == 1 => {
+            if let PatKind::Wild = patterns[0].kind {
+                get_good_method(cx, arms, path_left)
+            } else {
+                None
+            }
+        },
+        (PatKind::Path(ref path_left), PatKind::Wild) => get_good_method(cx, arms, path_left),
+        _ => None,
+    }
+}
+
+fn get_ident(path: &QPath<'_>) -> Option<rustc_span::symbol::Ident> {
+    match path {
+        QPath::Resolved(_, path) => {
+            let name = path.segments[0].ident;
+            Some(name)
+        },
+        _ => None,
+    }
+}
+
+fn get_good_method<'a>(cx: &LateContext<'_>, arms: &[Arm<'_>], path_left: &QPath<'_>) -> Option<&'a str> {
+    if let Some(name) = get_ident(path_left) {
+        return match name.as_str() {
+            "Ok" => {
+                find_good_method_for_matches_macro(cx, arms, path_left, Item::Lang(ResultOk), "is_ok()", "is_err()")
+            },
+            "Err" => {
+                find_good_method_for_matches_macro(cx, arms, path_left, Item::Lang(ResultErr), "is_err()", "is_ok()")
+            },
+            "Some" => find_good_method_for_matches_macro(
+                cx,
+                arms,
+                path_left,
+                Item::Lang(OptionSome),
+                "is_some()",
+                "is_none()",
+            ),
+            "None" => find_good_method_for_matches_macro(
+                cx,
+                arms,
+                path_left,
+                Item::Lang(OptionNone),
+                "is_none()",
+                "is_some()",
+            ),
+            _ => None,
+        };
+    }
+    None
+}
+
 #[derive(Clone, Copy)]
 enum Item {
     Lang(LangItem),
@@ -345,3 +400,29 @@ fn find_good_method_for_match<'a>(
         _ => None,
     }
 }
+
+fn find_good_method_for_matches_macro<'a>(
+    cx: &LateContext<'_>,
+    arms: &[Arm<'_>],
+    path_left: &QPath<'_>,
+    expected_item_left: Item,
+    should_be_left: &'a str,
+    should_be_right: &'a str,
+) -> Option<&'a str> {
+    let first_pat = arms[0].pat;
+
+    let body_node_pair = if is_pat_variant(cx, first_pat, path_left, expected_item_left) {
+        (&arms[0].body.kind, &arms[1].body.kind)
+    } else {
+        return None;
+    };
+
+    match body_node_pair {
+        (ExprKind::Lit(lit_left), ExprKind::Lit(lit_right)) => match (&lit_left.node, &lit_right.node) {
+            (LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
+            (LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
+            _ => None,
+        },
+        _ => None,
+    }
+}

tests/ui/match_expr_like_matches_macro.fixed

@@ -15,7 +15,7 @@ fn main() {
     let _y = matches!(x, Some(0));
 
     // Lint
-    let _w = matches!(x, Some(_));
+    let _w = x.is_some();
 
     // Turn into is_none
     let _z = x.is_none();

tests/ui/match_expr_like_matches_macro.stderr

@@ -10,15 +10,17 @@ LL | |     };
    |
    = note: `-D clippy::match-like-matches-macro` implied by `-D warnings`
 
-error: match expression looks like `matches!` macro
+error: redundant pattern matching, consider using `is_some()`
   --> $DIR/match_expr_like_matches_macro.rs:21:14
    |
 LL |       let _w = match x {
    |  ______________^
 LL | |         Some(_) => true,
 LL | |         _ => false,
 LL | |     };
-   | |_____^ help: try this: `matches!(x, Some(_))`
+   | |_____^ help: try this: `x.is_some()`
+   |
+   = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings`
 
 error: redundant pattern matching, consider using `is_none()`
   --> $DIR/match_expr_like_matches_macro.rs:27:14
@@ -29,8 +31,6 @@ LL | |         Some(_) => false,
 LL | |         None => true,
 LL | |     };
    | |_____^ help: try this: `x.is_none()`
-   |
-   = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings`
 
 error: match expression looks like `matches!` macro
   --> $DIR/match_expr_like_matches_macro.rs:33:15

tests/ui/redundant_pattern_matching_option.fixed

@@ -46,6 +46,7 @@ fn main() {
     let _ = if opt.is_some() { true } else { false };
 
     issue6067();
+    issue10726();
 
     let _ = if gen_opt().is_some() {
         1
@@ -88,3 +89,21 @@ fn issue7921() {
     if (&None::<()>).is_none() {}
     if (&None::<()>).is_none() {}
 }
+
+fn issue10726() {
+    let x = Some(42);
+
+    x.is_some();
+
+    x.is_none();
+
+    x.is_none();
+
+    x.is_some();
+
+    // Don't lint
+    match x {
+        Some(21) => true,
+        _ => false,
+    };
+}

tests/ui/redundant_pattern_matching_option.rs

@@ -55,6 +55,7 @@ fn main() {
     let _ = if let Some(_) = opt { true } else { false };
 
     issue6067();
+    issue10726();
 
     let _ = if let Some(_) = gen_opt() {
         1
@@ -103,3 +104,33 @@ fn issue7921() {
     if let None = *(&None::<()>) {}
     if let None = *&None::<()> {}
 }
+
+fn issue10726() {
+    let x = Some(42);
+
+    match x {
+        Some(_) => true,
+        _ => false,
+    };
+
+    match x {
+        None => true,
+        _ => false,
+    };
+
+    match x {
+        Some(_) => false,
+        _ => true,
+    };
+
+    match x {
+        None => false,
+        _ => true,
+    };
+
+    // Don't lint
+    match x {
+        Some(21) => true,
+        _ => false,
+    };
+}