diff --git a/CHANGELOG.md b/CHANGELOG.md index 687f7447202..41c334c6816 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2135,6 +2135,7 @@ Released 2018-09-13 [`invisible_characters`]: https://rust-lang.github.io/rust-clippy/master/index.html#invisible_characters [`items_after_statements`]: https://rust-lang.github.io/rust-clippy/master/index.html#items_after_statements [`iter_cloned_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_cloned_collect +[`iter_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_count [`iter_next_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_loop [`iter_next_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_slice [`iter_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index fa8f03eb445..1ace4c8a10c 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -775,6 +775,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &methods::INTO_ITER_ON_REF, &methods::ITERATOR_STEP_BY_ZERO, &methods::ITER_CLONED_COLLECT, + &methods::ITER_COUNT, &methods::ITER_NEXT_SLICE, &methods::ITER_NTH, &methods::ITER_NTH_ZERO, @@ -1577,6 +1578,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&methods::INTO_ITER_ON_REF), LintId::of(&methods::ITERATOR_STEP_BY_ZERO), LintId::of(&methods::ITER_CLONED_COLLECT), + LintId::of(&methods::ITER_COUNT), LintId::of(&methods::ITER_NEXT_SLICE), LintId::of(&methods::ITER_NTH), LintId::of(&methods::ITER_NTH_ZERO), @@ -1881,6 +1883,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&methods::FILTER_NEXT), LintId::of(&methods::FLAT_MAP_IDENTITY), LintId::of(&methods::INSPECT_FOR_EACH), + LintId::of(&methods::ITER_COUNT), LintId::of(&methods::MANUAL_FILTER_MAP), LintId::of(&methods::MANUAL_FIND_MAP), LintId::of(&methods::OPTION_AS_REF_DEREF), diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 6f491144435..17e7f6f662d 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1540,6 +1540,32 @@ declare_clippy_lint! { "implicitly cloning a value by invoking a function on its dereferenced type" } +declare_clippy_lint! { + /// **What it does:** Checks for the use of `.iter().count()`. + /// + /// **Why is this bad?** `.len()` is more efficient and more + /// readable. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// // Bad + /// let some_vec = vec![0, 1, 2, 3]; + /// let _ = some_vec.iter().count(); + /// let _ = &some_vec[..].iter().count(); + /// + /// // Good + /// let some_vec = vec![0, 1, 2, 3]; + /// let _ = some_vec.len(); + /// let _ = &some_vec[..].len(); + /// ``` + pub ITER_COUNT, + complexity, + "replace `.iter().count()` with `.len()`" +} + pub struct Methods { msrv: Option, } @@ -1585,6 +1611,7 @@ impl_lint_pass!(Methods => [ MAP_FLATTEN, ITERATOR_STEP_BY_ZERO, ITER_NEXT_SLICE, + ITER_COUNT, ITER_NTH, ITER_NTH_ZERO, BYTES_NTH, @@ -1664,6 +1691,8 @@ impl<'tcx> LateLintPass<'tcx> for Methods { lint_search_is_some(cx, expr, "rposition", arg_lists[1], arg_lists[0], method_spans[1]) }, ["extend", ..] => lint_extend(cx, expr, arg_lists[0]), + ["count", "iter"] => lint_iter_count(cx, expr, &arg_lists[1], false), + ["count", "iter_mut"] => lint_iter_count(cx, expr, &arg_lists[1], true), ["nth", "iter"] => lint_iter_nth(cx, expr, &arg_lists, false), ["nth", "iter_mut"] => lint_iter_nth(cx, expr, &arg_lists, true), ["nth", "bytes"] => bytes_nth::lints(cx, expr, &arg_lists[1]), @@ -2632,6 +2661,39 @@ fn lint_iter_next<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, iter_ } } +fn lint_iter_count<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'tcx [Expr<'tcx>], is_mut: bool) { + let mut_str = if is_mut { "_mut" } else { "" }; + if_chain! { + let caller_type = if derefs_to_slice(cx, &iter_args[0], cx.typeck_results().expr_ty(&iter_args[0])).is_some() { + Some("slice") + } else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&iter_args[0]), sym::vec_type) { + Some("Vec") + } else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&iter_args[0]), sym!(vecdeque_type)) { + Some("VecDeque") + } else if match_trait_method(cx, expr, &paths::ITERATOR) { + Some("std::iter::Iterator") + } else { + None + }; + if let Some(caller_type) = caller_type; + then { + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + ITER_COUNT, + expr.span, + &format!("called `.iter{}().count()` on a `{}`", mut_str, caller_type), + "try", + format!( + "{}.len()", + snippet_with_applicability(cx, iter_args[0].span, "..", &mut applicability), + ), + applicability, + ); + } + } +} + fn lint_iter_nth<'tcx>( cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, diff --git a/tests/ui/auxiliary/option_helpers.rs b/tests/ui/auxiliary/option_helpers.rs index ed11c41e21c..7dc3f4ebd4d 100644 --- a/tests/ui/auxiliary/option_helpers.rs +++ b/tests/ui/auxiliary/option_helpers.rs @@ -48,4 +48,8 @@ impl IteratorFalsePositives { pub fn skip_while(self) -> IteratorFalsePositives { self } + + pub fn count(self) -> usize { + self.foo as usize + } } diff --git a/tests/ui/iter_count.fixed b/tests/ui/iter_count.fixed new file mode 100644 index 00000000000..14cae1cd73e --- /dev/null +++ b/tests/ui/iter_count.fixed @@ -0,0 +1,58 @@ +// run-rustfix +// aux-build:option_helpers.rs + +#![warn(clippy::iter_count)] +#![allow(unused_variables)] +#![allow(unused_mut)] + +extern crate option_helpers; + +use option_helpers::IteratorFalsePositives; +use std::collections::{HashSet, VecDeque}; + +/// Struct to generate false positives for things with `.iter()`. +#[derive(Copy, Clone)] +struct HasIter; + +impl HasIter { + fn iter(self) -> IteratorFalsePositives { + IteratorFalsePositives { foo: 0 } + } + + fn iter_mut(self) -> IteratorFalsePositives { + IteratorFalsePositives { foo: 0 } + } +} + +fn main() { + let mut some_vec = vec![0, 1, 2, 3]; + let mut boxed_slice: Box<[u8]> = Box::new([0, 1, 2, 3]); + let mut some_vec_deque: VecDeque<_> = some_vec.iter().cloned().collect(); + let mut some_hash_set = HashSet::new(); + some_hash_set.insert(1); + + { + // Make sure we lint `.iter()` for relevant types. + let bad_vec = some_vec.len(); + let bad_slice = &some_vec[..].len(); + let bad_boxed_slice = boxed_slice.len(); + let bad_vec_deque = some_vec_deque.len(); + let bad_hash_set = some_hash_set.len(); + } + + { + // Make sure we lint `.iter_mut()` for relevant types. + let bad_vec = some_vec.len(); + } + { + let bad_slice = &some_vec[..].len(); + } + { + let bad_vec_deque = some_vec_deque.len(); + } + + // Make sure we don't lint for non-relevant types. + let false_positive = HasIter; + let ok = false_positive.iter().count(); + let ok_mut = false_positive.iter_mut().count(); +} diff --git a/tests/ui/iter_count.rs b/tests/ui/iter_count.rs new file mode 100644 index 00000000000..dfe02e5d53e --- /dev/null +++ b/tests/ui/iter_count.rs @@ -0,0 +1,58 @@ +// run-rustfix +// aux-build:option_helpers.rs + +#![warn(clippy::iter_count)] +#![allow(unused_variables)] +#![allow(unused_mut)] + +extern crate option_helpers; + +use option_helpers::IteratorFalsePositives; +use std::collections::{HashSet, VecDeque}; + +/// Struct to generate false positives for things with `.iter()`. +#[derive(Copy, Clone)] +struct HasIter; + +impl HasIter { + fn iter(self) -> IteratorFalsePositives { + IteratorFalsePositives { foo: 0 } + } + + fn iter_mut(self) -> IteratorFalsePositives { + IteratorFalsePositives { foo: 0 } + } +} + +fn main() { + let mut some_vec = vec![0, 1, 2, 3]; + let mut boxed_slice: Box<[u8]> = Box::new([0, 1, 2, 3]); + let mut some_vec_deque: VecDeque<_> = some_vec.iter().cloned().collect(); + let mut some_hash_set = HashSet::new(); + some_hash_set.insert(1); + + { + // Make sure we lint `.iter()` for relevant types. + let bad_vec = some_vec.iter().count(); + let bad_slice = &some_vec[..].iter().count(); + let bad_boxed_slice = boxed_slice.iter().count(); + let bad_vec_deque = some_vec_deque.iter().count(); + let bad_hash_set = some_hash_set.iter().count(); + } + + { + // Make sure we lint `.iter_mut()` for relevant types. + let bad_vec = some_vec.iter_mut().count(); + } + { + let bad_slice = &some_vec[..].iter_mut().count(); + } + { + let bad_vec_deque = some_vec_deque.iter_mut().count(); + } + + // Make sure we don't lint for non-relevant types. + let false_positive = HasIter; + let ok = false_positive.iter().count(); + let ok_mut = false_positive.iter_mut().count(); +} diff --git a/tests/ui/iter_count.stderr b/tests/ui/iter_count.stderr new file mode 100644 index 00000000000..a465d221994 --- /dev/null +++ b/tests/ui/iter_count.stderr @@ -0,0 +1,52 @@ +error: called `.iter().count()` on a `Vec` + --> $DIR/iter_count.rs:36:23 + | +LL | let bad_vec = some_vec.iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_vec.len()` + | + = note: `-D clippy::iter-count` implied by `-D warnings` + +error: called `.iter().count()` on a `slice` + --> $DIR/iter_count.rs:37:26 + | +LL | let bad_slice = &some_vec[..].iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_vec[..].len()` + +error: called `.iter().count()` on a `slice` + --> $DIR/iter_count.rs:38:31 + | +LL | let bad_boxed_slice = boxed_slice.iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `boxed_slice.len()` + +error: called `.iter().count()` on a `VecDeque` + --> $DIR/iter_count.rs:39:29 + | +LL | let bad_vec_deque = some_vec_deque.iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_vec_deque.len()` + +error: called `.iter().count()` on a `std::iter::Iterator` + --> $DIR/iter_count.rs:40:28 + | +LL | let bad_hash_set = some_hash_set.iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_hash_set.len()` + +error: called `.iter_mut().count()` on a `Vec` + --> $DIR/iter_count.rs:45:23 + | +LL | let bad_vec = some_vec.iter_mut().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_vec.len()` + +error: called `.iter_mut().count()` on a `slice` + --> $DIR/iter_count.rs:48:26 + | +LL | let bad_slice = &some_vec[..].iter_mut().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_vec[..].len()` + +error: called `.iter_mut().count()` on a `VecDeque` + --> $DIR/iter_count.rs:51:29 + | +LL | let bad_vec_deque = some_vec_deque.iter_mut().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_vec_deque.len()` + +error: aborting due to 8 previous errors +