diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs
index df3239ee1c7..09b3da1e99c 100644
--- a/clippy_lints/src/len_zero.rs
+++ b/clippy_lints/src/len_zero.rs
@@ -1,7 +1,7 @@
-use rustc::lint::*;
use rustc::hir::def_id::DefId;
-use rustc::ty;
use rustc::hir::*;
+use rustc::lint::*;
+use rustc::ty;
use std::collections::HashSet;
use syntax::ast::{Lit, LitKind, Name};
use syntax::codemap::{Span, Spanned};
@@ -81,8 +81,24 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LenZero {
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, "!"),
+ BiEq => {
+ check_cmp(cx, expr.span, left, right, "", 0); // len == 0
+ check_cmp(cx, expr.span, right, left, "", 0); // 0 == len
+ },
+ BiNe => {
+ check_cmp(cx, expr.span, left, right, "!", 0); // len != 0
+ check_cmp(cx, expr.span, right, left, "!", 0); // 0 != len
+ },
+ BiGt => {
+ check_cmp(cx, expr.span, left, right, "!", 0); // len > 0
+ check_cmp(cx, expr.span, right, left, "", 1); // 1 > len
+ },
+ BiLt => {
+ check_cmp(cx, expr.span, left, right, "", 1); // len < 1
+ check_cmp(cx, expr.span, right, left, "!", 0); // 0 < len
+ },
+ BiGe => check_cmp(cx, expr.span, left, right, "!", 1), // len <= 1
+ BiLe => check_cmp(cx, expr.span, right, left, "!", 1), // 1 >= len
_ => (),
}
}
@@ -168,40 +184,45 @@ fn check_impl_items(cx: &LateContext, item: &Item, impl_items: &[ImplItemRef]) {
cx,
LEN_WITHOUT_IS_EMPTY,
item.span,
- &format!("item `{}` has a public `len` method but {} `is_empty` method", ty, is_empty),
+ &format!(
+ "item `{}` has a public `len` method but {} `is_empty` method",
+ ty, is_empty
+ ),
);
}
}
}
-fn check_cmp(cx: &LateContext, span: Span, left: &Expr, right: &Expr, op: &str) {
- // check if we are in an is_empty() method
- if let Some(name) = get_item_name(cx, left) {
- if name == "is_empty" {
- return;
+fn check_cmp(cx: &LateContext, span: Span, method: &Expr, lit: &Expr, op: &str, compare_to: u32) {
+ if let (&ExprMethodCall(ref method_path, _, ref args), &ExprLit(ref lit)) = (&method.node, &lit.node) {
+ // check if we are in an is_empty() method
+ if let Some(name) = get_item_name(cx, method) {
+ if name == "is_empty" {
+ return;
+ }
}
- }
- match (&left.node, &right.node) {
- (&ExprLit(ref lit), &ExprMethodCall(ref method_path, _, ref args)) |
- (&ExprMethodCall(ref method_path, _, ref args), &ExprLit(ref lit)) => {
- check_len_zero(cx, span, method_path.name, args, lit, op)
- },
- _ => (),
+
+ check_len(cx, span, method_path.name, args, lit, op, compare_to)
}
}
-fn check_len_zero(cx: &LateContext, span: Span, name: Name, args: &[Expr], lit: &Lit, op: &str) {
+fn check_len(cx: &LateContext, span: Span, method_name: Name, args: &[Expr], lit: &Lit, op: &str, compare_to: u32) {
if let Spanned {
- node: LitKind::Int(0, _),
+ node: LitKind::Int(lit, _),
..
} = *lit
{
- if name == "len" && args.len() == 1 && has_is_empty(cx, &args[0]) {
+ // check if length is compared to the specified number
+ if lit != u128::from(compare_to) {
+ return;
+ }
+
+ if method_name == "len" && args.len() == 1 && has_is_empty(cx, &args[0]) {
span_lint_and_sugg(
cx,
LEN_ZERO,
span,
- "length comparison to zero",
+ &format!("length comparison to {}", if compare_to == 0 { "zero" } else { "one" }),
"using `is_empty` is more concise",
format!("{}{}.is_empty()", op, snippet(cx, args[0].span, "_")),
);
diff --git a/tests/ui/len_zero.rs b/tests/ui/len_zero.rs
index aba1dd3055a..2e71c2761fa 100644
--- a/tests/ui/len_zero.rs
+++ b/tests/ui/len_zero.rs
@@ -1,6 +1,3 @@
-
-
-
#![warn(len_without_is_empty, len_zero)]
#![allow(dead_code, unused)]
@@ -12,7 +9,8 @@ impl PubOne {
}
}
-impl PubOne { // A second impl for this struct - the error span shouldn't mention this
+impl PubOne {
+ // A second impl for this struct - the error span shouldn't mention this
pub fn irrelevant(self: &Self) -> bool {
false
}
@@ -39,7 +37,8 @@ impl PubAllowed {
struct NotPubOne;
impl NotPubOne {
- pub fn len(self: &Self) -> isize { // no error, len is pub but `NotPubOne` is not exported anyway
+ pub fn len(self: &Self) -> isize {
+ // no error, len is pub but `NotPubOne` is not exported anyway
1
}
}
@@ -47,7 +46,8 @@ impl NotPubOne {
struct One;
impl One {
- fn len(self: &Self) -> isize { // no error, len is private, see #1085
+ fn len(self: &Self) -> isize {
+ // no error, len is private, see #1085
1
}
}
@@ -120,7 +120,7 @@ impl HasWrongIsEmpty {
1
}
- pub fn is_empty(self: &Self, x : u32) -> bool {
+ pub fn is_empty(self: &Self, x: u32) -> bool {
false
}
}
@@ -129,28 +129,28 @@ pub trait Empty {
fn is_empty(&self) -> bool;
}
-pub trait InheritingEmpty: Empty { //must not trigger LEN_WITHOUT_IS_EMPTY
+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 {
println!("This should not happen!");
}
- if "".len() == 0 {
- }
+ if "".len() == 0 {}
let y = One;
- if y.len() == 0 { //no error because One does not have .is_empty()
+ if y.len() == 0 {
+ //no error because One does not have .is_empty()
println!("This should not happen either!");
}
- let z : &TraitsToo = &y;
- if z.len() > 0 { //no error, because TraitsToo has no .is_empty() method
+ let z: &TraitsToo = &y;
+ if z.len() > 0 {
+ //no error, because TraitsToo has no .is_empty() method
println!("Nor should this!");
}
@@ -164,6 +164,43 @@ fn main() {
if has_is_empty.len() > 0 {
println!("Or this!");
}
+ if has_is_empty.len() < 1 {
+ println!("Or this!");
+ }
+ if has_is_empty.len() >= 1 {
+ println!("Or this!");
+ }
+ if has_is_empty.len() > 1 {
+ // no error
+ println!("This can happen.");
+ }
+ if has_is_empty.len() <= 1 {
+ // no error
+ println!("This can happen.");
+ }
+ if 0 == has_is_empty.len() {
+ println!("Or this!");
+ }
+ if 0 != has_is_empty.len() {
+ println!("Or this!");
+ }
+ if 0 < has_is_empty.len() {
+ println!("Or this!");
+ }
+ if 1 <= has_is_empty.len() {
+ println!("Or this!");
+ }
+ if 1 > has_is_empty.len() {
+ println!("Or this!");
+ }
+ if 1 < has_is_empty.len() {
+ // no error
+ println!("This can happen.");
+ }
+ if 1 >= has_is_empty.len() {
+ // no error
+ println!("This can happen.");
+ }
assert!(!has_is_empty.is_empty());
let with_is_empty: &WithIsEmpty = &Wither;
@@ -173,14 +210,14 @@ fn main() {
assert!(!with_is_empty.is_empty());
let has_wrong_is_empty = HasWrongIsEmpty;
- if has_wrong_is_empty.len() == 0 { //no error as HasWrongIsEmpty does not have .is_empty()
+ if has_wrong_is_empty.len() == 0 {
+ //no error as HasWrongIsEmpty does not have .is_empty()
println!("Or this!");
}
}
fn test_slice(b: &[u8]) {
- if b.len() != 0 {
- }
+ if b.len() != 0 {}
}
// this used to ICE
diff --git a/tests/ui/len_zero.stderr b/tests/ui/len_zero.stderr
index 6e3cf1b3ca1..a04185bc63f 100644
--- a/tests/ui/len_zero.stderr
+++ b/tests/ui/len_zero.stderr
@@ -1,11 +1,11 @@
error: item `PubOne` has a public `len` method but no corresponding `is_empty` method
- --> $DIR/len_zero.rs:9:1
+ --> $DIR/len_zero.rs:6:1
|
-9 | / impl PubOne {
-10 | | pub fn len(self: &Self) -> isize {
-11 | | 1
-12 | | }
-13 | | }
+6 | / impl PubOne {
+7 | | pub fn len(self: &Self) -> isize {
+8 | | 1
+9 | | }
+10 | | }
| |_^
|
= note: `-D len-without-is-empty` implied by `-D warnings`
@@ -43,17 +43,17 @@ error: item `HasWrongIsEmpty` has a public `len` method but no corresponding `is
| |_^
error: length comparison to zero
- --> $DIR/len_zero.rs:140:8
+ --> $DIR/len_zero.rs:139:8
|
-140 | if x.len() == 0 {
+139 | 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:144:8
+ --> $DIR/len_zero.rs:143:8
|
-144 | if "".len() == 0 {
+143 | if "".len() == 0 {}
| ^^^^^^^^^^^^^ help: using `is_empty` is more concise: `"".is_empty()`
error: length comparison to zero
@@ -74,25 +74,67 @@ error: length comparison to zero
164 | if has_is_empty.len() > 0 {
| ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `!has_is_empty.is_empty()`
-error: length comparison to zero
+error: length comparison to one
+ --> $DIR/len_zero.rs:167:8
+ |
+167 | if has_is_empty.len() < 1 {
+ | ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `has_is_empty.is_empty()`
+
+error: length comparison to one
--> $DIR/len_zero.rs:170:8
|
-170 | if with_is_empty.len() == 0 {
+170 | if has_is_empty.len() >= 1 {
+ | ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `!has_is_empty.is_empty()`
+
+error: length comparison to zero
+ --> $DIR/len_zero.rs:181:8
+ |
+181 | if 0 == has_is_empty.len() {
+ | ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `has_is_empty.is_empty()`
+
+error: length comparison to zero
+ --> $DIR/len_zero.rs:184:8
+ |
+184 | if 0 != has_is_empty.len() {
+ | ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `!has_is_empty.is_empty()`
+
+error: length comparison to zero
+ --> $DIR/len_zero.rs:187:8
+ |
+187 | if 0 < has_is_empty.len() {
+ | ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `!has_is_empty.is_empty()`
+
+error: length comparison to one
+ --> $DIR/len_zero.rs:190:8
+ |
+190 | if 1 <= has_is_empty.len() {
+ | ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `!has_is_empty.is_empty()`
+
+error: length comparison to one
+ --> $DIR/len_zero.rs:193:8
+ |
+193 | if 1 > has_is_empty.len() {
+ | ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is more concise: `has_is_empty.is_empty()`
+
+error: length comparison to zero
+ --> $DIR/len_zero.rs:207:8
+ |
+207 | 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:182:8
+ --> $DIR/len_zero.rs:220:8
|
-182 | if b.len() != 0 {
+220 | if b.len() != 0 {}
| ^^^^^^^^^^^^ help: using `is_empty` is more concise: `!b.is_empty()`
error: trait `DependsOnFoo` has a `len` method but no (possibly inherited) `is_empty` method
- --> $DIR/len_zero.rs:189:1
+ --> $DIR/len_zero.rs:226:1
|
-189 | / pub trait DependsOnFoo: Foo {
-190 | | fn len(&mut self) -> usize;
-191 | | }
+226 | / pub trait DependsOnFoo: Foo {
+227 | | fn len(&mut self) -> usize;
+228 | | }
| |_^
-error: aborting due to 12 previous errors
+error: aborting due to 19 previous errors