mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #1998 from montrivo/bug/is_empty-false-positive

Browse Source 
len_without_is_empty false positive #1740
This commit is contained in:
Oliver Schneider 2017-08-29 08:49:16 +02:00 committed by GitHub
commit 5f1f12f555
3 changed files with 70 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
clippy_lints/src/len_zero.rs
View File

@ -2,6 +2,7 @@
use rustc::hir::def_id::DefId;
use rustc::ty;
use rustc::hir::*;
use std::collections::HashSet;
use syntax::ast::{Lit, LitKind, Name};
use syntax::codemap::{Span, Spanned};
use utils::{get_item_name, in_macro, snippet, span_lint, span_lint_and_sugg, walk_ptrs_ty};
@ -88,7 +89,7 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
}
}
fn check_trait_items(cx: &LateContext, item: &Item, trait_items: &[TraitItemRef]) {
fn check_trait_items(cx: &LateContext, visited_trait: &Item, trait_items: &[TraitItemRef]) {
fn is_named_self(cx: &LateContext, item: &TraitItemRef, name: &str) -> bool {
item.name == name &&
if let AssociatedItemKind::Method { has_self } = item.kind {
@ -102,18 +103,52 @@ fn is_named_self(cx: &LateContext, item: &TraitItemRef, name: &str) -> bool {
}
}
if !trait_items.iter().any(|i| is_named_self(cx, i, "is_empty")) {
if let Some(i) = trait_items.iter().find(|i| is_named_self(cx, i, "len")) {
if cx.access_levels.is_exported(i.id.node_id) {
span_lint(
cx,
LEN_WITHOUT_IS_EMPTY,
item.span,
&format!("trait `{}` has a `len` method but no `is_empty` method", item.name),
);
// fill the set with current and super traits
fn fill_trait_set<'a, 'b: 'a>(traitt: &'b Item, set: &'a mut HashSet<&'b Item>, cx: &'b LateContext) {
if set.insert(traitt) {
if let ItemTrait(.., ref ty_param_bounds, _) = traitt.node {
for ty_param_bound in ty_param_bounds {
if let TraitTyParamBound(ref poly_trait_ref, _) = *ty_param_bound {
let super_trait_node_id = cx.tcx
.hir
.as_local_node_id(poly_trait_ref.trait_ref.path.def.def_id())
.expect("the DefId is local, the NodeId should be available");
let super_trait = cx.tcx.hir.expect_item(super_trait_node_id);
fill_trait_set(super_trait, set, cx);
}
}
}
}
}
if cx.access_levels.is_exported(visited_trait.id) &&
trait_items
.iter()
.any(|i| is_named_self(cx, i, "len"))
{
let mut current_and_super_traits = HashSet::new();
fill_trait_set(visited_trait, &mut current_and_super_traits, cx);
let is_empty_method_found = current_and_super_traits
.iter()
.flat_map(|i| match i.node {
ItemTrait(.., ref trait_items) => trait_items.iter(),
_ => bug!("should only handle traits"),
})
.any(|i| is_named_self(cx, i, "is_empty"));
if !is_empty_method_found {
span_lint(
cx,
LEN_WITHOUT_IS_EMPTY,
visited_trait.span,
&format!(
"trait `{}` has a `len` method but no (possibly inherited) `is_empty` method",
visited_trait.name
),
);
}
}
}
fn check_impl_items(cx: &LateContext, item: &Item, impl_items: &[ImplItemRef]) {

10
tests/ui/len_zero.rs
View File

@ -125,6 +125,16 @@ pub fn is_empty(self: &Self, x : u32) -> bool {
}
}
pub trait Empty {
fn is_empty(&self) -> bool;
}
pub trait InheritingEmpty: Empty { //must not trigger LEN_WITHOUT_IS_EMPTY
fn len(&self) -> isize;
}
fn main() {
let x = [1, 2];
if x.len() == 0 {

30
tests/ui/len_zero.stderr
View File

@ -10,7 +10,7 @@ error: item `PubOne` has a public `len` method but no corresponding `is_empty` m
|
= note: `-D len-without-is-empty` implied by `-D warnings`
error: trait `PubTraitsToo` has a `len` method but no `is_empty` method
error: trait `PubTraitsToo` has a `len` method but no (possibly inherited) `is_empty` method
--> $DIR/len_zero.rs:55:1
|
55 | / pub trait PubTraitsToo {
@ -43,47 +43,47 @@ error: item `HasWrongIsEmpty` has a public `len` method but no corresponding `is
| |_^
error: length comparison to zero
--> $DIR/len_zero.rs:130:8
--> $DIR/len_zero.rs:140:8
|
130 | if x.len() == 0 {
140 | if x.len() == 0 {
| ^^^^^^^^^^^^ help: using `is_empty` is more concise: `x.is_empty()`
|
= note: `-D len-zero` implied by `-D warnings`
error: length comparison to zero
--> $DIR/len_zero.rs:134:8
--> $DIR/len_zero.rs:144:8
|
134 | if "".len() == 0 {
144 | if "".len() == 0 {
| ^^^^^^^^^^^^^ help: using `is_empty` is more concise: `"".is_empty()`
error: length comparison to zero
--> $DIR/len_zero.rs:148:8
--> $DIR/len_zero.rs:158:8
|
148 | if has_is_empty.len() == 0 {
158 | if has_is_empty.len() == 0 {
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `has_is_empty.is_empty()`
error: length comparison to zero
--> $DIR/len_zero.rs:151:8
--> $DIR/len_zero.rs:161:8
|
151 | if has_is_empty.len() != 0 {
161 | if has_is_empty.len() != 0 {
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `!has_is_empty.is_empty()`
error: length comparison to zero
--> $DIR/len_zero.rs:154:8
--> $DIR/len_zero.rs:164:8
|
154 | if has_is_empty.len() > 0 {
164 | if has_is_empty.len() > 0 {
| ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `!has_is_empty.is_empty()`
error: length comparison to zero
--> $DIR/len_zero.rs:160:8
--> $DIR/len_zero.rs:170:8
|
160 | if with_is_empty.len() == 0 {
170 | if with_is_empty.len() == 0 {
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `with_is_empty.is_empty()`
error: length comparison to zero
--> $DIR/len_zero.rs:172:8
--> $DIR/len_zero.rs:182:8
|
172 | if b.len() != 0 {
182 | if b.len() != 0 {
| ^^^^^^^^^^^^ help: using `is_empty` is more concise: `!b.is_empty()`
error: aborting due to 11 previous errors