Merge branch 'pr-645'

Author: Manishearth

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 117 lints included in this crate:
+There are 118 lints included in this crate:
 
 name                                                                                                           | default | meaning
 ---------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -15,6 +15,7 @@ name
 [bad_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#bad_bit_mask)                                   | warn    | expressions of the form `_ & mask == select` that will only ever return `true` or `false` (because in the example `select` containing bits that `mask` doesn't have)
 [block_in_if_condition_expr](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_expr)       | warn    | braces can be eliminated in conditions that are expressions, e.g `if { true } ...`
 [block_in_if_condition_stmt](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_stmt)       | warn    | avoid complex blocks in conditions, instead move the block higher and bind it with 'let'; e.g: `if { let x = true; x } ...`
+[bool_comparison](https://github.com/Manishearth/rust-clippy/wiki#bool_comparison)                             | warn    | comparing a variable to a boolean, e.g. `if x == true`
 [box_vec](https://github.com/Manishearth/rust-clippy/wiki#box_vec)                                             | warn    | usage of `Box<Vec<T>>`, vector elements are already on the heap
 [boxed_local](https://github.com/Manishearth/rust-clippy/wiki#boxed_local)                                     | warn    | using Box<T> where unnecessary
 [cast_possible_truncation](https://github.com/Manishearth/rust-clippy/wiki#cast_possible_truncation)           | allow   | casts that may cause truncation of the value, e.g `x as u8` where `x: u32`, or `x as i32` where `x: f32`

src/lib.rs

@@ -105,6 +105,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_late_lint_pass(box bit_mask::BitMask);
     reg.register_late_lint_pass(box ptr_arg::PtrArg);
     reg.register_late_lint_pass(box needless_bool::NeedlessBool);
+    reg.register_late_lint_pass(box needless_bool::BoolComparison);
     reg.register_late_lint_pass(box approx_const::ApproxConstant);
     reg.register_late_lint_pass(box misc::FloatCmp);
     reg.register_early_lint_pass(box precedence::Precedence);
@@ -252,6 +253,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         misc_early::UNNEEDED_FIELD_PATTERN,
         mut_reference::UNNECESSARY_MUT_PASSED,
         mutex_atomic::MUTEX_ATOMIC,
+        needless_bool::BOOL_COMPARISON,
         needless_bool::NEEDLESS_BOOL,
         needless_features::UNSTABLE_AS_MUT_SLICE,
         needless_features::UNSTABLE_AS_SLICE,

src/needless_bool.rs

@@ -6,8 +6,9 @@ use rustc::lint::*;
 use rustc_front::hir::*;
 
 use syntax::ast::Lit_;
+use syntax::codemap::Spanned;
 
-use utils::{span_lint, snippet};
+use utils::{span_lint, span_lint_and_then, snippet};
 
 /// **What it does:** This lint checks for expressions of the form `if c { true } else { false }` (or vice versa) and suggest using the condition directly.
 ///
@@ -23,6 +24,20 @@ declare_lint! {
      `if p { true } else { false }`"
 }
 
+/// **What it does:** This lint checks for expressions of the form `x == true` (or vice versa) and suggest using the variable directly.
+///
+/// **Why is this bad?** Unnecessary code.
+///
+/// **Known problems:** None.
+///
+/// **Example:** `if x == true { }` could be `if x { }`
+declare_lint! {
+    pub BOOL_COMPARISON,
+    Warn,
+    "comparing a variable to a boolean, e.g. \
+     `if x == true`"
+}
+
 #[derive(Copy,Clone)]
 pub struct NeedlessBool;
 
@@ -78,6 +93,65 @@ impl LateLintPass for NeedlessBool {
     }
 }
 
+#[derive(Copy,Clone)]
+pub struct BoolComparison;
+
+impl LintPass for BoolComparison {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(BOOL_COMPARISON)
+    }
+}
+
+impl LateLintPass for BoolComparison {
+    fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
+        if let ExprBinary(Spanned{ node: BiEq, .. }, ref left_side, ref right_side) = e.node {
+            match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) {
+                (Some(true), None) => {
+                    let hint = snippet(cx, right_side.span, "..").into_owned();
+                    span_lint_and_then(cx,
+                                       BOOL_COMPARISON,
+                                       e.span,
+                                       "equality checks against true are unnecesary",
+                                       |db| {
+                                           db.span_suggestion(e.span, "try simplifying it as shown:", hint);
+                                       });
+                }
+                (None, Some(true)) => {
+                    let hint = snippet(cx, left_side.span, "..").into_owned();
+                    span_lint_and_then(cx,
+                                       BOOL_COMPARISON,
+                                       e.span,
+                                       "equality checks against true are unnecesary",
+                                       |db| {
+                                           db.span_suggestion(e.span, "try simplifying it as shown:", hint);
+                                       });
+                }
+                (Some(false), None) => {
+                    let hint = format!("!{}", snippet(cx, right_side.span, ".."));
+                    span_lint_and_then(cx,
+                                       BOOL_COMPARISON,
+                                       e.span,
+                                       "equality checks against false can be replaced by a negation",
+                                       |db| {
+                                           db.span_suggestion(e.span, "try simplifying it as shown:", hint);
+                                       });
+                }
+                (None, Some(false)) => {
+                    let hint = format!("!{}", snippet(cx, left_side.span, ".."));
+                    span_lint_and_then(cx,
+                                       BOOL_COMPARISON,
+                                       e.span,
+                                       "equality checks against false can be replaced by a negation",
+                                       |db| {
+                                           db.span_suggestion(e.span, "try simplifying it as shown:", hint);
+                                       });
+                }
+                _ => (),
+            }
+        }
+    }
+}
+
 fn fetch_bool_block(block: &Block) -> Option<bool> {
     if block.stmts.is_empty() {
         block.expr.as_ref().and_then(|e| fetch_bool_expr(e))

tests/compile-fail/bool_comparison.rs

@@ -0,0 +1,23 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#[deny(bool_comparison)]
+fn main() {
+    let x = true;
+    if x == true { "yes" } else { "no" };
+    //~^ ERROR equality checks against true are unnecesary
+    //~| HELP try simplifying it as shown:
+    //~| SUGGESTION if x { "yes" } else { "no" };
+    if x == false { "yes" } else { "no" };
+    //~^ ERROR equality checks against false can be replaced by a negation
+    //~| HELP try simplifying it as shown:
+    //~| SUGGESTION if !x { "yes" } else { "no" };
+    if true == x { "yes" } else { "no" };
+    //~^ ERROR equality checks against true are unnecesary
+    //~| HELP try simplifying it as shown:
+    //~| SUGGESTION if x { "yes" } else { "no" };
+    if false == x { "yes" } else { "no" };
+    //~^ ERROR equality checks against false can be replaced by a negation
+    //~| HELP try simplifying it as shown:
+    //~| SUGGESTION if !x { "yes" } else { "no" };
+}