new lint: needless_bool (TODO: The warnings could give more specific directions)

README.md

@@ -10,13 +10,13 @@ Lints included in this crate:
  - `box_vec`: Warns on usage of `Box<Vec<T>>`
  - `dlist`: Warns on usage of `DList`
  - `str_to_string`: Warns on usage of `str::to_string()`
- - `toplevel_ref_arg`: Warns when a function argument is declared `ref` (i.e. `fn foo(ref x: u8)`, but not `fn foo((ref x, ref y): (u8, u8))`).
+ - `toplevel_ref_arg`: Warns when a function argument is declared `ref` (i.e. `fn foo(ref x: u8)`, but not `fn foo((ref x, ref y): (u8, u8))`)
  - `eq_op`: Warns on equal operands on both sides of a comparison or bitwise combination
  - `bad_bit_mask`: Denies 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)
+ - `needless_bool` : Warns on if-statements with plain booleans in the then- and else-clause, e.g. `if p { true } else { false }`
 
 You can allow/warn/deny the whole set using the `clippy` lint group (`#[allow(clippy)]`, etc)
 
-
 More to come, please [file an issue](https://github.com/Manishearth/rust-clippy/issues) if you have ideas!
 
 Licensed under [MPL](https://www.mozilla.org/MPL/2.0/). If you're having issues with the license, let me know and I'll try to change it to something more permissive.

src/lib.rs

@@ -18,6 +18,7 @@ pub mod types;
 pub mod misc;
 pub mod eq_op;
 pub mod bit_mask;
+pub mod needless_bool;
 
 #[plugin_registrar]
 pub fn plugin_registrar(reg: &mut Registry) {
@@ -27,8 +28,10 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_lint_pass(box misc::TopLevelRefPass as LintPassObject);
     reg.register_lint_pass(box eq_op::EqOp as LintPassObject);
     reg.register_lint_pass(box bit_mask::BitMask as LintPassObject);
+    reg.register_lint_pass(box needless_bool::NeedlessBool as LintPassObject);
     reg.register_lint_group("clippy", vec![types::BOX_VEC, types::LINKEDLIST,
                                            misc::SINGLE_MATCH, misc::STR_TO_STRING,
                                            misc::TOPLEVEL_REF_ARG, eq_op::EQ_OP,
-                                           bit_mask::BAD_BIT_MASK]);
+                                           bit_mask::BAD_BIT_MASK, 
+                                           needless_bool::NEEDLESS_BOOL]);
 }

src/needless_bool.rs

@@ -0,0 +1,51 @@
+//! Checks for needless boolean results of if-else expressions
+//!
+//! This lint is **deny** by default
+
+use rustc::plugin::Registry;
+use rustc::lint::*;
+use rustc::middle::const_eval::lookup_const_by_id;
+use rustc::middle::def::*;
+use syntax::ast::*;
+use syntax::ast_util::{is_comparison_binop, binop_to_string};
+use syntax::ptr::P;
+use syntax::codemap::Span;
+
+declare_lint! {
+    pub NEEDLESS_BOOL,
+    Warn,
+    "Warn on needless use of if x { true } else { false } (or vice versa)"
+}
+
+#[derive(Copy,Clone)]
+pub struct NeedlessBool;
+
+impl LintPass for NeedlessBool {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(NEEDLESS_BOOL)
+    }
+    
+    fn check_expr(&mut self, cx: &Context, e: &Expr) {
+        if let ExprIf(_, ref then_block, Option::Some(ref else_expr)) = e.node {
+			match (fetch_bool_block(then_block), fetch_bool_expr(else_expr)) {
+				(Option::Some(true), Option::Some(true)) => { cx.span_lint(NEEDLESS_BOOL, e.span, "your if-then-else expression will always return true"); },
+				(Option::Some(true), Option::Some(false)) => { cx.span_lint(NEEDLESS_BOOL, e.span, "you can reduce your if-statement to its predicate"); },
+				(Option::Some(false), Option::Some(true)) => { cx.span_lint(NEEDLESS_BOOL, e.span, "you can reduce your if-statement to '!' + your predicate"); },
+				(Option::Some(false), Option::Some(false)) => { cx.span_lint(NEEDLESS_BOOL, e.span, "your if-then-else expression will always return false"); },
+				_ => ()
+			}
+		}
+    }
+}
+
+fn fetch_bool_block(block: &Block) -> Option<bool> {
+	if block.stmts.is_empty() { block.expr.as_ref().and_then(|e| fetch_bool_expr(e)) } else { Option::None }
+}
+	
+fn fetch_bool_expr(expr: &Expr) -> Option<bool> {
+	match &expr.node {
+		&ExprBlock(ref block) => fetch_bool_block(block),
+		&ExprLit(ref lit_ptr) => if let &LitBool(value) = &lit_ptr.node { Option::Some(value) } else { Option::None },
+		_ => Option::None
+	}
+}

tests/compile-fail/needless_bool.rs

@@ -0,0 +1,12 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#[deny(needless_bool)]
+fn main() {
+	let x = true;
+	if x { true } else { true }; //~ERROR 
+	if x { false } else { false }; //~ERROR
+	if x { true } else { false }; //~ERROR
+	if x { false } else { true }; //~ERROR
+	if x { x } else { false }; // would also be questionable, but we don't catch this yet
+}