1
Fork 0

Restrict len_without_is_empty to exported items

This commit is contained in:
mcarton 2016-08-29 23:06:59 +02:00
parent 943e9b5aeb
commit b2de244cfd
No known key found for this signature in database
GPG key ID: 5E427C794CBA45E8
3 changed files with 76 additions and 36 deletions

View file

@ -84,7 +84,7 @@ name
[items_after_statements](https://github.com/Manishearth/rust-clippy/wiki#items_after_statements) | allow | blocks where an item comes after a statement [items_after_statements](https://github.com/Manishearth/rust-clippy/wiki#items_after_statements) | allow | blocks where an item comes after a statement
[iter_next_loop](https://github.com/Manishearth/rust-clippy/wiki#iter_next_loop) | warn | for-looping over `_.next()` which is probably not intended [iter_next_loop](https://github.com/Manishearth/rust-clippy/wiki#iter_next_loop) | warn | for-looping over `_.next()` which is probably not intended
[iter_nth](https://github.com/Manishearth/rust-clippy/wiki#iter_nth) | warn | using `.iter().nth()` on a standard library type with O(1) element access [iter_nth](https://github.com/Manishearth/rust-clippy/wiki#iter_nth) | warn | using `.iter().nth()` on a standard library type with O(1) element access
[len_without_is_empty](https://github.com/Manishearth/rust-clippy/wiki#len_without_is_empty) | warn | traits and impls that have `.len()` but not `.is_empty()` [len_without_is_empty](https://github.com/Manishearth/rust-clippy/wiki#len_without_is_empty) | warn | traits or impls with a public `len` method but no corresponding `is_empty` method
[len_zero](https://github.com/Manishearth/rust-clippy/wiki#len_zero) | warn | checking `.len() == 0` or `.len() > 0` (or similar) when `.is_empty()` could be used instead [len_zero](https://github.com/Manishearth/rust-clippy/wiki#len_zero) | warn | checking `.len() == 0` or `.len() > 0` (or similar) when `.is_empty()` could be used instead
[let_and_return](https://github.com/Manishearth/rust-clippy/wiki#let_and_return) | warn | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block [let_and_return](https://github.com/Manishearth/rust-clippy/wiki#let_and_return) | warn | creating a let-binding and then immediately returning it like `let x = expr; x` at the end of a block
[let_unit_value](https://github.com/Manishearth/rust-clippy/wiki#let_unit_value) | warn | creating a let binding to a value of unit type, which usually can't be used afterwards [let_unit_value](https://github.com/Manishearth/rust-clippy/wiki#let_unit_value) | warn | creating a let binding to a value of unit type, which usually can't be used afterwards

View file

@ -42,13 +42,13 @@ declare_lint! {
/// **Example:** /// **Example:**
/// ```rust /// ```rust
/// impl X { /// impl X {
/// fn len(&self) -> usize { .. } /// pub fn len(&self) -> usize { .. }
/// } /// }
/// ``` /// ```
declare_lint! { declare_lint! {
pub LEN_WITHOUT_IS_EMPTY, pub LEN_WITHOUT_IS_EMPTY,
Warn, Warn,
"traits and impls that have `.len()` but not `.is_empty()`" "traits or impls with a public `len` method but no corresponding `is_empty` method"
} }
#[derive(Copy,Clone)] #[derive(Copy,Clone)]
@ -99,13 +99,12 @@ fn check_trait_items(cx: &LateContext, item: &Item, trait_items: &[TraitItem]) {
} }
if !trait_items.iter().any(|i| is_named_self(i, "is_empty")) { if !trait_items.iter().any(|i| is_named_self(i, "is_empty")) {
for i in trait_items { if let Some(i) = trait_items.iter().find(|i| is_named_self(i, "len")) {
if is_named_self(i, "len") { if cx.access_levels.is_exported(i.id) {
span_lint(cx, span_lint(cx,
LEN_WITHOUT_IS_EMPTY, LEN_WITHOUT_IS_EMPTY,
i.span, i.span,
&format!("trait `{}` has a `.len(_: &Self)` method, but no `.is_empty(_: &Self)` method. \ &format!("trait `{}` has a `len` method but no `is_empty` method",
Consider adding one",
item.name)); item.name));
} }
} }
@ -122,19 +121,26 @@ fn check_impl_items(cx: &LateContext, item: &Item, impl_items: &[ImplItem]) {
} }
} }
if !impl_items.iter().any(|i| is_named_self(i, "is_empty")) { let is_empty = if let Some(is_empty) = impl_items.iter().find(|i| is_named_self(i, "is_empty")) {
for i in impl_items { if cx.access_levels.is_exported(is_empty.id) {
if is_named_self(i, "len") { return;
let ty = cx.tcx.node_id_to_type(item.id); } else {
"a private"
}
} else {
"no corresponding"
};
span_lint(cx, if let Some(i) = impl_items.iter().find(|i| is_named_self(i, "len")) {
LEN_WITHOUT_IS_EMPTY, if cx.access_levels.is_exported(i.id) {
i.span, let ty = cx.tcx.node_id_to_type(item.id);
&format!("item `{}` has a `.len(_: &Self)` method, but no `.is_empty(_: &Self)` method. \
Consider adding one", span_lint(cx,
ty)); LEN_WITHOUT_IS_EMPTY,
return; i.span,
} &format!("item `{}` has a public `len` method but {} `is_empty` method",
ty,
is_empty));
} }
} }
} }

View file

@ -1,18 +1,45 @@
#![feature(plugin)] #![feature(plugin)]
#![plugin(clippy)] #![plugin(clippy)]
struct One; #![deny(len_without_is_empty, len_zero)]
#![allow(dead_code, unused)]
#[deny(len_without_is_empty)] pub struct PubOne;
impl One {
fn len(self: &Self) -> isize { //~ERROR item `One` has a `.len(_: &Self)` impl PubOne {
pub fn len(self: &Self) -> isize { //~ERROR item `PubOne` has a public `len` method but no corresponding `is_empty`
1 1
} }
} }
#[deny(len_without_is_empty)] struct NotPubOne;
impl NotPubOne {
pub fn len(self: &Self) -> isize { // no error, len is pub but `NotPubOne` is not exported anyway
1
}
}
struct One;
impl One {
fn len(self: &Self) -> isize { // no error, len is private, see #1085
1
}
}
pub trait PubTraitsToo {
fn len(self: &Self) -> isize; //~ERROR trait `PubTraitsToo` has a `len` method but no `is_empty`
}
impl PubTraitsToo for One {
fn len(self: &Self) -> isize {
0
}
}
trait TraitsToo { trait TraitsToo {
fn len(self: &Self) -> isize; //~ERROR trait `TraitsToo` has a `.len(_: fn len(self: &Self) -> isize; // no error, len is private, see #1085
} }
impl TraitsToo for One { impl TraitsToo for One {
@ -21,11 +48,22 @@ impl TraitsToo for One {
} }
} }
struct HasIsEmpty; struct HasPrivateIsEmpty;
impl HasPrivateIsEmpty {
pub fn len(self: &Self) -> isize {
1
}
fn is_empty(self: &Self) -> bool {
false
}
}
pub struct HasIsEmpty;
#[deny(len_without_is_empty)]
impl HasIsEmpty { impl HasIsEmpty {
fn len(self: &Self) -> isize { pub fn len(self: &Self) -> isize { //~ERROR item `HasIsEmpty` has a public `len` method but a private `is_empty`
1 1
} }
@ -36,8 +74,7 @@ impl HasIsEmpty {
struct Wither; struct Wither;
#[deny(len_without_is_empty)] pub trait WithIsEmpty {
trait WithIsEmpty {
fn len(self: &Self) -> isize; fn len(self: &Self) -> isize;
fn is_empty(self: &Self) -> bool; fn is_empty(self: &Self) -> bool;
} }
@ -52,21 +89,18 @@ impl WithIsEmpty for Wither {
} }
} }
struct HasWrongIsEmpty; pub struct HasWrongIsEmpty;
#[deny(len_without_is_empty)]
impl HasWrongIsEmpty { impl HasWrongIsEmpty {
fn len(self: &Self) -> isize { //~ERROR item `HasWrongIsEmpty` has a `.len(_: &Self)` pub fn len(self: &Self) -> isize { //~ERROR item `HasWrongIsEmpty` has a public `len` method but no corresponding `is_empty`
1 1
} }
#[allow(dead_code, unused)] pub fn is_empty(self: &Self, x : u32) -> bool {
fn is_empty(self: &Self, x : u32) -> bool {
false false
} }
} }
#[deny(len_zero)]
fn main() { fn main() {
let x = [1, 2]; let x = [1, 2];
if x.len() == 0 { if x.len() == 0 {