Auto merge of #6791 - TaKO8Ki:iter-count, r=matthiaskrgr

New lint: `iter_count`

This pull request adds a new lint named `iter_count`.

---

closes #6262

changelog: new lint `iter_count`

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

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),

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,
+    );
+}

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<RustcVersion>,
 }
@@ -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]),

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
+    }
 }

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();
+}

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();
+}