Add lint for redundant pattern matching for explicit return boolean

diff --git a/clippy_lints/src/if_let_redundant_pattern_matching.rs b/clippy_lints/src/if_let_redundant_pattern_matching.rs
index bced0c9552d003614b178c353db495d6b2384265..6f786fc565955c2045d48c9d8d838de8f163ee93 100644 (file)
--- a/clippy_lints/src/if_let_redundant_pattern_matching.rs
+++ b/clippy_lints/src/if_let_redundant_pattern_matching.rs
@@ -11,6 +11,8 @@
 use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use crate::rustc::{declare_tool_lint, lint_array};
 use crate::rustc::hir::*;
+use crate::syntax::ptr::P;
+use crate::syntax::ast::LitKind;
 use crate::utils::{match_qpath, paths, snippet, span_lint_and_then};
 use crate::rustc_errors::Applicability;
 
@@ -58,46 +60,164 @@ fn get_lints(&self) -> LintArray {
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
     #[allow(clippy::similar_names)]
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
-        if let ExprKind::Match(ref op, ref arms, MatchSource::IfLetDesugar { .. }) = expr.node {
-            if arms[0].pats.len() == 1 {
-                let good_method = match arms[0].pats[0].node {
-                    PatKind::TupleStruct(ref path, ref pats, _) if pats.len() == 1 => {
-                        if let PatKind::Wild = pats[0].node {
-                            if match_qpath(path, &paths::RESULT_OK) {
-                                "is_ok()"
-                            } else if match_qpath(path, &paths::RESULT_ERR) {
-                                "is_err()"
-                            } else if match_qpath(path, &paths::OPTION_SOME) {
-                                "is_some()"
-                            } else {
-                                return;
-                            }
-                        } else {
-                            return;
-                        }
-                    },
+        if let ExprKind::Match(ref op, ref arms, ref match_source) = expr.node {
+            match match_source {
+                MatchSource::Normal => find_sugg_for_match(cx, expr, op, arms),
+                MatchSource::IfLetDesugar { .. } => find_sugg_for_if_let(cx, expr, op, arms),
+                _ => return,
+            }
+        }
+    }
+}
 
-                    PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()",
+fn find_sugg_for_if_let<'a, 'tcx>(
+    cx: &LateContext<'a, 'tcx>,
+    expr: &'tcx Expr,
+    op: &P<Expr>,
+    arms: &HirVec<Arm>
+) {
+    if arms[0].pats.len() == 1 {
+        let good_method = match arms[0].pats[0].node {
+            PatKind::TupleStruct(ref path, ref pats, _) if pats.len() == 1 => {
+                if let PatKind::Wild = pats[0].node {
+                    if match_qpath(path, &paths::RESULT_OK) {
+                        "is_ok()"
+                    } else if match_qpath(path, &paths::RESULT_ERR) {
+                        "is_err()"
+                    } else if match_qpath(path, &paths::OPTION_SOME) {
+                        "is_some()"
+                    } else {
+                        return;
+                    }
+                } else {
+                    return;
+                }
+            },
 
-                    _ => return,
-                };
+            PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()",
 
-                span_lint_and_then(
-                    cx,
-                    IF_LET_REDUNDANT_PATTERN_MATCHING,
-                    arms[0].pats[0].span,
-                    &format!("redundant pattern matching, consider using `{}`", good_method),
-                    |db| {
-                        let span = expr.span.to(op.span);
-                        db.span_suggestion_with_applicability(
-                            span,
-                            "try this",
-                            format!("if {}.{}", snippet(cx, op.span, "_"), good_method),
-                            Applicability::MachineApplicable, // snippet
-                        );
-                    },
+            _ => return,
+        };
+
+        span_lint_and_then(
+            cx,
+            IF_LET_REDUNDANT_PATTERN_MATCHING,
+            arms[0].pats[0].span,
+            &format!("redundant pattern matching, consider using `{}`", good_method),
+            |db| {
+                let span = expr.span.to(op.span);
+                db.span_suggestion_with_applicability(
+                    span,
+                    "try this",
+                    format!("if {}.{}", snippet(cx, op.span, "_"), good_method),
+                    Applicability::MachineApplicable, // snippet
                 );
-            }
+            },
+        );
+    } else {
+        return;
+    }
+}
+
+fn find_sugg_for_match<'a, 'tcx>(
+    cx: &LateContext<'a, 'tcx>,
+    expr: &'tcx Expr,
+    op: &P<Expr>,
+    arms: &HirVec<Arm>
+) {
+    if arms.len() == 2 {
+        let node_pair = (&arms[0].pats[0].node, &arms[1].pats[0].node);
+
+        let found_good_method = match node_pair {
+            (
+                PatKind::TupleStruct(ref path_left, ref pats_left, _),
+                PatKind::TupleStruct(ref path_right, ref pats_right, _)
+            ) if pats_left.len() == 1 && pats_right.len() == 1 => {
+                if let (PatKind::Wild, PatKind::Wild) = (&pats_left[0].node, &pats_right[0].node) {
+                    find_good_method_for_match(
+                        arms,
+                        path_left,
+                        path_right,
+                        &paths::RESULT_OK,
+                        &paths::RESULT_ERR,
+                        "is_ok()",
+                        "is_err()"
+                    )
+                } else {
+                    None
+                }
+            },
+            (
+                PatKind::TupleStruct(ref path_left, ref pats, _),
+                PatKind::Path(ref path_right)
+            ) | (
+                PatKind::Path(ref path_left),
+                PatKind::TupleStruct(ref path_right, ref pats, _)
+            ) if pats.len() == 1 => {
+                if let PatKind::Wild = pats[0].node {
+                    find_good_method_for_match(
+                        arms,
+                        path_left,
+                        path_right,
+                        &paths::OPTION_SOME,
+                        &paths::OPTION_NONE,
+                        "is_some()",
+                        "is_none()"
+                    )
+                } else {
+                    None
+                }
+            },
+            _ => None,
+        };
+
+        if let Some(good_method) = found_good_method {
+            span_lint_and_then(
+                cx,
+                IF_LET_REDUNDANT_PATTERN_MATCHING,
+                expr.span,
+                &format!("redundant pattern matching, consider using `{}`", good_method),
+                |db| {
+                    let span = expr.span.to(op.span);
+                    db.span_suggestion_with_applicability(
+                        span,
+                        "try this",
+                        format!("{}.{}", snippet(cx, op.span, "_"), good_method),
+                        Applicability::MachineApplicable, // snippet
+                    );
+                },
+            );
         }
+    } else {
+        return;
+    }
+}
+
+fn find_good_method_for_match<'a>(
+    arms: &HirVec<Arm>,
+    path_left: &QPath,
+    path_right: &QPath,
+    expected_left: &[&str],
+    expected_right: &[&str],
+    should_be_left: &'a str,
+    should_be_right: &'a str
+) -> Option<&'a str> {
+    let body_node_pair = if match_qpath(path_left, expected_left) && match_qpath(path_right, expected_right) {
+        (&(*arms[0].body).node, &(*arms[1].body).node)
+    } else if match_qpath(path_right, expected_left) && match_qpath(path_left, expected_right) {
+        (&(*arms[1].body).node, &(*arms[0].body).node)
+    } else {
+        return None;
+    };
+
+    match body_node_pair {
+        (ExprKind::Lit(ref lit_left), ExprKind::Lit(ref 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,
     }
 }

diff --git a/tests/ui/if_let_redundant_pattern_matching.rs b/tests/ui/if_let_redundant_pattern_matching.rs
index 1c0e7e79c68f04bb8cd81e9fe1e90472321ea822..3f7d0c8e1bda76c4ba90b34d46e92be6afe95699 100644 (file)
--- a/tests/ui/if_let_redundant_pattern_matching.rs
+++ b/tests/ui/if_let_redundant_pattern_matching.rs
@@ -42,4 +42,34 @@ fn main() {
     if let Ok(x) = Ok::<i32,i32>(42) {
         println!("{}", x);
     }
+
+    match Ok::<i32, i32>(42) {
+        Ok(_) => true,
+        Err(_) => false,
+    };
+
+    match Ok::<i32, i32>(42) {
+        Ok(_) => false,
+        Err(_) => true,
+    };
+
+    match Err::<i32, i32>(42) {
+        Ok(_) => false,
+        Err(_) => true,
+    };
+
+    match Err::<i32, i32>(42) {
+        Ok(_) => true,
+        Err(_) => false,
+    };
+
+    match Some(42) {
+        Some(_) => true,
+        None => false,
+    };
+
+    match None::<()> {
+        Some(_) => false,
+        None => true,
+    };
 }

diff --git a/tests/ui/if_let_redundant_pattern_matching.stderr b/tests/ui/if_let_redundant_pattern_matching.stderr
index 5111de671890f94484396bcd59e2a22af9f122af..93bafa7fcbd5bf7793e205e837c2882852ddee5b 100644 (file)
--- a/tests/ui/if_let_redundant_pattern_matching.stderr
+++ b/tests/ui/if_let_redundant_pattern_matching.stderr
@@ -30,5 +30,59 @@ error: redundant pattern matching, consider using `is_some()`
 28 | |     }
    | |_____- help: try this: `if Some(42).is_some()`
 
