From 53fa76dff968787fc39e7a6b96059f151bab6b2e Mon Sep 17 00:00:00 2001 From: llogiq Date: Sat, 2 May 2015 00:35:49 +0200 Subject: [PATCH] new lint: needless_bool (TODO: The warnings could give more specific directions) --- README.md | 4 +-- src/lib.rs | 5 ++- src/needless_bool.rs | 51 +++++++++++++++++++++++++++++ tests/compile-fail/needless_bool.rs | 12 +++++++ 4 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 src/needless_bool.rs create mode 100644 tests/compile-fail/needless_bool.rs diff --git a/README.md b/README.md index 577de498dab..0fc9a6c8869 100644 --- a/README.md +++ b/README.md @@ -10,13 +10,13 @@ Lints included in this crate: - `box_vec`: Warns on usage of `Box>` - `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. diff --git a/src/lib.rs b/src/lib.rs index ad477a21042..ea8a3962810 100644 --- a/src/lib.rs +++ b/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]); } diff --git a/src/needless_bool.rs b/src/needless_bool.rs new file mode 100644 index 00000000000..5b14e2fe1f3 --- /dev/null +++ b/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 { + 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 { + 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 + } +} diff --git a/tests/compile-fail/needless_bool.rs b/tests/compile-fail/needless_bool.rs new file mode 100644 index 00000000000..97a478ee410 --- /dev/null +++ b/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 +}