From 2951b70d1575f62771ac2da007ef426f9ac600c6 Mon Sep 17 00:00:00 2001
From: Vikas Kumar <kr.vikas@gmail.com>
Date: Tue, 20 Oct 2015 10:18:48 -0700
Subject: [PATCH] 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.
---
 src/lib.rs                    |  1 +
 src/matches.rs                | 19 ++++++++++++++++++-
 tests/compile-fail/matches.rs | 22 ++++++++++++++++++++++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/src/lib.rs b/src/lib.rs
index a3657460fe99..6d6c6dfeeb0c 100755
--- 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 e935a6aa6e16..0606c2447476 100644
--- 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 SINGLE_MATCH, Warn,
 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 @@ impl LateLintPass for MatchPass {
                          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 f25a5fa3fa4a..1cdf813bc00b 100755
--- 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);