diff --git a/CHANGELOG.md b/CHANGELOG.md index 687f74472020..41c334c68169 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 fa8f03eb4453..1ace4c8a10c9 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/iter_count.rs b/clippy_lints/src/methods/iter_count.rs new file mode 100644 index 000000000000..1b99bacc3f1c --- /dev/null +++ b/clippy_lints/src/methods/iter_count.rs @@ -0,0 +1,47 @@ +use crate::methods::derefs_to_slice; +use crate::utils::{is_type_diagnostic_item, match_type, paths, snippet_with_applicability, span_lint_and_sugg}; + +use rustc_errors::Applicability; +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_span::sym; + +use super::ITER_COUNT; + +pub(crate) fn lints<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'tcx [Expr<'tcx>], iter_method: &str) { + let ty = cx.typeck_results().expr_ty(&iter_args[0]); + let caller_type = if derefs_to_slice(cx, &iter_args[0], ty).is_some() { + "slice" + } else if is_type_diagnostic_item(cx, ty, sym::vec_type) { + "Vec" + } else if is_type_diagnostic_item(cx, ty, sym!(vecdeque_type)) { + "VecDeque" + } else if is_type_diagnostic_item(cx, ty, sym!(hashset_type)) { + "HashSet" + } else if is_type_diagnostic_item(cx, ty, sym!(hashmap_type)) { + "HashMap" + } else if match_type(cx, ty, &paths::BTREEMAP) { + "BTreeMap" + } else if match_type(cx, ty, &paths::BTREESET) { + "BTreeSet" + } else if match_type(cx, ty, &paths::LINKED_LIST) { + "LinkedList" + } else if match_type(cx, ty, &paths::BINARY_HEAP) { + "BinaryHeap" + } else { + return; + }; + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + ITER_COUNT, + expr.span, + &format!("called `.{}().count()` on a `{}`", iter_method, caller_type), + "try", + format!( + "{}.len()", + snippet_with_applicability(cx, iter_args[0].span, "..", &mut applicability), + ), + applicability, + ); +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 6f491144435a..4a63de3cf48d 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -4,6 +4,7 @@ mod filter_map_identity; mod implicit_clone; mod inefficient_to_string; mod inspect_for_each; +mod iter_count; mod manual_saturating_arithmetic; mod option_map_unwrap_or; mod unnecessary_filter_map; @@ -1540,6 +1541,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 +1612,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 +1692,9 @@ 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", "into_iter"] => iter_count::lints(cx, expr, &arg_lists[1], "into_iter"), + ["count", "iter"] => iter_count::lints(cx, expr, &arg_lists[1], "iter"), + ["count", "iter_mut"] => iter_count::lints(cx, expr, &arg_lists[1], "iter_mut"), ["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]), diff --git a/tests/ui/auxiliary/option_helpers.rs b/tests/ui/auxiliary/option_helpers.rs index ed11c41e21c1..7dc3f4ebd4d4 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 000000000000..b11dadda6c24 --- /dev/null +++ b/tests/ui/iter_count.fixed @@ -0,0 +1,86 @@ +// run-rustfix +// aux-build:option_helpers.rs + +#![warn(clippy::iter_count)] +#![allow( + unused_variables, + array_into_iter, + unused_mut, + clippy::into_iter_on_ref, + clippy::unnecessary_operation +)] + +extern crate option_helpers; + +use option_helpers::IteratorFalsePositives; +use std::collections::{BTreeMap, BTreeSet, BinaryHeap, HashMap, HashSet, LinkedList, 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 into_iter(self) -> IteratorFalsePositives { + IteratorFalsePositives { foo: 0 } + } +} + +fn main() { + let mut vec = vec![0, 1, 2, 3]; + let mut boxed_slice: Box<[u8]> = Box::new([0, 1, 2, 3]); + let mut vec_deque: VecDeque<_> = vec.iter().cloned().collect(); + let mut hash_set = HashSet::new(); + let mut hash_map = HashMap::new(); + let mut b_tree_map = BTreeMap::new(); + let mut b_tree_set = BTreeSet::new(); + let mut linked_list = LinkedList::new(); + let mut binary_heap = BinaryHeap::new(); + hash_set.insert(1); + hash_map.insert(1, 2); + b_tree_map.insert(1, 2); + b_tree_set.insert(1); + linked_list.push_back(1); + binary_heap.push(1); + + &vec[..].len(); + vec.len(); + boxed_slice.len(); + vec_deque.len(); + hash_set.len(); + hash_map.len(); + b_tree_map.len(); + b_tree_set.len(); + linked_list.len(); + binary_heap.len(); + + vec.len(); + &vec[..].len(); + vec_deque.len(); + hash_map.len(); + b_tree_map.len(); + linked_list.len(); + + &vec[..].len(); + vec.len(); + vec_deque.len(); + hash_set.len(); + hash_map.len(); + b_tree_map.len(); + b_tree_set.len(); + linked_list.len(); + binary_heap.len(); + + // Make sure we don't lint for non-relevant types. + let false_positive = HasIter; + false_positive.iter().count(); + false_positive.iter_mut().count(); + false_positive.into_iter().count(); +} diff --git a/tests/ui/iter_count.rs b/tests/ui/iter_count.rs new file mode 100644 index 000000000000..7d49c6a3dbbb --- /dev/null +++ b/tests/ui/iter_count.rs @@ -0,0 +1,86 @@ +// run-rustfix +// aux-build:option_helpers.rs + +#![warn(clippy::iter_count)] +#![allow( + unused_variables, + array_into_iter, + unused_mut, + clippy::into_iter_on_ref, + clippy::unnecessary_operation +)] + +extern crate option_helpers; + +use option_helpers::IteratorFalsePositives; +use std::collections::{BTreeMap, BTreeSet, BinaryHeap, HashMap, HashSet, LinkedList, 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 into_iter(self) -> IteratorFalsePositives { + IteratorFalsePositives { foo: 0 } + } +} + +fn main() { + let mut vec = vec![0, 1, 2, 3]; + let mut boxed_slice: Box<[u8]> = Box::new([0, 1, 2, 3]); + let mut vec_deque: VecDeque<_> = vec.iter().cloned().collect(); + let mut hash_set = HashSet::new(); + let mut hash_map = HashMap::new(); + let mut b_tree_map = BTreeMap::new(); + let mut b_tree_set = BTreeSet::new(); + let mut linked_list = LinkedList::new(); + let mut binary_heap = BinaryHeap::new(); + hash_set.insert(1); + hash_map.insert(1, 2); + b_tree_map.insert(1, 2); + b_tree_set.insert(1); + linked_list.push_back(1); + binary_heap.push(1); + + &vec[..].iter().count(); + vec.iter().count(); + boxed_slice.iter().count(); + vec_deque.iter().count(); + hash_set.iter().count(); + hash_map.iter().count(); + b_tree_map.iter().count(); + b_tree_set.iter().count(); + linked_list.iter().count(); + binary_heap.iter().count(); + + vec.iter_mut().count(); + &vec[..].iter_mut().count(); + vec_deque.iter_mut().count(); + hash_map.iter_mut().count(); + b_tree_map.iter_mut().count(); + linked_list.iter_mut().count(); + + &vec[..].into_iter().count(); + vec.into_iter().count(); + vec_deque.into_iter().count(); + hash_set.into_iter().count(); + hash_map.into_iter().count(); + b_tree_map.into_iter().count(); + b_tree_set.into_iter().count(); + linked_list.into_iter().count(); + binary_heap.into_iter().count(); + + // Make sure we don't lint for non-relevant types. + let false_positive = HasIter; + false_positive.iter().count(); + false_positive.iter_mut().count(); + false_positive.into_iter().count(); +} diff --git a/tests/ui/iter_count.stderr b/tests/ui/iter_count.stderr new file mode 100644 index 000000000000..f3fb98e65b99 --- /dev/null +++ b/tests/ui/iter_count.stderr @@ -0,0 +1,154 @@ +error: called `.iter().count()` on a `slice` + --> $DIR/iter_count.rs:53:6 + | +LL | &vec[..].iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec[..].len()` + | + = note: `-D clippy::iter-count` implied by `-D warnings` + +error: called `.iter().count()` on a `Vec` + --> $DIR/iter_count.rs:54:5 + | +LL | vec.iter().count(); + | ^^^^^^^^^^^^^^^^^^ help: try: `vec.len()` + +error: called `.iter().count()` on a `slice` + --> $DIR/iter_count.rs:55:5 + | +LL | boxed_slice.iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `boxed_slice.len()` + +error: called `.iter().count()` on a `VecDeque` + --> $DIR/iter_count.rs:56:5 + | +LL | vec_deque.iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec_deque.len()` + +error: called `.iter().count()` on a `HashSet` + --> $DIR/iter_count.rs:57:5 + | +LL | hash_set.iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `hash_set.len()` + +error: called `.iter().count()` on a `HashMap` + --> $DIR/iter_count.rs:58:5 + | +LL | hash_map.iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `hash_map.len()` + +error: called `.iter().count()` on a `BTreeMap` + --> $DIR/iter_count.rs:59:5 + | +LL | b_tree_map.iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b_tree_map.len()` + +error: called `.iter().count()` on a `BTreeSet` + --> $DIR/iter_count.rs:60:5 + | +LL | b_tree_set.iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b_tree_set.len()` + +error: called `.iter().count()` on a `LinkedList` + --> $DIR/iter_count.rs:61:5 + | +LL | linked_list.iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `linked_list.len()` + +error: called `.iter().count()` on a `BinaryHeap` + --> $DIR/iter_count.rs:62:5 + | +LL | binary_heap.iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `binary_heap.len()` + +error: called `.iter_mut().count()` on a `Vec` + --> $DIR/iter_count.rs:64:5 + | +LL | vec.iter_mut().count(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.len()` + +error: called `.iter_mut().count()` on a `slice` + --> $DIR/iter_count.rs:65:6 + | +LL | &vec[..].iter_mut().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec[..].len()` + +error: called `.iter_mut().count()` on a `VecDeque` + --> $DIR/iter_count.rs:66:5 + | +LL | vec_deque.iter_mut().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec_deque.len()` + +error: called `.iter_mut().count()` on a `HashMap` + --> $DIR/iter_count.rs:67:5 + | +LL | hash_map.iter_mut().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `hash_map.len()` + +error: called `.iter_mut().count()` on a `BTreeMap` + --> $DIR/iter_count.rs:68:5 + | +LL | b_tree_map.iter_mut().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b_tree_map.len()` + +error: called `.iter_mut().count()` on a `LinkedList` + --> $DIR/iter_count.rs:69:5 + | +LL | linked_list.iter_mut().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `linked_list.len()` + +error: called `.into_iter().count()` on a `slice` + --> $DIR/iter_count.rs:71:6 + | +LL | &vec[..].into_iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec[..].len()` + +error: called `.into_iter().count()` on a `Vec` + --> $DIR/iter_count.rs:72:5 + | +LL | vec.into_iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.len()` + +error: called `.into_iter().count()` on a `VecDeque` + --> $DIR/iter_count.rs:73:5 + | +LL | vec_deque.into_iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec_deque.len()` + +error: called `.into_iter().count()` on a `HashSet` + --> $DIR/iter_count.rs:74:5 + | +LL | hash_set.into_iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `hash_set.len()` + +error: called `.into_iter().count()` on a `HashMap` + --> $DIR/iter_count.rs:75:5 + | +LL | hash_map.into_iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `hash_map.len()` + +error: called `.into_iter().count()` on a `BTreeMap` + --> $DIR/iter_count.rs:76:5 + | +LL | b_tree_map.into_iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b_tree_map.len()` + +error: called `.into_iter().count()` on a `BTreeSet` + --> $DIR/iter_count.rs:77:5 + | +LL | b_tree_set.into_iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `b_tree_set.len()` + +error: called `.into_iter().count()` on a `LinkedList` + --> $DIR/iter_count.rs:78:5 + | +LL | linked_list.into_iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `linked_list.len()` + +error: called `.into_iter().count()` on a `BinaryHeap` + --> $DIR/iter_count.rs:79:5 + | +LL | binary_heap.into_iter().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `binary_heap.len()` + +error: aborting due to 25 previous errors + diff --git a/tests/ui/needless_collect.fixed b/tests/ui/needless_collect.fixed index 7f2fcf02f6b5..af6c7bf15ea6 100644 --- a/tests/ui/needless_collect.fixed +++ b/tests/ui/needless_collect.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused, clippy::suspicious_map)] +#![allow(unused, clippy::suspicious_map, clippy::iter_count)] use std::collections::{BTreeSet, HashMap, HashSet}; diff --git a/tests/ui/needless_collect.rs b/tests/ui/needless_collect.rs index 788a9eb3264e..6ae14f370b14 100644 --- a/tests/ui/needless_collect.rs +++ b/tests/ui/needless_collect.rs @@ -1,6 +1,6 @@ // run-rustfix -#![allow(unused, clippy::suspicious_map)] +#![allow(unused, clippy::suspicious_map, clippy::iter_count)] use std::collections::{BTreeSet, HashMap, HashSet};