new lint: iter_count

Author: TaKO8Ki

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/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<RustcVersion>,
 }
@@ -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<'_>,

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

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

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
+