Merge pull request #401 from kvikas/match_bool

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

README.md

@@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your Rust code.
 [Jump to usage instructions](#usage)
 
 ##Lints
-There are 66 lints included in this crate:
+There are 67 lints included in this crate:
 
 name                                                                                                   | default | meaning
 -------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -34,6 +34,7 @@ name
 [let_and_return](https://github.com/Manishearth/rust-clippy/wiki#let_and_return)                       | warn    | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block
 [let_unit_value](https://github.com/Manishearth/rust-clippy/wiki#let_unit_value)                       | warn    | creating a let binding to a value of unit type, which usually can't be used afterwards
 [linkedlist](https://github.com/Manishearth/rust-clippy/wiki#linkedlist)                               | warn    | usage of LinkedList, usually a vector is faster, or a more specialized data structure like a VecDeque
+[match_bool](https://github.com/Manishearth/rust-clippy/wiki#match_bool)                               | warn    | a match on boolean expression; recommends `if..else` block instead
 [match_ref_pats](https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats)                       | warn    | a match has all arms prefixed with `&`; the match expression can be dereferenced instead
 [min_max](https://github.com/Manishearth/rust-clippy/wiki#min_max)                                     | warn    | `min(_, max(_, _))` (or vice versa) with bounds clamping the result to a constant
 [modulo_one](https://github.com/Manishearth/rust-clippy/wiki#modulo_one)                               | warn    | taking a number modulo 1, which always returns 0

src/lib.rs

@@ -136,6 +136,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         loops::REVERSE_RANGE_LOOP,
         loops::UNUSED_COLLECT,
         loops::WHILE_LET_LOOP,
+        matches::MATCH_BOOL,
         matches::MATCH_REF_PATS,
         matches::SINGLE_MATCH,
         methods::SHOULD_IMPLEMENT_TRAIT,

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,19 +10,23 @@ 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)
     }
 }
 
 impl LateLintPass for MatchPass {
     fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
         if let ExprMatch(ref ex, ref arms, MatchSource::Normal) = expr.node {
+            if in_external_macro(cx, expr.span) { return; }
+
             // check preconditions for SINGLE_MATCH
                 // only two arms
             if arms.len() == 2 &&
@@ -36,7 +41,6 @@ impl LateLintPass for MatchPass {
                 // finally, we don't want any content in the second arm (unit or empty block)
                 is_unit_expr(&arms[1].body)
             {
-                if in_external_macro(cx, expr.span) {return;}
                 span_help_and_lint(cx, SINGLE_MATCH, expr.span,
                                    "you seem to be trying to use match for destructuring a \
                                     single pattern. Consider using `if let`",
@@ -48,7 +52,6 @@ impl LateLintPass for MatchPass {
 
             // check preconditions for MATCH_REF_PATS
             if has_only_ref_pats(arms) {
-                if in_external_macro(cx, expr.span) { return; }
                 if let ExprAddrOf(Mutability::MutImmutable, ref inner) = ex.node {
                     span_lint(cx, MATCH_REF_PATS, expr.span, &format!(
                         "you don't need to add `&` to both the expression to match \
@@ -59,6 +62,15 @@ impl LateLintPass for MatchPass {
                          expression to match: `match *{} {{ ...`", snippet(cx, ex.span, "..")));
                 }
             }
+
+            // check preconditions for MATCH_BOOL
+            // type of expression == bool
+            if cx.tcx.expr_ty(ex).sty == ty::TyBool {
+
+                span_lint(cx, MATCH_BOOL, expr.span,
+                                   "you seem to be trying to match on a boolean expression. \
+                                   Consider using an if..else block");
+            }
         }
     }
 }

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);