Merge pull request #62 from Manishearth/len_zero_is_empty

Here goes nothing... 😄
This commit is contained in:
llogiq 2015-05-20 10:38:08 +02:00
commit 57a6b93ed0
4 changed files with 163 additions and 0 deletions

View File

@ -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:

101
src/len_zero.rs Normal file
View File

@ -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<TraitItem>]) {
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<ImplItem>]) {
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<Expr>], 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))
}
}
}

View File

@ -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,
]);
}

View File

@ -0,0 +1,56 @@
#![feature(plugin)]
#![plugin(clippy)]
struct One;
#[deny(len_without_is_empty)]
impl One {
fn len(self: &Self) -> isize { //~ERROR Item 'One' has a '.len()' method
1
}
}
#[deny(len_without_is_empty)]
trait TraitsToo {
fn len(self: &Self) -> isize; //~ERROR Trait 'TraitsToo' has a '.len()' method,
}
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 Consider replacing the len comparison
println!("This should not happen!");
}
let y = One;
// false positives here
if y.len() == 0 { //~ERROR Consider replacing the len comparison
println!("This should not happen either!");
}
let z : &TraitsToo = &y;
if z.len() > 0 { //~ERROR Consider replacing the len comparison
println!("Nor should this!");
}
}