Skip to content

Commit

Permalink
add lint for checking unnecessary iter().any()
Browse files Browse the repository at this point in the history
  • Loading branch information
@lapla-cogito
lapla-cogito committed Dec 14, 2024
1 parent 85d3dcc commit fe5f2c4
Show file tree
Hide file tree
Showing 10 changed files with 139 additions and 82 deletions.
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::methods::UNNECESSARY_FIRST_THEN_CHECK_INFO,
crate::methods::UNNECESSARY_FOLD_INFO,
crate::methods::UNNECESSARY_GET_THEN_CHECK_INFO,
crate::methods::UNNECESSARY_ITER_ANY_INFO,
crate::methods::UNNECESSARY_JOIN_INFO,
crate::methods::UNNECESSARY_LAZY_EVALUATIONS_INFO,
crate::methods::UNNECESSARY_LITERAL_UNWRAP_INFO,
Expand Down
50 changes: 50 additions & 0 deletions clippy_lints/src/methods/iter_any.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use clippy_utils::diagnostics::span_lint;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_middle::ty::{self};

use super::{SLICE_ITER_ANY, UNNECESSARY_ITER_ANY, method_call};

pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) {
if !expr.span.from_expansion()
// any()
&& let Some((name, recv, args, _, _)) = method_call(expr)
&& name == "any"
// check if the inner closure is a equality check
&& args.len() == 1
&& let ExprKind::Closure(closure) = args[0].kind
&& let body = cx.tcx.hir().body(closure.body)
&& let ExprKind::Binary(op, _, _) = body.value.kind
&& op.node == rustc_ast::ast::BinOpKind::Eq
// iter()
&& let Some((name, recv, _, _, _)) = method_call(recv)
&& name == "iter"
{
let ref_type = cx.typeck_results().expr_ty(recv);

match ref_type.kind() {
ty::Ref(_, inner_type, _) if inner_type.is_slice() => {
// check if the receiver is a u8/i8 slice
if let ty::Slice(slice_type) = inner_type.kind()
&& (slice_type.to_string() == "u8" || slice_type.to_string() == "i8")
{
span_lint(
cx,
SLICE_ITER_ANY,
expr.span,
"using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient",
);
}
},
// if it's an array that uses `iter().any()` and its closure is an equality check, suggest using
// `contains()`
ty::Array(array_type, _) if array_type.is_numeric() => span_lint(
cx,
UNNECESSARY_ITER_ANY,
expr.span,
"using `contains()` instead of `iter().any()` is more readable",
),
_ => {},
}
}
}
27 changes: 25 additions & 2 deletions clippy_lints/src/methods/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ mod inspect_for_each;
mod into_iter_on_ref;
mod is_digit_ascii_radix;
mod is_empty;
mod iter_any;
mod iter_cloned_collect;
mod iter_count;
mod iter_filter;
Expand Down Expand Up @@ -100,7 +101,6 @@ mod single_char_add_str;
mod single_char_insert_string;
mod single_char_push_string;
mod skip_while_next;
mod slice_iter_any;
mod stable_sort_primitive;
mod str_split;
mod str_splitn;
Expand Down Expand Up @@ -4310,6 +4310,29 @@ declare_clippy_lint! {
"using `contains()` instead of `iter().any()` on `u8`/`i8` slices is more fast"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `iter().any()` when it can be replaced with `contains()` and suggests doing so.
///
/// ### Why is this bad?
/// It makes the code less readable.
///
/// ### Example
/// ```no_run
/// let values = &[1, 2, 3];
/// let _ = values.iter().any(|&v| v == 2);
/// ```
/// Use instead:
/// ```no_run
/// let values = &[1, 2, 3];
/// let _ = values.contains(&2);
/// ```
#[clippy::version = "1.85.0"]
pub UNNECESSARY_ITER_ANY,
style,
"using `contains()` instead of `iter().any()` is more readable"
}

pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Msrv,
Expand Down Expand Up @@ -4710,7 +4733,7 @@ impl Methods {
("any", [arg]) => {
unused_enumerate_index::check(cx, expr, recv, arg);
needless_character_iteration::check(cx, expr, recv, arg, false);
slice_iter_any::check(cx, expr);
iter_any::check(cx, expr);
match method_call(recv) {
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(
cx,
Expand Down
42 changes: 0 additions & 42 deletions clippy_lints/src/methods/slice_iter_any.rs
View file

This file was deleted.

0 tests/ui/slice_iter_any.rs → tests/ui/iter_any.rs
View file
File renamed without changes.
32 changes: 32 additions & 0 deletions tests/ui/iter_any.stderr
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
error: using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient
--> tests/ui/iter_any.rs:6:13
|
LL | let _ = values.iter().any(|&v| v == 4);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::slice-iter-any` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::slice_iter_any)]`

error: using `contains()` instead of `iter().any()` is more readable
--> tests/ui/iter_any.rs:20:13
|
LL | let _ = values.iter().any(|&v| v == 10);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::unnecessary-iter-any` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_iter_any)]`

error: using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient
--> tests/ui/iter_any.rs:29:5
|
LL | values.iter().any(|&v| v == 10)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: using `contains()` instead of `iter().any()` is more readable
--> tests/ui/iter_any.rs:34:5
|
LL | values.iter().any(|&v| v == 10)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 4 previous errors

7 changes: 6 additions & 1 deletion tests/ui/needless_collect.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
use std::collections::{BTreeMap, BTreeSet, BinaryHeap, HashMap, HashSet, LinkedList};

#[warn(clippy::needless_collect)]
#[allow(unused_variables, clippy::iter_cloned_collect, clippy::iter_next_slice)]
#[allow(
unused_variables,
clippy::unnecessary_iter_any,
clippy::iter_cloned_collect,
clippy::iter_next_slice
)]
fn main() {
let sample = [1; 5];
let len = sample.iter().count();
Expand Down
7 changes: 6 additions & 1 deletion tests/ui/needless_collect.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
use std::collections::{BTreeMap, BTreeSet, BinaryHeap, HashMap, HashSet, LinkedList};

#[warn(clippy::needless_collect)]
#[allow(unused_variables, clippy::iter_cloned_collect, clippy::iter_next_slice)]
#[allow(
unused_variables,
clippy::unnecessary_iter_any,
clippy::iter_cloned_collect,
clippy::iter_next_slice
)]
fn main() {
let sample = [1; 5];
let len = sample.iter().collect::<Vec<_>>().len();
Expand Down
38 changes: 19 additions & 19 deletions tests/ui/needless_collect.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:9:29
--> tests/ui/needless_collect.rs:14:29
|
LL | let len = sample.iter().collect::<Vec<_>>().len();
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()`
Expand All @@ -8,109 +8,109 @@ LL | let len = sample.iter().collect::<Vec<_>>().len();
= help: to override `-D warnings` add `#[allow(clippy::needless_collect)]`

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:10:22
--> tests/ui/needless_collect.rs:15:22
|
LL | if sample.iter().collect::<Vec<_>>().is_empty() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:13:28
--> tests/ui/needless_collect.rs:18:28
|
LL | sample.iter().cloned().collect::<Vec<_>>().contains(&1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == 1)`

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:18:35
--> tests/ui/needless_collect.rs:23:35
|
LL | sample.iter().map(|x| (x, x)).collect::<HashMap<_, _>>().is_empty();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:19:35
--> tests/ui/needless_collect.rs:24:35
|
LL | sample.iter().map(|x| (x, x)).collect::<BTreeMap<_, _>>().is_empty();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:26:19
--> tests/ui/needless_collect.rs:31:19
|
LL | sample.iter().collect::<LinkedList<_>>().len();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()`

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:27:19
--> tests/ui/needless_collect.rs:32:19
|
LL | sample.iter().collect::<LinkedList<_>>().is_empty();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:28:28
--> tests/ui/needless_collect.rs:33:28
|
LL | sample.iter().cloned().collect::<LinkedList<_>>().contains(&1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == 1)`

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:29:19
--> tests/ui/needless_collect.rs:34:19
|
LL | sample.iter().collect::<LinkedList<_>>().contains(&&1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == &1)`

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:32:19
--> tests/ui/needless_collect.rs:37:19
|
LL | sample.iter().collect::<BinaryHeap<_>>().len();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()`

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:33:19
--> tests/ui/needless_collect.rs:38:19
|
LL | sample.iter().collect::<BinaryHeap<_>>().is_empty();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:38:27
--> tests/ui/needless_collect.rs:43:27
|
LL | let _ = sample.iter().collect::<HashSet<_>>().is_empty();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:39:27
--> tests/ui/needless_collect.rs:44:27
|
LL | let _ = sample.iter().collect::<HashSet<_>>().contains(&&0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == &0)`

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:61:27
--> tests/ui/needless_collect.rs:66:27
|
LL | let _ = sample.iter().collect::<VecWrapper<_>>().is_empty();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `next().is_none()`

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:62:27
--> tests/ui/needless_collect.rs:67:27
|
LL | let _ = sample.iter().collect::<VecWrapper<_>>().contains(&&0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `any(|x| x == &0)`

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:66:40
--> tests/ui/needless_collect.rs:71:40
|
LL | Vec::<u8>::new().extend((0..10).collect::<Vec<_>>());
| ^^^^^^^^^^^^^^^^^^^^ help: remove this call

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:67:20
--> tests/ui/needless_collect.rs:72:20
|
LL | foo((0..10).collect::<Vec<_>>());
| ^^^^^^^^^^^^^^^^^^^^ help: remove this call

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:68:49
--> tests/ui/needless_collect.rs:73:49
|
LL | bar((0..10).collect::<Vec<_>>(), (0..10).collect::<Vec<_>>());
| ^^^^^^^^^^^^^^^^^^^^ help: remove this call

error: avoid using `collect()` when not needed
--> tests/ui/needless_collect.rs:69:37
--> tests/ui/needless_collect.rs:74:37
|
LL | baz((0..10), (), ('a'..='z').collect::<Vec<_>>())
| ^^^^^^^^^^^^^^^^^^^^ help: remove this call
Expand Down
17 changes: 0 additions & 17 deletions tests/ui/slice_iter_any.stderr
View file

This file was deleted.

0 comments on commit fe5f2c4

Please sign in to comment.