Skip to content

Commit 5f1f12f

Browse files
oli-obkoli-obk
authored
Merge pull request #1998 from montrivo/bug/is_empty-false-positive
len_without_is_empty false positive #1740
2 parents 73d87d9 + ec79970 commit 5f1f12f

File tree

3 files changed

+70
-25
lines changed

3 files changed

+70
-25
lines changed

‎clippy_lints/src/len_zero.rs

+45-10
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use rustc::lint::*;
22
use rustc::hir::def_id::DefId;
33
use rustc::ty;
44
use rustc::hir::*;
5+
use std::collections::HashSet;
56
use syntax::ast::{Lit, LitKind, Name};
67
use syntax::codemap::{Span, Spanned};
78
use utils::{get_item_name, in_macro, snippet, span_lint, span_lint_and_sugg, walk_ptrs_ty};
@@ -88,7 +89,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LenZero {
8889
}
8990
}
9091

91-
fn check_trait_items(cx: &LateContext, item: &Item, trait_items: &[TraitItemRef]) {
92+
fn check_trait_items(cx: &LateContext, visited_trait: &Item, trait_items: &[TraitItemRef]) {
9293
fn is_named_self(cx: &LateContext, item: &TraitItemRef, name: &str) -> bool {
9394
item.name == name &&
9495
if let AssociatedItemKind::Method { has_self } = item.kind {
@@ -102,18 +103,52 @@ fn check_trait_items(cx: &LateContext, item: &Item, trait_items: &[TraitItemRef]
102103
}
103104
}
104105

105-
if !trait_items.iter().any(|i| is_named_self(cx, i, "is_empty")) {
106-
if let Some(i) = trait_items.iter().find(|i| is_named_self(cx, i, "len")) {
107-
if cx.access_levels.is_exported(i.id.node_id) {
108-
span_lint(
109-
cx,
110-
LEN_WITHOUT_IS_EMPTY,
111-
item.span,
112-
&format!("trait `{}` has a `len` method but no `is_empty` method", item.name),
113-
);
106+
// fill the set with current and super traits
107+
fn fill_trait_set<'a, 'b: 'a>(traitt: &'b Item, set: &'a mut HashSet<&'b Item>, cx: &'b LateContext) {
108+
if set.insert(traitt) {
109+
if let ItemTrait(.., ref ty_param_bounds, _) = traitt.node {
110+
for ty_param_bound in ty_param_bounds {
111+
if let TraitTyParamBound(ref poly_trait_ref, _) = *ty_param_bound {
112+
let super_trait_node_id = cx.tcx
113+
.hir
114+
.as_local_node_id(poly_trait_ref.trait_ref.path.def.def_id())
115+
.expect("the DefId is local, the NodeId should be available");
116+
let super_trait = cx.tcx.hir.expect_item(super_trait_node_id);
117+
fill_trait_set(super_trait, set, cx);
118+
}
119+
}
114120
}
115121
}
116122
}
123+
124+
if cx.access_levels.is_exported(visited_trait.id) &&
125+
trait_items
126+
.iter()
127+
.any(|i| is_named_self(cx, i, "len"))
128+
{
129+
let mut current_and_super_traits = HashSet::new();
130+
fill_trait_set(visited_trait, &mut current_and_super_traits, cx);
131+
132+
let is_empty_method_found = current_and_super_traits
133+
.iter()
134+
.flat_map(|i| match i.node {
135+
ItemTrait(.., ref trait_items) => trait_items.iter(),
136+
_ => bug!("should only handle traits"),
137+
})
138+
.any(|i| is_named_self(cx, i, "is_empty"));
139+
140+
if !is_empty_method_found {
141+
span_lint(
142+
cx,
143+
LEN_WITHOUT_IS_EMPTY,
144+
visited_trait.span,
145+
&format!(
146+
"trait `{}` has a `len` method but no (possibly inherited) `is_empty` method",
147+
visited_trait.name
148+
),
149+
);
150+
}
151+
}
117152
}
118153

119154
fn check_impl_items(cx: &LateContext, item: &Item, impl_items: &[ImplItemRef]) {

‎tests/ui/len_zero.rs

+10
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,16 @@ impl HasWrongIsEmpty {
125125
}
126126
}
127127

128+
pub trait Empty {
129+
fn is_empty(&self) -> bool;
130+
}
131+
132+
pub trait InheritingEmpty: Empty { //must not trigger LEN_WITHOUT_IS_EMPTY
133+
fn len(&self) -> isize;
134+
}
135+
136+
137+
128138
fn main() {
129139
let x = [1, 2];
130140
if x.len() == 0 {

‎tests/ui/len_zero.stderr

+15-15
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ error: item `PubOne` has a public `len` method but no corresponding `is_empty` m
1010
|
1111
= note: `-D len-without-is-empty` implied by `-D warnings`
1212

13-
error: trait `PubTraitsToo` has a `len` method but no `is_empty` method
13+
error: trait `PubTraitsToo` has a `len` method but no (possibly inherited) `is_empty` method
1414
--> $DIR/len_zero.rs:55:1
1515
|
1616
55 | / pub trait PubTraitsToo {
@@ -43,47 +43,47 @@ error: item `HasWrongIsEmpty` has a public `len` method but no corresponding `is
4343
| |_^
4444

4545
error: length comparison to zero
46-
--> $DIR/len_zero.rs:130:8
46+
--> $DIR/len_zero.rs:140:8
4747
|
48-
130 | if x.len() == 0 {
48+
140 | if x.len() == 0 {
4949
| ^^^^^^^^^^^^ help: using `is_empty` is more concise: `x.is_empty()`
5050
|
5151
= note: `-D len-zero` implied by `-D warnings`
5252

5353
error: length comparison to zero
54-
--> $DIR/len_zero.rs:134:8
54+
--> $DIR/len_zero.rs:144:8
5555
|
56-
134 | if "".len() == 0 {
56+
144 | if "".len() == 0 {
5757
| ^^^^^^^^^^^^^ help: using `is_empty` is more concise: `"".is_empty()`
5858

5959
error: length comparison to zero
60-
--> $DIR/len_zero.rs:148:8
60+
--> $DIR/len_zero.rs:158:8
6161
|
62-
148 | if has_is_empty.len() == 0 {
62+
158 | if has_is_empty.len() == 0 {
6363
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `has_is_empty.is_empty()`
6464

6565
error: length comparison to zero
66-
--> $DIR/len_zero.rs:151:8
66+
--> $DIR/len_zero.rs:161:8
6767
|
68-
151 | if has_is_empty.len() != 0 {
68+
161 | if has_is_empty.len() != 0 {
6969
| ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `!has_is_empty.is_empty()`
7070

7171
error: length comparison to zero
72-
--> $DIR/len_zero.rs:154:8
72+
--> $DIR/len_zero.rs:164:8
7373
|
74-
154 | if has_is_empty.len() > 0 {
74+
164 | if has_is_empty.len() > 0 {
7575
| ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `!has_is_empty.is_empty()`
7676

7777
error: length comparison to zero
78-
--> $DIR/len_zero.rs:160:8
78+
--> $DIR/len_zero.rs:170:8
7979
|
80-
160 | if with_is_empty.len() == 0 {
80+
170 | if with_is_empty.len() == 0 {
8181
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `with_is_empty.is_empty()`
8282

8383
error: length comparison to zero
84-
--> $DIR/len_zero.rs:172:8
84+
--> $DIR/len_zero.rs:182:8
8585
|
86-
172 | if b.len() != 0 {
86+
182 | if b.len() != 0 {
8787
| ^^^^^^^^^^^^ help: using `is_empty` is more concise: `!b.is_empty()`
8888

8989
error: aborting due to 11 previous errors

0 commit comments

Comments
 (0)