Match on bool should be replaced with if..else block

1. Added another conditional in `check_expr` impl to lint if match expr
is a bool.
2. Test cases.

diff --git a/src/lib.rs b/src/lib.rs
index a3657460fe9924d8c3d0ac13635a4a62133ffcde..6d6c6dfeeb0c0b4bf4def02c3f7a2c7c40ba3c64 100755 (executable)
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -138,6 +138,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         loops::WHILE_LET_LOOP,
         matches::MATCH_REF_PATS,
         matches::SINGLE_MATCH,
+        matches::MATCH_BOOL,
         methods::SHOULD_IMPLEMENT_TRAIT,
         methods::STR_TO_STRING,
         methods::STRING_TO_STRING,

diff --git a/src/matches.rs b/src/matches.rs
index e935a6aa6e16ef4c3842c60028b14e07ea62cda6..0606c2447476e00e609eb3a2e162b68a2a18f834 100644 (file)
--- a/src/matches.rs
+++ b/src/matches.rs
@@ -1,5 +1,6 @@
 use rustc::lint::*;
 use rustc_front::hir::*;
+use rustc::middle::ty;
 
 use utils::{snippet, span_lint, span_help_and_lint, in_external_macro, expr_block};
 
@@ -9,13 +10,15 @@
 declare_lint!(pub MATCH_REF_PATS, Warn,
               "a match has all arms prefixed with `&`; the match expression can be \
                dereferenced instead");
+declare_lint!(pub MATCH_BOOL, Warn,
+              "a match on boolean expression; recommends `if..else` block instead");
 
 #[allow(missing_copy_implementations)]
 pub struct MatchPass;
 
 impl LintPass for MatchPass {
     fn get_lints(&self) -> LintArray {
-        lint_array!(SINGLE_MATCH, MATCH_REF_PATS)
+        lint_array!(SINGLE_MATCH, MATCH_REF_PATS, MATCH_BOOL)
     }
 }
 
@@ -59,6 +62,16 @@ fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
                          expression to match: `match *{} {{ ...`", snippet(cx, ex.span, "..")));
                 }
             }
+
+            // check preconditions for MATCH_BOOL
+            // type of expression == bool
+            if is_bool_expr(cx, ex) {
+                if in_external_macro(cx, expr.span) { return; }
+
+                span_lint(cx, MATCH_BOOL, expr.span,
+                                   "you seem to be trying to match on a boolean expression. \
+                                   Consider using if..else block");
+            }
         }
     }
 }
@@ -71,6 +84,10 @@ fn is_unit_expr(expr: &Expr) -> bool {
     }
 }
 
+fn is_bool_expr(cx: &LateContext, ex: &Expr ) -> bool {
+    cx.tcx.expr_ty(ex).sty == ty::TyBool
+}
+
 fn has_only_ref_pats(arms: &[Arm]) -> bool {
     let mapped = arms.iter().flat_map(|a| &a.pats).map(|p| match p.node {
         PatRegion(..) => Some(true),  // &-patterns

diff --git a/tests/compile-fail/matches.rs b/tests/compile-fail/matches.rs
index f25a5fa3fa4a574b12e7a5267c2d655612f09710..1cdf813bc00b9e93eff2cd125eff6ef5ffb2f412 100755 (executable)
--- a/tests/compile-fail/matches.rs
+++ b/tests/compile-fail/matches.rs
@@ -38,6 +38,28 @@ fn single_match(){
     }
 }
 
+fn match_bool() {
+    let test: bool = true;
+
+    match test {  //~ ERROR you seem to be trying to match on a boolean expression
+        true => (),
+        false => (),
+    };
+
+    let option = 1;
+    match option == 1 {  //~ ERROR you seem to be trying to match on a boolean expression
+        true => (),
+        false => (),
+    };
+
+    // Not linted
+    match option {
+        1 ... 10 => (),
+        10 ... 20 => (),
+        _ => (),
+    };
+}
+
 fn ref_pats() {
     {
         let v = &Some(0);