From b3837dc522e18486b9328e65b6d1d4e680c2fb0f Mon Sep 17 00:00:00 2001 From: lapla-cogito Date: Sat, 14 Dec 2024 10:46:53 +0900 Subject: [PATCH] add lint for checking unnecessary iter().any() --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/methods/iter_any.rs | 50 +++++++++++++++++++++ clippy_lints/src/methods/mod.rs | 28 +++++++++++- clippy_lints/src/methods/slice_iter_any.rs | 42 ----------------- tests/ui/{slice_iter_any.rs => iter_any.rs} | 4 +- tests/ui/iter_any.stderr | 32 +++++++++++++ tests/ui/needless_collect.fixed | 7 ++- tests/ui/needless_collect.rs | 7 ++- tests/ui/needless_collect.stderr | 38 ++++++++-------- tests/ui/slice_iter_any.stderr | 17 ------- 11 files changed, 143 insertions(+), 84 deletions(-) create mode 100644 clippy_lints/src/methods/iter_any.rs delete mode 100644 clippy_lints/src/methods/slice_iter_any.rs rename tests/ui/{slice_iter_any.rs => iter_any.rs} (87%) create mode 100644 tests/ui/iter_any.stderr delete mode 100644 tests/ui/slice_iter_any.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index e271a655859e..a2d9e0d3c235 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6116,6 +6116,7 @@ Released 2018-09-13 [`unnecessary_first_then_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_first_then_check [`unnecessary_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fold [`unnecessary_get_then_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_get_then_check +[`unnecessary_iter_any`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_iter_any [`unnecessary_join`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_join [`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations [`unnecessary_literal_bound`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_literal_bound diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 6789d9e75006..309249d5dace 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -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, diff --git a/clippy_lints/src/methods/iter_any.rs b/clippy_lints/src/methods/iter_any.rs new file mode 100644 index 000000000000..8626958b979d --- /dev/null +++ b/clippy_lints/src/methods/iter_any.rs @@ -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", + ), + _ => {}, + } + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 70c118b0b05e..2b096f1bf6b9 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -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; @@ -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; @@ -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, @@ -4476,6 +4499,7 @@ impl_lint_pass!(Methods => [ MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES, UNNECESSARY_MAP_OR, SLICE_ITER_ANY, + UNNECESSARY_ITER_ANY, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -4710,7 +4734,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, diff --git a/clippy_lints/src/methods/slice_iter_any.rs b/clippy_lints/src/methods/slice_iter_any.rs deleted file mode 100644 index 849f760e4c77..000000000000 --- a/clippy_lints/src/methods/slice_iter_any.rs +++ /dev/null @@ -1,42 +0,0 @@ -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, 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" - { - // check if the receiver is a u8/i8 slice - let ref_type = cx.typeck_results().expr_ty(recv); - - match ref_type.kind() { - ty::Ref(_, inner_type, _) - if inner_type.is_slice() - && 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", - ); - }, - _ => {}, - } - } -} diff --git a/tests/ui/slice_iter_any.rs b/tests/ui/iter_any.rs similarity index 87% rename from tests/ui/slice_iter_any.rs rename to tests/ui/iter_any.rs index 1489362d75f5..38edf8c61189 100644 --- a/tests/ui/slice_iter_any.rs +++ b/tests/ui/iter_any.rs @@ -18,7 +18,7 @@ fn main() { let values: [u8; 6] = [3, 14, 15, 92, 6, 5]; let _ = values.iter().any(|&v| v == 10); - // no error, because it's an array + //~^ ERROR: using `contains()` instead of `iter().any()` is more readable let values: [u8; 6] = [3, 14, 15, 92, 6, 5]; let _ = values.iter().any(|&v| v > 10); @@ -32,5 +32,5 @@ fn foo(values: &[u8]) -> bool { fn bar(values: [u8; 3]) -> bool { values.iter().any(|&v| v == 10) - // no error, because it's an array + //~^ ERROR: using `contains()` instead of `iter().any()` is more readable } diff --git a/tests/ui/iter_any.stderr b/tests/ui/iter_any.stderr new file mode 100644 index 000000000000..71800b5b763c --- /dev/null +++ b/tests/ui/iter_any.stderr @@ -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 + diff --git a/tests/ui/needless_collect.fixed b/tests/ui/needless_collect.fixed index bd83581bdd97..af08669da6a4 100644 --- a/tests/ui/needless_collect.fixed +++ b/tests/ui/needless_collect.fixed @@ -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(); diff --git a/tests/ui/needless_collect.rs b/tests/ui/needless_collect.rs index 6a81a767bbb6..d357f4d4d43f 100644 --- a/tests/ui/needless_collect.rs +++ b/tests/ui/needless_collect.rs @@ -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::>().len(); diff --git a/tests/ui/needless_collect.stderr b/tests/ui/needless_collect.stderr index ea317896d368..89a4d913b0a2 100644 --- a/tests/ui/needless_collect.stderr +++ b/tests/ui/needless_collect.stderr @@ -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::>().len(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()` @@ -8,109 +8,109 @@ LL | let len = sample.iter().collect::>().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::>().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::>().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::>().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::>().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::>().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::>().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::>().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::>().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::>().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::>().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::>().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::>().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::>().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::>().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::::new().extend((0..10).collect::>()); | ^^^^^^^^^^^^^^^^^^^^ 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::>()); | ^^^^^^^^^^^^^^^^^^^^ 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::>(), (0..10).collect::>()); | ^^^^^^^^^^^^^^^^^^^^ 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::>()) | ^^^^^^^^^^^^^^^^^^^^ help: remove this call diff --git a/tests/ui/slice_iter_any.stderr b/tests/ui/slice_iter_any.stderr deleted file mode 100644 index 13bdf235d2f3..000000000000 --- a/tests/ui/slice_iter_any.stderr +++ /dev/null @@ -1,17 +0,0 @@ -error: using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient - --> tests/ui/slice_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()` on u8/i8 slices is more efficient - --> tests/ui/slice_iter_any.rs:29:5 - | -LL | values.iter().any(|&v| v == 10) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 2 previous errors -