diff --git a/README.md b/README.md index ddc28a95f03..e081630326f 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,8 @@ Lints included in this crate: - `redundant_closure`: Warns on usage of eta-reducible closures like `|a| foo(a)` (which can be written as just `foo`) - `identity_op`: Warns on identity operations like `x + 0` or `y / 1` (which can be reduced to `x` and `y`, respectively) - `mut_mut`: Warns on `&mut &mut` which is either a copy'n'paste error, or shows a fundamental misunderstanding of references + - `len_zero`: Warns on `_.len() == 0` and suggests using `_.is_empty()` (or similar comparisons with `>` or `!=`) + - `len_without_is_empty`: Warns on traits or impls that have a `.len()` but no `.is_empty()` method To use, add the following lines to your Cargo.toml: diff --git a/src/len_zero.rs b/src/len_zero.rs new file mode 100644 index 00000000000..18ddbccc9a2 --- /dev/null +++ b/src/len_zero.rs @@ -0,0 +1,101 @@ +extern crate rustc_typeck as typeck; + +use syntax::ptr::P; +use syntax::ast::*; +use rustc::lint::{Context, LintPass, LintArray, Lint}; +use rustc::middle::ty::{self, node_id_to_type, sty, ty_ptr, ty_rptr, mt, MethodTraitItemId}; +use rustc::middle::def::{DefTy, DefStruct, DefTrait}; +use syntax::codemap::{Span, Spanned}; + +declare_lint!(pub LEN_ZERO, Warn, + "Warn on usage of double-mut refs, e.g. '&mut &mut ...'"); + +declare_lint!(pub LEN_WITHOUT_IS_EMPTY, Warn, + "Warn on traits and impls that have .len() but not .is_empty()"); + +#[derive(Copy,Clone)] +pub struct LenZero; + +impl LintPass for LenZero { + fn get_lints(&self) -> LintArray { + lint_array!(LEN_ZERO, LEN_WITHOUT_IS_EMPTY) + } + + fn check_item(&mut self, cx: &Context, item: &Item) { + match &item.node { + &ItemTrait(_, _, _, ref trait_items) => + check_trait_items(cx, item, trait_items), + &ItemImpl(_, _, _, None, _, ref impl_items) => // only non-trait + check_impl_items(cx, item, impl_items), + _ => () + } + } + + fn check_expr(&mut self, cx: &Context, expr: &Expr) { + if let &ExprBinary(Spanned{node: cmp, ..}, ref left, ref right) = + &expr.node { + match cmp { + BiEq => check_cmp(cx, expr.span, left, right, ""), + BiGt | BiNe => check_cmp(cx, expr.span, left, right, "!"), + _ => () + } + } + } +} + +fn check_trait_items(cx: &Context, item: &Item, trait_items: &[P]) { + fn is_named_self(item: &TraitItem, name: &str) -> bool { + item.ident.as_str() == name && item.attrs.len() == 0 + } + + if !trait_items.iter().any(|i| is_named_self(i, "is_empty")) { + //cx.span_lint(LEN_WITHOUT_IS_EMPTY, item.span, &format!("trait {}", item.ident.as_str())); + for i in trait_items { + if is_named_self(i, "len") { + cx.span_lint(LEN_WITHOUT_IS_EMPTY, i.span, + &format!("Trait '{}' has a '.len()' method, but no \ + '.is_empty()' method. Consider adding one.", + item.ident.as_str())); + } + }; + } +} + +fn check_impl_items(cx: &Context, item: &Item, impl_items: &[P]) { + fn is_named_self(item: &ImplItem, name: &str) -> bool { + item.ident.as_str() == name && item.attrs.len() == 0 + } + + if !impl_items.iter().any(|i| is_named_self(i, "is_empty")) { + for i in impl_items { + if is_named_self(i, "len") { + cx.span_lint(LEN_WITHOUT_IS_EMPTY, i.span, + &format!("Item '{}' has a '.len()' method, but no \ + '.is_empty()' method. Consider adding one.", + item.ident.as_str())); + return; + } + } + } +} + +fn check_cmp(cx: &Context, span: Span, left: &Expr, right: &Expr, empty: &str) { + match (&left.node, &right.node) { + (&ExprLit(ref lit), &ExprMethodCall(ref method, _, ref args)) => + check_len_zero(cx, span, method, args, lit, empty), + (&ExprMethodCall(ref method, _, ref args), &ExprLit(ref lit)) => + check_len_zero(cx, span, method, args, lit, empty), + _ => () + } +} + +fn check_len_zero(cx: &Context, span: Span, method: &SpannedIdent, + args: &[P], lit: &Lit, empty: &str) { + if let &Spanned{node: LitInt(0, _), ..} = lit { + if method.node.as_str() == "len" && args.len() == 1 { + cx.span_lint(LEN_ZERO, span, &format!( + "Consider replacing the len comparison with '{}_.is_empty()' if available", + empty)) + } + } +} diff --git a/src/lib.rs b/src/lib.rs index cd918bff459..e22ee28e77e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,6 +24,7 @@ pub mod approx_const; pub mod eta_reduction; pub mod identity_op; pub mod mut_mut; +pub mod len_zero; #[plugin_registrar] pub fn plugin_registrar(reg: &mut Registry) { @@ -42,6 +43,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_lint_pass(box eta_reduction::EtaPass as LintPassObject); reg.register_lint_pass(box identity_op::IdentityOp as LintPassObject); reg.register_lint_pass(box mut_mut::MutMut as LintPassObject); + reg.register_lint_pass(box len_zero::LenZero as LintPassObject); reg.register_lint_group("clippy", vec![types::BOX_VEC, types::LINKEDLIST, misc::SINGLE_MATCH, misc::STR_TO_STRING, @@ -56,5 +58,7 @@ pub fn plugin_registrar(reg: &mut Registry) { eta_reduction::REDUNDANT_CLOSURE, identity_op::IDENTITY_OP, mut_mut::MUT_MUT, + len_zero::LEN_ZERO, + len_zero::LEN_WITHOUT_IS_EMPTY, ]); } diff --git a/tests/compile-fail/len_zero.rs b/tests/compile-fail/len_zero.rs new file mode 100644 index 00000000000..7b1aafd409d --- /dev/null +++ b/tests/compile-fail/len_zero.rs @@ -0,0 +1,56 @@ +#![feature(plugin)] +#![plugin(clippy)] + +struct One; + +#[deny(len_without_is_empty)] +impl One { + fn len(self: &Self) -> isize { //~ERROR + 1 + } +} + +#[deny(len_without_is_empty)] +trait TraitsToo { + fn len(self: &Self) -> isize; //~ERROR +} + +impl TraitsToo for One { + fn len(self: &Self) -> isize { + 0 + } +} + +#[allow(dead_code)] +struct HasIsEmpty; + +#[deny(len_without_is_empty)] +#[allow(dead_code)] +impl HasIsEmpty { + fn len(self: &Self) -> isize { + 1 + } + + fn is_empty() -> bool { + false + } +} + +#[deny(len_zero)] +fn main() { + let x = [1, 2]; + if x.len() == 0 { //~ERROR + println!("This should not happen!"); + } + + let y = One; + // false positives here + if y.len() == 0 { //~ERROR + println!("This should not happen either!"); + } + + let z : &TraitsToo = &y; + if z.len() > 0 { //~ERROR + println!("Nor should this!"); + } +}