From 56b3e7b4c2308be84ce4eed18f03743cc592b780 Mon Sep 17 00:00:00 2001 From: Joshua Holmer Date: Tue, 9 Feb 2016 14:10:22 -0500 Subject: [PATCH] lint comparison to bool (e.g. `y == true`) Addresses #630 --- src/lib.rs | 2 + src/needless_bool.rs | 66 +++++++++++++++++++++++++++ tests/compile-fail/bool_comparison.rs | 12 +++++ 3 files changed, 80 insertions(+) create mode 100644 tests/compile-fail/bool_comparison.rs diff --git a/src/lib.rs b/src/lib.rs index 728aa124a18..c1e8c3c9f01 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); @@ -253,6 +254,7 @@ pub fn plugin_registrar(reg: &mut Registry) { mut_reference::UNNECESSARY_MUT_PASSED, mutex_atomic::MUTEX_ATOMIC, needless_bool::NEEDLESS_BOOL, + needless_bool::BOOL_COMPARISON, needless_features::UNSTABLE_AS_MUT_SLICE, needless_features::UNSTABLE_AS_SLICE, needless_update::NEEDLESS_UPDATE, diff --git a/src/needless_bool.rs b/src/needless_bool.rs index bfd819edcb6..a2a0d13e17c 100644 --- a/src/needless_bool.rs +++ b/src/needless_bool.rs @@ -6,6 +6,7 @@ use rustc::lint::*; use rustc_front::hir::*; use syntax::ast::Lit_; +use syntax::codemap::Spanned; use utils::{span_lint, snippet}; @@ -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,57 @@ 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 side_snip = snippet(cx, right_side.span, ".."); + let hint = format!("`{}`", side_snip); + span_lint(cx, + BOOL_COMPARISON, + e.span, + &format!("you can simplify this boolean comparison to {}", hint)); + } + (None, Some(true)) => { + let side_snip = snippet(cx, left_side.span, ".."); + let hint = format!("`{}`", side_snip); + span_lint(cx, + BOOL_COMPARISON, + e.span, + &format!("you can simplify this boolean comparison to {}", hint)); + } + (Some(false), None) => { + let side_snip = snippet(cx, right_side.span, ".."); + let hint = format!("`!{}`", side_snip); + span_lint(cx, + BOOL_COMPARISON, + e.span, + &format!("you can simplify this boolean comparison to {}", hint)); + } + (None, Some(false)) => { + let side_snip = snippet(cx, left_side.span, ".."); + let hint = format!("`!{}`", side_snip); + span_lint(cx, + BOOL_COMPARISON, + e.span, + &format!("you can simplify this boolean comparison to {}", 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..d2a362af2f4 --- /dev/null +++ b/tests/compile-fail/bool_comparison.rs @@ -0,0 +1,12 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#[allow(needless_bool)] +#[deny(bool_comparison)] +fn main() { + let x = true; + if x == true { true } else { false }; //~ERROR you can simplify this boolean comparison to `x` + if x == false { true } else { false }; //~ERROR you can simplify this boolean comparison to `!x` + if true == x { true } else { false }; //~ERROR you can simplify this boolean comparison to `x` + if false == x { true } else { false }; //~ERROR you can simplify this boolean comparison to `!x` +}