diff --git a/README.md b/README.md index 4d655c85951..acc3946635a 100644 --- a/README.md +++ b/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>`, vector elements are already on the heap [boxed_local](https://github.com/Manishearth/rust-clippy/wiki#boxed_local) | warn | using Box 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` diff --git a/src/lib.rs b/src/lib.rs index cd9ac226321..97101651d91 100644 --- a/src/lib.rs +++ b/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, diff --git a/src/needless_bool.rs b/src/needless_bool.rs index bfd819edcb6..4ddd8f2f3c5 100644 --- a/src/needless_bool.rs +++ b/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 { if block.stmts.is_empty() { block.expr.as_ref().and_then(|e| fetch_bool_expr(e)) diff --git a/tests/compile-fail/bool_comparison.rs b/tests/compile-fail/bool_comparison.rs new file mode 100644 index 00000000000..8a792931d02 --- /dev/null +++ b/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" }; +}