diff --git a/CHANGELOG.md b/CHANGELOG.md index 864321865c2..d46048b0e93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,11 @@ All notable changes to this project will be documented in this file. ## 0.0.88 — ?? -* The following lints are not new but were missing from the [`clippy`] and [`clippy_pedantic`] groups: [`filter_next`], [`for_loop_over_option`], [`for_loop_over_result`], [`match_overlapping_arm`]. +* The following lints are not new but were only usable through the `clippy` + lint groups: [`filter_next`], [`for_loop_over_option`], + [`for_loop_over_result`] and [`match_overlapping_arm`]. You should now be + able to `#[allow/deny]` them individually and they are available directly + through [`cargo clippy`]. ## 0.0.87 — 2016-08-31 * Rustup to *rustc 1.13.0-nightly (eac41469d 2016-08-30)* diff --git a/README.md b/README.md index d305908ea62..83b9788fba5 100644 --- a/README.md +++ b/README.md @@ -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 [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 -[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 [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 diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index 9b18eed93ee..fbcae087836 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -42,13 +42,13 @@ /// **Example:** /// ```rust /// impl X { -/// fn len(&self) -> usize { .. } +/// pub fn len(&self) -> usize { .. } /// } /// ``` declare_lint! { pub LEN_WITHOUT_IS_EMPTY, 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)] @@ -99,13 +99,12 @@ fn is_named_self(item: &TraitItem, name: &str) -> bool { } if !trait_items.iter().any(|i| is_named_self(i, "is_empty")) { - for i in trait_items { - if is_named_self(i, "len") { + if let Some(i) = trait_items.iter().find(|i| is_named_self(i, "len")) { + if cx.access_levels.is_exported(i.id) { span_lint(cx, LEN_WITHOUT_IS_EMPTY, i.span, - &format!("trait `{}` has a `.len(_: &Self)` method, but no `.is_empty(_: &Self)` method. \ - Consider adding one", + &format!("trait `{}` has a `len` method but no `is_empty` method", item.name)); } } @@ -122,19 +121,26 @@ fn is_named_self(item: &ImplItem, name: &str) -> bool { } } - if !impl_items.iter().any(|i| is_named_self(i, "is_empty")) { - for i in impl_items { - if is_named_self(i, "len") { - let ty = cx.tcx.node_id_to_type(item.id); + let is_empty = if let Some(is_empty) = impl_items.iter().find(|i| is_named_self(i, "is_empty")) { + if cx.access_levels.is_exported(is_empty.id) { + return; + } else { + "a private" + } + } else { + "no corresponding" + }; - span_lint(cx, - LEN_WITHOUT_IS_EMPTY, - i.span, - &format!("item `{}` has a `.len(_: &Self)` method, but no `.is_empty(_: &Self)` method. \ - Consider adding one", - ty)); - return; - } + if let Some(i) = impl_items.iter().find(|i| is_named_self(i, "len")) { + if cx.access_levels.is_exported(i.id) { + let ty = cx.tcx.node_id_to_type(item.id); + + span_lint(cx, + LEN_WITHOUT_IS_EMPTY, + i.span, + &format!("item `{}` has a public `len` method but {} `is_empty` method", + ty, + is_empty)); } } } diff --git a/tests/compile-fail/len_zero.rs b/tests/compile-fail/len_zero.rs index 4fd0203b912..b640c6db3a3 100644 --- a/tests/compile-fail/len_zero.rs +++ b/tests/compile-fail/len_zero.rs @@ -1,18 +1,45 @@ #![feature(plugin)] #![plugin(clippy)] -struct One; +#![deny(len_without_is_empty, len_zero)] +#![allow(dead_code, unused)] -#[deny(len_without_is_empty)] -impl One { - fn len(self: &Self) -> isize { //~ERROR item `One` has a `.len(_: &Self)` +pub struct PubOne; + +impl PubOne { + pub fn len(self: &Self) -> isize { //~ERROR item `PubOne` has a public `len` method but no corresponding `is_empty` 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 { - 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 { @@ -21,11 +48,22 @@ fn len(self: &Self) -> isize { } } -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 { - 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 } @@ -36,8 +74,7 @@ fn is_empty(self: &Self) -> bool { struct Wither; -#[deny(len_without_is_empty)] -trait WithIsEmpty { +pub trait WithIsEmpty { fn len(self: &Self) -> isize; fn is_empty(self: &Self) -> bool; } @@ -52,21 +89,18 @@ fn is_empty(self: &Self) -> bool { } } -struct HasWrongIsEmpty; +pub struct HasWrongIsEmpty; -#[deny(len_without_is_empty)] 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 } - #[allow(dead_code, unused)] - fn is_empty(self: &Self, x : u32) -> bool { + pub fn is_empty(self: &Self, x : u32) -> bool { false } } -#[deny(len_zero)] fn main() { let x = [1, 2]; if x.len() == 0 {