Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New lint: iter_count #6791

Merged
merged 9 commits into from
Mar 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
47 changes: 47 additions & 0 deletions clippy_lints/src/methods/iter_count.rs
Original file line number Diff line number Diff line change
@@ -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;
};
TaKO8Ki marked this conversation as resolved.
Show resolved Hide resolved
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,
);
}
31 changes: 31 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>,
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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]),
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/auxiliary/option_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,8 @@ impl IteratorFalsePositives {
pub fn skip_while(self) -> IteratorFalsePositives {
self
}

pub fn count(self) -> usize {
self.foo as usize
}
}
86 changes: 86 additions & 0 deletions tests/ui/iter_count.fixed
Original file line number Diff line number Diff line change
@@ -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();
}
86 changes: 86 additions & 0 deletions tests/ui/iter_count.rs
Original file line number Diff line number Diff line change
@@ -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();
}
Loading