now the method lookup actually works (and I understand why! 😄), reduces unnecessary loops, and has a few comments
This commit is contained in:
parent
c5dfcaeeaf
commit
cdca2c93c1
@ -10,6 +10,7 @@ use rustc::middle::ty::{self, node_id_to_type, sty, ty_ptr, ty_rptr, expr_ty,
|
|||||||
use rustc::middle::def::{DefTy, DefStruct, DefTrait};
|
use rustc::middle::def::{DefTy, DefStruct, DefTrait};
|
||||||
use syntax::codemap::{Span, Spanned};
|
use syntax::codemap::{Span, Spanned};
|
||||||
use syntax::ast::*;
|
use syntax::ast::*;
|
||||||
|
use misc::walk_ty;
|
||||||
|
|
||||||
declare_lint!(pub LEN_ZERO, Warn,
|
declare_lint!(pub LEN_ZERO, Warn,
|
||||||
"Warn on usage of double-mut refs, e.g. '&mut &mut ...'");
|
"Warn on usage of double-mut refs, e.g. '&mut &mut ...'");
|
||||||
@ -49,7 +50,8 @@ impl LintPass for LenZero {
|
|||||||
|
|
||||||
fn check_trait_items(cx: &Context, item: &Item, trait_items: &[P<TraitItem>]) {
|
fn check_trait_items(cx: &Context, item: &Item, trait_items: &[P<TraitItem>]) {
|
||||||
fn is_named_self(item: &TraitItem, name: &str) -> bool {
|
fn is_named_self(item: &TraitItem, name: &str) -> bool {
|
||||||
item.ident.as_str() == name && item.attrs.len() == 0
|
item.ident.as_str() == name && if let MethodTraitItem(ref sig, _) =
|
||||||
|
item.node { is_self_sig(sig) } else { false }
|
||||||
}
|
}
|
||||||
|
|
||||||
if !trait_items.iter().any(|i| is_named_self(i, "is_empty")) {
|
if !trait_items.iter().any(|i| is_named_self(i, "is_empty")) {
|
||||||
@ -57,8 +59,8 @@ fn check_trait_items(cx: &Context, item: &Item, trait_items: &[P<TraitItem>]) {
|
|||||||
for i in trait_items {
|
for i in trait_items {
|
||||||
if is_named_self(i, "len") {
|
if is_named_self(i, "len") {
|
||||||
cx.span_lint(LEN_WITHOUT_IS_EMPTY, i.span,
|
cx.span_lint(LEN_WITHOUT_IS_EMPTY, i.span,
|
||||||
&format!("Trait '{}' has a '.len()' method, but no \
|
&format!("Trait '{}' has a '.len(_: &Self)' method, but no \
|
||||||
'.is_empty()' method. Consider adding one.",
|
'.is_empty(_: &Self)' method. Consider adding one.",
|
||||||
item.ident.as_str()));
|
item.ident.as_str()));
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
@ -67,7 +69,8 @@ fn check_trait_items(cx: &Context, item: &Item, trait_items: &[P<TraitItem>]) {
|
|||||||
|
|
||||||
fn check_impl_items(cx: &Context, item: &Item, impl_items: &[P<ImplItem>]) {
|
fn check_impl_items(cx: &Context, item: &Item, impl_items: &[P<ImplItem>]) {
|
||||||
fn is_named_self(item: &ImplItem, name: &str) -> bool {
|
fn is_named_self(item: &ImplItem, name: &str) -> bool {
|
||||||
item.ident.as_str() == name && item.attrs.len() == 0
|
item.ident.as_str() == name && if let MethodImplItem(ref sig, _) =
|
||||||
|
item.node { is_self_sig(sig) } else { false }
|
||||||
}
|
}
|
||||||
|
|
||||||
if !impl_items.iter().any(|i| is_named_self(i, "is_empty")) {
|
if !impl_items.iter().any(|i| is_named_self(i, "is_empty")) {
|
||||||
@ -76,8 +79,8 @@ fn check_impl_items(cx: &Context, item: &Item, impl_items: &[P<ImplItem>]) {
|
|||||||
let s = i.span;
|
let s = i.span;
|
||||||
cx.span_lint(LEN_WITHOUT_IS_EMPTY,
|
cx.span_lint(LEN_WITHOUT_IS_EMPTY,
|
||||||
Span{ lo: s.lo, hi: s.lo, expn_id: s.expn_id },
|
Span{ lo: s.lo, hi: s.lo, expn_id: s.expn_id },
|
||||||
&format!("Item '{}' has a '.len()' method, but no \
|
&format!("Item '{}' has a '.len(_: &Self)' method, but no \
|
||||||
'.is_empty()' method. Consider adding one.",
|
'.is_empty(_: &Self)' method. Consider adding one.",
|
||||||
item.ident.as_str()));
|
item.ident.as_str()));
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@ -85,6 +88,11 @@ fn check_impl_items(cx: &Context, item: &Item, impl_items: &[P<ImplItem>]) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn is_self_sig(sig: &MethodSig) -> bool {
|
||||||
|
if let SelfStatic = sig.explicit_self.node {
|
||||||
|
false } else { sig.decl.inputs.len() == 1 }
|
||||||
|
}
|
||||||
|
|
||||||
fn check_cmp(cx: &Context, span: Span, left: &Expr, right: &Expr, empty: &str) {
|
fn check_cmp(cx: &Context, span: Span, left: &Expr, right: &Expr, empty: &str) {
|
||||||
match (&left.node, &right.node) {
|
match (&left.node, &right.node) {
|
||||||
(&ExprLit(ref lit), &ExprMethodCall(ref method, _, ref args)) =>
|
(&ExprLit(ref lit), &ExprMethodCall(ref method, _, ref args)) =>
|
||||||
@ -99,29 +107,45 @@ fn check_len_zero(cx: &Context, span: Span, method: &SpannedIdent,
|
|||||||
args: &[P<Expr>], lit: &Lit, empty: &str) {
|
args: &[P<Expr>], lit: &Lit, empty: &str) {
|
||||||
if let &Spanned{node: LitInt(0, _), ..} = lit {
|
if let &Spanned{node: LitInt(0, _), ..} = lit {
|
||||||
if method.node.as_str() == "len" && args.len() == 1 &&
|
if method.node.as_str() == "len" && args.len() == 1 &&
|
||||||
has_is_empty(cx, &expr_ty(cx.tcx, &*args[0])) {
|
has_is_empty(cx, &*args[0]) {
|
||||||
cx.span_lint(LEN_ZERO, span, &format!(
|
cx.span_lint(LEN_ZERO, span, &format!(
|
||||||
"Consider replacing the len comparison with \
|
"Consider replacing the len comparison with '{}_.is_empty()'",
|
||||||
'{}_.is_empty()' if available",
|
|
||||||
empty))
|
empty))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn has_is_empty(cx: &Context, ty: &::rustc::middle::ty::Ty) -> bool {
|
/// check if this type has an is_empty method
|
||||||
fn check_item(cx: &Context, id: &ImplOrTraitItemId) -> bool {
|
fn has_is_empty(cx: &Context, expr: &Expr) -> bool {
|
||||||
if let &MethodTraitItemId(ref def_id) = id {
|
/// get a ImplOrTraitItem and return true if it matches is_empty(self)
|
||||||
if let ty::MethodTraitItem(ref method) = ty::impl_or_trait_item(
|
fn is_is_empty(cx: &Context, id: &ImplOrTraitItemId) -> bool {
|
||||||
cx.tcx, *def_id) {
|
if let &MethodTraitItemId(def_id) = id {
|
||||||
|
if let ty::MethodTraitItem(ref method) =
|
||||||
|
ty::impl_or_trait_item(cx.tcx, def_id) {
|
||||||
method.name.as_str() == "is_empty"
|
method.name.as_str() == "is_empty"
|
||||||
|
&& method.fty.sig.skip_binder().inputs.len() == 1
|
||||||
} else { false }
|
} else { false }
|
||||||
} else { false }
|
} else { false }
|
||||||
}
|
}
|
||||||
|
|
||||||
::rustc::middle::ty::ty_to_def_id(ty).map_or(true, |id| {
|
/// check the inherent impl's items for an is_empty(self) method
|
||||||
cx.tcx.impl_items.borrow().get(&id).map_or(false, |item_ids| {
|
fn has_is_empty_impl(cx: &Context, id: &DefId) -> bool {
|
||||||
item_ids.iter().any(|i| check_item(cx, i))
|
let impl_items = cx.tcx.impl_items.borrow();
|
||||||
}) || cx.tcx.trait_item_def_ids.borrow().get(&id).map_or(false,
|
cx.tcx.inherent_impls.borrow().get(id).map_or(false,
|
||||||
|item_ids| { item_ids.iter().any(|i| check_item(cx, i)) })
|
|ids| ids.iter().any(|iid| impl_items.get(iid).map_or(false,
|
||||||
})
|
|iids| iids.iter().any(|i| is_is_empty(cx, i)))))
|
||||||
|
}
|
||||||
|
|
||||||
|
let ty = &walk_ty(&expr_ty(cx.tcx, expr));
|
||||||
|
match ty.sty {
|
||||||
|
ty::ty_trait(_) => cx.tcx.trait_item_def_ids.borrow().get(
|
||||||
|
&ty::ty_to_def_id(ty).expect("trait impl not found")).map_or(false,
|
||||||
|
|ids| ids.iter().any(|i| is_is_empty(cx, i))),
|
||||||
|
ty::ty_projection(_) => ty::ty_to_def_id(ty).map_or(false,
|
||||||
|
|id| has_is_empty_impl(cx, &id)),
|
||||||
|
ty::ty_enum(ref id, _) | ty::ty_struct(ref id, _) =>
|
||||||
|
has_is_empty_impl(cx, id),
|
||||||
|
ty::ty_vec(..) => true,
|
||||||
|
_ => false,
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -10,7 +10,7 @@ use syntax::codemap::{Span, Spanned};
|
|||||||
|
|
||||||
use types::span_note_and_lint;
|
use types::span_note_and_lint;
|
||||||
|
|
||||||
fn walk_ty<'t>(ty: ty::Ty<'t>) -> ty::Ty<'t> {
|
pub fn walk_ty<'t>(ty: ty::Ty<'t>) -> ty::Ty<'t> {
|
||||||
match ty.sty {
|
match ty.sty {
|
||||||
ty_ptr(ref tm) | ty_rptr(_, ref tm) => walk_ty(tm.ty),
|
ty_ptr(ref tm) | ty_rptr(_, ref tm) => walk_ty(tm.ty),
|
||||||
_ => ty
|
_ => ty
|
||||||
|
@ -5,14 +5,14 @@ struct One;
|
|||||||
|
|
||||||
#[deny(len_without_is_empty)]
|
#[deny(len_without_is_empty)]
|
||||||
impl One {
|
impl One {
|
||||||
fn len(self: &Self) -> isize { //~ERROR Item 'One' has a '.len()' method
|
fn len(self: &Self) -> isize { //~ERROR Item 'One' has a '.len(_: &Self)'
|
||||||
1
|
1
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[deny(len_without_is_empty)]
|
#[deny(len_without_is_empty)]
|
||||||
trait TraitsToo {
|
trait TraitsToo {
|
||||||
fn len(self: &Self) -> isize; //~ERROR Trait 'TraitsToo' has a '.len()' method,
|
fn len(self: &Self) -> isize; //~ERROR Trait 'TraitsToo' has a '.len(_:
|
||||||
}
|
}
|
||||||
|
|
||||||
impl TraitsToo for One {
|
impl TraitsToo for One {
|
||||||
@ -21,17 +21,47 @@ impl TraitsToo for One {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[allow(dead_code)]
|
|
||||||
struct HasIsEmpty;
|
struct HasIsEmpty;
|
||||||
|
|
||||||
#[deny(len_without_is_empty)]
|
#[deny(len_without_is_empty)]
|
||||||
#[allow(dead_code)]
|
|
||||||
impl HasIsEmpty {
|
impl HasIsEmpty {
|
||||||
fn len(self: &Self) -> isize {
|
fn len(self: &Self) -> isize {
|
||||||
1
|
1
|
||||||
}
|
}
|
||||||
|
|
||||||
fn is_empty() -> bool {
|
fn is_empty(self: &Self) -> bool {
|
||||||
|
false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
struct Wither;
|
||||||
|
|
||||||
|
#[deny(len_without_is_empty)]
|
||||||
|
trait WithIsEmpty {
|
||||||
|
fn len(self: &Self) -> isize;
|
||||||
|
fn is_empty(self: &Self) -> bool;
|
||||||
|
}
|
||||||
|
|
||||||
|
impl WithIsEmpty for Wither {
|
||||||
|
fn len(self: &Self) -> isize {
|
||||||
|
1
|
||||||
|
}
|
||||||
|
|
||||||
|
fn is_empty(self: &Self) -> bool {
|
||||||
|
false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
struct HasWrongIsEmpty;
|
||||||
|
|
||||||
|
#[deny(len_without_is_empty)]
|
||||||
|
impl HasWrongIsEmpty {
|
||||||
|
fn len(self: &Self) -> isize { //~ERROR Item 'HasWrongIsEmpty' has a '.len(_: &Self)'
|
||||||
|
1
|
||||||
|
}
|
||||||
|
|
||||||
|
#[allow(dead_code, unused)]
|
||||||
|
fn is_empty(self: &Self, x : u32) -> bool {
|
||||||
false
|
false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -49,7 +79,24 @@ fn main() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
let z : &TraitsToo = &y;
|
let z : &TraitsToo = &y;
|
||||||
if z.len() > 0 { //~ERROR Consider replacing the len comparison
|
if z.len() > 0 { //no error, because TraitsToo has no .is_empty() method
|
||||||
println!("Nor should this!");
|
println!("Nor should this!");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let hie = HasIsEmpty;
|
||||||
|
if hie.len() == 0 { //~ERROR Consider replacing the len comparison
|
||||||
|
println!("Or this!");
|
||||||
|
}
|
||||||
|
assert!(!hie.is_empty());
|
||||||
|
|
||||||
|
let wie : &WithIsEmpty = &Wither;
|
||||||
|
if wie.len() == 0 { //~ERROR Consider replacing the len comparison
|
||||||
|
println!("Or this!");
|
||||||
|
}
|
||||||
|
assert!(!wie.is_empty());
|
||||||
|
|
||||||
|
let hwie = HasWrongIsEmpty;
|
||||||
|
if hwie.len() == 0 { //no error as HasWrongIsEmpty does not have .is_empty()
|
||||||
|
println!("Or this!");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user