-error: aborting due to 4 previous errors
+error: redundant pattern matching, consider using `is_ok()`
+  --> $DIR/if_let_redundant_pattern_matching.rs:46:5
+   |
+46 | /     match Ok::<i32, i32>(42) {
+47 | |         Ok(_) => true,
+48 | |         Err(_) => false,
+49 | |     };
+   | |_____^ help: try this: `Ok::<i32, i32>(42).is_ok()`
+
+error: redundant pattern matching, consider using `is_err()`
+  --> $DIR/if_let_redundant_pattern_matching.rs:51:5
+   |
+51 | /     match Ok::<i32, i32>(42) {
+52 | |         Ok(_) => false,
+53 | |         Err(_) => true,
+54 | |     };
+   | |_____^ help: try this: `Ok::<i32, i32>(42).is_err()`
+
+error: redundant pattern matching, consider using `is_err()`
+  --> $DIR/if_let_redundant_pattern_matching.rs:56:5
+   |
+56 | /     match Err::<i32, i32>(42) {
+57 | |         Ok(_) => false,
+58 | |         Err(_) => true,
+59 | |     };
+   | |_____^ help: try this: `Err::<i32, i32>(42).is_err()`
+
+error: redundant pattern matching, consider using `is_ok()`
+  --> $DIR/if_let_redundant_pattern_matching.rs:61:5
+   |
+61 | /     match Err::<i32, i32>(42) {
+62 | |         Ok(_) => true,
+63 | |         Err(_) => false,
+64 | |     };
+   | |_____^ help: try this: `Err::<i32, i32>(42).is_ok()`
+
+error: redundant pattern matching, consider using `is_some()`
+  --> $DIR/if_let_redundant_pattern_matching.rs:66:5
+   |
+66 | /     match Some(42) {
+67 | |         Some(_) => true,
+68 | |         None => false,
+69 | |     };
+   | |_____^ help: try this: `Some(42).is_some()`
+
+error: redundant pattern matching, consider using `is_none()`
+  --> $DIR/if_let_redundant_pattern_matching.rs:71:5
+   |
+71 | /     match None::<()> {
+72 | |         Some(_) => false,
+73 | |         None => true,
+74 | |     };
+   | |_____^ help: try this: `None::<()>.is_none()`
+
+error: aborting due to 10 previous errors