From c52ebf2c672859b2fe48960b12915eadbb34b010 Mon Sep 17 00:00:00 2001 From: lapla-cogito Date: Thu, 12 Dec 2024 09:58:06 +0900 Subject: [PATCH 1/5] new lint to use contains() instead of iter().any() for u8 and i8 slices --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + .../src/methods/contains_for_slice.rs | 42 +++++++++++++++++++ clippy_lints/src/methods/mod.rs | 28 +++++++++++++ tests/ui/contains_for_slice.rs | 36 ++++++++++++++++ tests/ui/contains_for_slice.stderr | 17 ++++++++ 6 files changed, 125 insertions(+) create mode 100644 clippy_lints/src/methods/contains_for_slice.rs create mode 100644 tests/ui/contains_for_slice.rs create mode 100644 tests/ui/contains_for_slice.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index cc966972939a..e24cd2b300a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5455,6 +5455,7 @@ Released 2018-09-13 [`comparison_to_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_to_empty [`const_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_is_empty [`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime +[`contains_for_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#contains_for_slice [`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator [`crate_in_macro_def`]: https://rust-lang.github.io/rust-clippy/master/index.html#crate_in_macro_def [`create_dir`]: https://rust-lang.github.io/rust-clippy/master/index.html#create_dir diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 304622afe530..8684ee279e8d 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -371,6 +371,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::methods::CLONE_ON_REF_PTR_INFO, crate::methods::COLLAPSIBLE_STR_REPLACE_INFO, crate::methods::CONST_IS_EMPTY_INFO, + crate::methods::CONTAINS_FOR_SLICE_INFO, crate::methods::DRAIN_COLLECT_INFO, crate::methods::ERR_EXPECT_INFO, crate::methods::EXPECT_FUN_CALL_INFO, diff --git a/clippy_lints/src/methods/contains_for_slice.rs b/clippy_lints/src/methods/contains_for_slice.rs new file mode 100644 index 000000000000..714dd2c7dfa4 --- /dev/null +++ b/clippy_lints/src/methods/contains_for_slice.rs @@ -0,0 +1,42 @@ +use clippy_utils::diagnostics::span_lint; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_middle::ty::{self}; + +use super::{CONTAINS_FOR_SLICE, 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, + CONTAINS_FOR_SLICE, + expr.span, + "using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient", + ); + }, + _ => {}, + } + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 810287fa5416..f827a8c640fb 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -14,6 +14,7 @@ mod clone_on_copy; mod clone_on_ref_ptr; mod cloned_instead_of_copied; mod collapsible_str_replace; +mod contains_for_slice; mod drain_collect; mod err_expect; mod expect_fun_call; @@ -4284,6 +4285,31 @@ declare_clippy_lint! { "map of a trivial closure (not dependent on parameter) over a range" } +declare_clippy_lint! { + /// ### What it does + /// Checks for usage of `iter().any()` on slices of `u8` or `i8` and suggests using `contains()` instead. + /// + /// ### Why is this bad? + /// `iter().any()` on slices of `u8` or `i8` is optimized to use `memchr`. + /// + /// ### Example + /// ```no_run + /// fn foo(values: &[u8]) -> bool { + /// values.iter().any(|&v| v == 10) + /// } + /// ``` + /// Use instead: + /// ```no_run + /// fn foo(values: &[u8]) -> bool { + /// values.contains(&10) + /// } + /// ``` + #[clippy::version = "1.85.0"] + pub CONTAINS_FOR_SLICE, + perf, + "using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Msrv, @@ -4449,6 +4475,7 @@ impl_lint_pass!(Methods => [ MAP_ALL_ANY_IDENTITY, MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES, UNNECESSARY_MAP_OR, + CONTAINS_FOR_SLICE, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -4683,6 +4710,7 @@ impl Methods { ("any", [arg]) => { unused_enumerate_index::check(cx, expr, recv, arg); needless_character_iteration::check(cx, expr, recv, arg, false); + contains_for_slice::check(cx, expr); match method_call(recv) { Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check( cx, diff --git a/tests/ui/contains_for_slice.rs b/tests/ui/contains_for_slice.rs new file mode 100644 index 000000000000..ab69395c8a89 --- /dev/null +++ b/tests/ui/contains_for_slice.rs @@ -0,0 +1,36 @@ +#![warn(clippy::contains_for_slice)] + +fn main() { + let vec: Vec = vec![1, 2, 3, 4, 5, 6]; + let values = &vec[..]; + let _ = values.iter().any(|&v| v == 4); + //~^ ERROR: using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient + + let vec: Vec = vec![1, 2, 3, 4, 5, 6]; + let values = &vec[..]; + let _ = values.contains(&4); + // no error, because it uses `contains()` + + let vec: Vec = vec![1, 2, 3, 4, 5, 6]; + let values = &vec[..]; + let _ = values.iter().any(|&v| v == 4); + // no error, because it's not a slice of u8/i8 + + let values: [u8; 6] = [3, 14, 15, 92, 6, 5]; + let _ = values.iter().any(|&v| v == 10); + // no error, because it's an array + + let values: [u8; 6] = [3, 14, 15, 92, 6, 5]; + let _ = values.iter().any(|&v| v > 10); + // no error, because this `any()` is not for equality +} + +fn foo(values: &[u8]) -> bool { + values.iter().any(|&v| v == 10) + //~^ ERROR: using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient +} + +fn bar(values: [u8; 3]) -> bool { + values.iter().any(|&v| v == 10) + // no error, because it's an array +} diff --git a/tests/ui/contains_for_slice.stderr b/tests/ui/contains_for_slice.stderr new file mode 100644 index 000000000000..0f8b3f483084 --- /dev/null +++ b/tests/ui/contains_for_slice.stderr @@ -0,0 +1,17 @@ +error: using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient + --> tests/ui/contains_for_slice.rs:6:13 + | +LL | let _ = values.iter().any(|&v| v == 4); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::contains-for-slice` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::contains_for_slice)]` + +error: using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient + --> tests/ui/contains_for_slice.rs:29:5 + | +LL | values.iter().any(|&v| v == 10) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + From c1d510824caf297dbc5ae5420617ee90ed40d569 Mon Sep 17 00:00:00 2001 From: lapla-cogito Date: Fri, 13 Dec 2024 08:43:08 +0900 Subject: [PATCH 2/5] rename contains_for_slice to slice_iter_any --- CHANGELOG.md | 2 +- clippy_lints/src/declared_lints.rs | 2 +- clippy_lints/src/methods/mod.rs | 8 ++++---- .../methods/{contains_for_slice.rs => slice_iter_any.rs} | 4 ++-- tests/ui/{contains_for_slice.rs => slice_iter_any.rs} | 2 +- .../{contains_for_slice.stderr => slice_iter_any.stderr} | 8 ++++---- 6 files changed, 13 insertions(+), 13 deletions(-) rename clippy_lints/src/methods/{contains_for_slice.rs => slice_iter_any.rs} (94%) rename tests/ui/{contains_for_slice.rs => slice_iter_any.rs} (96%) rename tests/ui/{contains_for_slice.stderr => slice_iter_any.stderr} (63%) diff --git a/CHANGELOG.md b/CHANGELOG.md index e24cd2b300a0..e271a655859e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5455,7 +5455,6 @@ Released 2018-09-13 [`comparison_to_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_to_empty [`const_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_is_empty [`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime -[`contains_for_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#contains_for_slice [`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator [`crate_in_macro_def`]: https://rust-lang.github.io/rust-clippy/master/index.html#crate_in_macro_def [`create_dir`]: https://rust-lang.github.io/rust-clippy/master/index.html#create_dir @@ -6021,6 +6020,7 @@ Released 2018-09-13 [`size_of_in_element_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_in_element_count [`size_of_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_ref [`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next +[`slice_iter_any`]: https://rust-lang.github.io/rust-clippy/master/index.html#slice_iter_any [`slow_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_vector_initialization [`stable_sort_primitive`]: https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive [`std_instead_of_alloc`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_alloc diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 8684ee279e8d..6789d9e75006 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -371,7 +371,6 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::methods::CLONE_ON_REF_PTR_INFO, crate::methods::COLLAPSIBLE_STR_REPLACE_INFO, crate::methods::CONST_IS_EMPTY_INFO, - crate::methods::CONTAINS_FOR_SLICE_INFO, crate::methods::DRAIN_COLLECT_INFO, crate::methods::ERR_EXPECT_INFO, crate::methods::EXPECT_FUN_CALL_INFO, @@ -465,6 +464,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::methods::SHOULD_IMPLEMENT_TRAIT_INFO, crate::methods::SINGLE_CHAR_ADD_STR_INFO, crate::methods::SKIP_WHILE_NEXT_INFO, + crate::methods::SLICE_ITER_ANY_INFO, crate::methods::STABLE_SORT_PRIMITIVE_INFO, crate::methods::STRING_EXTEND_CHARS_INFO, crate::methods::STRING_LIT_CHARS_ANY_INFO, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index f827a8c640fb..de298b5db890 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -14,7 +14,6 @@ mod clone_on_copy; mod clone_on_ref_ptr; mod cloned_instead_of_copied; mod collapsible_str_replace; -mod contains_for_slice; mod drain_collect; mod err_expect; mod expect_fun_call; @@ -101,6 +100,7 @@ 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; @@ -4305,7 +4305,7 @@ declare_clippy_lint! { /// } /// ``` #[clippy::version = "1.85.0"] - pub CONTAINS_FOR_SLICE, + pub SLICE_ITER_ANY, perf, "using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient" } @@ -4475,7 +4475,7 @@ impl_lint_pass!(Methods => [ MAP_ALL_ANY_IDENTITY, MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES, UNNECESSARY_MAP_OR, - CONTAINS_FOR_SLICE, + SLICE_ITER_ANY, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -4710,7 +4710,7 @@ impl Methods { ("any", [arg]) => { unused_enumerate_index::check(cx, expr, recv, arg); needless_character_iteration::check(cx, expr, recv, arg, false); - contains_for_slice::check(cx, expr); + slice_iter_any::check(cx, expr); match method_call(recv) { Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check( cx, diff --git a/clippy_lints/src/methods/contains_for_slice.rs b/clippy_lints/src/methods/slice_iter_any.rs similarity index 94% rename from clippy_lints/src/methods/contains_for_slice.rs rename to clippy_lints/src/methods/slice_iter_any.rs index 714dd2c7dfa4..849f760e4c77 100644 --- a/clippy_lints/src/methods/contains_for_slice.rs +++ b/clippy_lints/src/methods/slice_iter_any.rs @@ -3,7 +3,7 @@ use rustc_hir::{Expr, ExprKind}; use rustc_lint::LateContext; use rustc_middle::ty::{self}; -use super::{CONTAINS_FOR_SLICE, method_call}; +use super::{SLICE_ITER_ANY, method_call}; pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) { if !expr.span.from_expansion() @@ -31,7 +31,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) { { span_lint( cx, - CONTAINS_FOR_SLICE, + SLICE_ITER_ANY, expr.span, "using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient", ); diff --git a/tests/ui/contains_for_slice.rs b/tests/ui/slice_iter_any.rs similarity index 96% rename from tests/ui/contains_for_slice.rs rename to tests/ui/slice_iter_any.rs index ab69395c8a89..1489362d75f5 100644 --- a/tests/ui/contains_for_slice.rs +++ b/tests/ui/slice_iter_any.rs @@ -1,4 +1,4 @@ -#![warn(clippy::contains_for_slice)] +#![warn(clippy::slice_iter_any)] fn main() { let vec: Vec = vec![1, 2, 3, 4, 5, 6]; diff --git a/tests/ui/contains_for_slice.stderr b/tests/ui/slice_iter_any.stderr similarity index 63% rename from tests/ui/contains_for_slice.stderr rename to tests/ui/slice_iter_any.stderr index 0f8b3f483084..13bdf235d2f3 100644 --- a/tests/ui/contains_for_slice.stderr +++ b/tests/ui/slice_iter_any.stderr @@ -1,14 +1,14 @@ error: using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient - --> tests/ui/contains_for_slice.rs:6:13 + --> tests/ui/slice_iter_any.rs:6:13 | LL | let _ = values.iter().any(|&v| v == 4); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: `-D clippy::contains-for-slice` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::contains_for_slice)]` + = 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/contains_for_slice.rs:29:5 + --> tests/ui/slice_iter_any.rs:29:5 | LL | values.iter().any(|&v| v == 10) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From b3a1693eb4bfd5c52ff9ae125722be45b5715afb Mon Sep 17 00:00:00 2001 From: lapla-cogito Date: Sat, 14 Dec 2024 09:50:18 +0900 Subject: [PATCH 3/5] correct description for slice_iter_any --- clippy_lints/src/methods/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index de298b5db890..70c118b0b05e 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -4287,10 +4287,10 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for usage of `iter().any()` on slices of `u8` or `i8` and suggests using `contains()` instead. + /// Checks for usage of `iter().any()` on slices of `u8`/`i8` when it can be replaced with `contains()` and suggests doing so. /// /// ### Why is this bad? - /// `iter().any()` on slices of `u8` or `i8` is optimized to use `memchr`. + /// `contains()` on slices of `u8`/`i8` is faster than `iter().any()` in such cases. /// /// ### Example /// ```no_run @@ -4307,7 +4307,7 @@ declare_clippy_lint! { #[clippy::version = "1.85.0"] pub SLICE_ITER_ANY, perf, - "using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient" + "using `contains()` instead of `iter().any()` on `u8`/`i8` slices is more fast" } pub struct Methods { From 19357caa5a1ed32ec99ee1a797c14da3263d8296 Mon Sep 17 00:00:00 2001 From: lapla-cogito Date: Sat, 14 Dec 2024 10:46:53 +0900 Subject: [PATCH 4/5] add lint for checking unnecessary iter().any() --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/methods/iter_any.rs | 60 +++++++++++++++++++++ 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} | 6 +-- tests/ui/iter_any.stderr | 38 +++++++++++++ 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, 160 insertions(+), 85 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} (81%) 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..703b6dfc6ef6 --- /dev/null +++ b/clippy_lints/src/methods/iter_any.rs @@ -0,0 +1,60 @@ +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", + ); + } else if let ty::Slice(slice_type) = inner_type.kind() + && slice_type.is_numeric() + { + span_lint( + cx, + UNNECESSARY_ITER_ANY, + expr.span, + "using `contains()` instead of `iter().any()` is more readable", + ); + } + }, + // if it's an array that uses `iter().any()` and its closure is an equality check, suggest using + // `contains()` (currently only for numeric arrays because of the difficulty in determining whether + // `contains()` can be replaced by `contains()` for arrays of general types) + 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 81% rename from tests/ui/slice_iter_any.rs rename to tests/ui/iter_any.rs index 1489362d75f5..3fa407b4ec05 100644 --- a/tests/ui/slice_iter_any.rs +++ b/tests/ui/iter_any.rs @@ -14,11 +14,11 @@ fn main() { let vec: Vec = vec![1, 2, 3, 4, 5, 6]; let values = &vec[..]; let _ = values.iter().any(|&v| v == 4); - // no error, because it's not a slice of u8/i8 + //~^ 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); - // 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..17c425811e08 --- /dev/null +++ b/tests/ui/iter_any.stderr @@ -0,0 +1,38 @@ +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:16:13 + | +LL | let _ = values.iter().any(|&v| v == 4); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = 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()` is more readable + --> tests/ui/iter_any.rs:20:13 + | +LL | let _ = values.iter().any(|&v| v == 10); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +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 5 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 - From e964cbc8380547e13ca85cd8563de796fb7b311d Mon Sep 17 00:00:00 2001 From: lapla-cogito Date: Sun, 22 Dec 2024 13:32:55 +0900 Subject: [PATCH 5/5] add slice_iter_any lint --- CHANGELOG.md | 1 - clippy_lints/src/declared_lints.rs | 1 - clippy_lints/src/methods/iter_any.rs | 60 ---------------- clippy_lints/src/methods/mod.rs | 34 ++------- clippy_lints/src/methods/slice_iter_any.rs | 66 +++++++++++++++++ tests/ui/iter_any.rs | 36 ---------- tests/ui/iter_any.stderr | 38 ---------- tests/ui/needless_collect.fixed | 15 ++-- tests/ui/needless_collect.rs | 15 ++-- tests/ui/needless_collect.stderr | 38 +++++----- tests/ui/search_is_some_fixable_none.fixed | 2 +- tests/ui/search_is_some_fixable_none.rs | 2 +- tests/ui/search_is_some_fixable_some.fixed | 2 +- tests/ui/search_is_some_fixable_some.rs | 2 +- tests/ui/slice_iter_any.rs | 82 ++++++++++++++++++++++ tests/ui/slice_iter_any.stderr | 65 +++++++++++++++++ 16 files changed, 257 insertions(+), 202 deletions(-) delete mode 100644 clippy_lints/src/methods/iter_any.rs create mode 100644 clippy_lints/src/methods/slice_iter_any.rs delete mode 100644 tests/ui/iter_any.rs delete mode 100644 tests/ui/iter_any.stderr create mode 100644 tests/ui/slice_iter_any.rs create mode 100644 tests/ui/slice_iter_any.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index a2d9e0d3c235..e271a655859e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6116,7 +6116,6 @@ 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 309249d5dace..6789d9e75006 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -483,7 +483,6 @@ 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 deleted file mode 100644 index 703b6dfc6ef6..000000000000 --- a/clippy_lints/src/methods/iter_any.rs +++ /dev/null @@ -1,60 +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, 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", - ); - } else if let ty::Slice(slice_type) = inner_type.kind() - && slice_type.is_numeric() - { - span_lint( - cx, - UNNECESSARY_ITER_ANY, - expr.span, - "using `contains()` instead of `iter().any()` is more readable", - ); - } - }, - // if it's an array that uses `iter().any()` and its closure is an equality check, suggest using - // `contains()` (currently only for numeric arrays because of the difficulty in determining whether - // `contains()` can be replaced by `contains()` for arrays of general types) - 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 2b096f1bf6b9..392c6fe7576f 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -37,7 +37,6 @@ 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; @@ -101,6 +100,7 @@ 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; @@ -4287,10 +4287,10 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for usage of `iter().any()` on slices of `u8`/`i8` when it can be replaced with `contains()` and suggests doing so. + /// Checks for usage of `iter().any()` on numeric slices when it can be replaced with `contains()` and suggests doing so. /// /// ### Why is this bad? - /// `contains()` on slices of `u8`/`i8` is faster than `iter().any()` in such cases. + /// `contains()` on numeric slices is faster than `iter().any()`. /// /// ### Example /// ```no_run @@ -4307,30 +4307,7 @@ declare_clippy_lint! { #[clippy::version = "1.85.0"] pub SLICE_ITER_ANY, perf, - "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" + "detect use of `iter().any()` on numeric slices and suggest using `contains()`" } pub struct Methods { @@ -4499,7 +4476,6 @@ 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. @@ -4734,7 +4710,7 @@ impl Methods { ("any", [arg]) => { unused_enumerate_index::check(cx, expr, recv, arg); needless_character_iteration::check(cx, expr, recv, arg, false); - iter_any::check(cx, expr); + slice_iter_any::check(cx, expr, &self.msrv); 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 new file mode 100644 index 000000000000..df9ef3bda38f --- /dev/null +++ b/clippy_lints/src/methods/slice_iter_any.rs @@ -0,0 +1,66 @@ +use clippy_utils::diagnostics::span_lint; +use clippy_utils::msrvs::Msrv; +use rustc_hir::{BinOpKind, Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_middle::ty::{self}; +use rustc_session::RustcVersion; +use rustc_span::source_map::Spanned; + +use super::{SLICE_ITER_ANY, method_call}; + +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, msrv: &Msrv) { + if !expr.span.from_expansion() + // any() + && let Some((name, recv, args, _, _)) = method_call(expr) + && name == "any" + // check if `iter().any()` can be replaced with `contains()` + && args.len() == 1 + && let ExprKind::Closure(closure) = args[0].kind + && let body = cx.tcx.hir().body(closure.body) + && let ExprKind::Binary(op, lhs, rhs) = body.value.kind + && can_replace_with_contains(op, lhs, rhs) + // iter() + && let Some((name, recv, _, _, _)) = method_call(recv) + && name == "iter" + { + let ref_type = cx.typeck_results().expr_ty_adjusted(recv); + + match ref_type.kind() { + ty::Ref(_, inner_type, _) if inner_type.is_slice() => { + // check if the receiver is a numeric slice + if let ty::Slice(slice_type) = inner_type.kind() + && ((slice_type.to_string() == "u8" || slice_type.to_string() == "i8") + || (slice_type.is_numeric() + && msrv.meets(RustcVersion { + major: 1, + minor: 84, + patch: 0, + }))) + { + span_lint( + cx, + SLICE_ITER_ANY, + expr.span, + "using `contains()` instead of `iter().any()` is more efficient", + ); + } + }, + _ => {}, + } + } +} + +fn can_replace_with_contains(op: Spanned, lhs: &Expr<'_>, rhs: &Expr<'_>) -> bool { + matches!( + (op.node, &lhs.kind, &rhs.kind), + ( + BinOpKind::Eq, + ExprKind::Path(_) | ExprKind::Unary(_, _), + ExprKind::Lit(_) | ExprKind::Path(_) + ) | ( + BinOpKind::Eq, + ExprKind::Lit(_), + ExprKind::Path(_) | ExprKind::Unary(_, _) + ) + ) +} diff --git a/tests/ui/iter_any.rs b/tests/ui/iter_any.rs deleted file mode 100644 index 3fa407b4ec05..000000000000 --- a/tests/ui/iter_any.rs +++ /dev/null @@ -1,36 +0,0 @@ -#![warn(clippy::slice_iter_any)] - -fn main() { - let vec: Vec = vec![1, 2, 3, 4, 5, 6]; - let values = &vec[..]; - let _ = values.iter().any(|&v| v == 4); - //~^ ERROR: using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient - - let vec: Vec = vec![1, 2, 3, 4, 5, 6]; - let values = &vec[..]; - let _ = values.contains(&4); - // no error, because it uses `contains()` - - let vec: Vec = vec![1, 2, 3, 4, 5, 6]; - let values = &vec[..]; - let _ = values.iter().any(|&v| v == 4); - //~^ 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); - //~^ 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); - // no error, because this `any()` is not for equality -} - -fn foo(values: &[u8]) -> bool { - values.iter().any(|&v| v == 10) - //~^ ERROR: using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient -} - -fn bar(values: [u8; 3]) -> bool { - values.iter().any(|&v| v == 10) - //~^ ERROR: using `contains()` instead of `iter().any()` is more readable -} diff --git a/tests/ui/iter_any.stderr b/tests/ui/iter_any.stderr deleted file mode 100644 index 17c425811e08..000000000000 --- a/tests/ui/iter_any.stderr +++ /dev/null @@ -1,38 +0,0 @@ -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:16:13 - | -LL | let _ = values.iter().any(|&v| v == 4); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = 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()` is more readable - --> tests/ui/iter_any.rs:20:13 - | -LL | let _ = values.iter().any(|&v| v == 10); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -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 5 previous errors - diff --git a/tests/ui/needless_collect.fixed b/tests/ui/needless_collect.fixed index af08669da6a4..636dc492a04f 100644 --- a/tests/ui/needless_collect.fixed +++ b/tests/ui/needless_collect.fixed @@ -1,14 +1,15 @@ -#![allow(unused, clippy::needless_if, clippy::suspicious_map, clippy::iter_count)] +#![allow( + unused, + clippy::needless_if, + clippy::suspicious_map, + clippy::iter_count, + clippy::slice_iter_any +)] use std::collections::{BTreeMap, BTreeSet, BinaryHeap, HashMap, HashSet, LinkedList}; #[warn(clippy::needless_collect)] -#[allow( - unused_variables, - clippy::unnecessary_iter_any, - clippy::iter_cloned_collect, - clippy::iter_next_slice -)] +#[allow(unused_variables, 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 d357f4d4d43f..39e241070000 100644 --- a/tests/ui/needless_collect.rs +++ b/tests/ui/needless_collect.rs @@ -1,14 +1,15 @@ -#![allow(unused, clippy::needless_if, clippy::suspicious_map, clippy::iter_count)] +#![allow( + unused, + clippy::needless_if, + clippy::suspicious_map, + clippy::iter_count, + clippy::slice_iter_any +)] use std::collections::{BTreeMap, BTreeSet, BinaryHeap, HashMap, HashSet, LinkedList}; #[warn(clippy::needless_collect)] -#[allow( - unused_variables, - clippy::unnecessary_iter_any, - clippy::iter_cloned_collect, - clippy::iter_next_slice -)] +#[allow(unused_variables, 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 89a4d913b0a2..c22ff1a13d3d 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:14:29 + --> tests/ui/needless_collect.rs:15: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:15:22 + --> tests/ui/needless_collect.rs:16: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:18:28 + --> tests/ui/needless_collect.rs:19: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:23: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:24:35 + --> tests/ui/needless_collect.rs:25: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:31:19 + --> tests/ui/needless_collect.rs:32:19 | LL | sample.iter().collect::>().len(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()` error: avoid using `collect()` when not needed - --> tests/ui/needless_collect.rs:32:19 + --> tests/ui/needless_collect.rs:33: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:33:28 + --> tests/ui/needless_collect.rs:34: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:34:19 + --> tests/ui/needless_collect.rs:35: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:37:19 + --> tests/ui/needless_collect.rs:38:19 | LL | sample.iter().collect::>().len(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `count()` error: avoid using `collect()` when not needed - --> tests/ui/needless_collect.rs:38:19 + --> tests/ui/needless_collect.rs:39: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:43:27 + --> tests/ui/needless_collect.rs:44: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:44:27 + --> tests/ui/needless_collect.rs:45: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:27 + --> tests/ui/needless_collect.rs:67: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:67:27 + --> tests/ui/needless_collect.rs:68: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:71:40 + --> tests/ui/needless_collect.rs:72:40 | LL | Vec::::new().extend((0..10).collect::>()); | ^^^^^^^^^^^^^^^^^^^^ help: remove this call error: avoid using `collect()` when not needed - --> tests/ui/needless_collect.rs:72:20 + --> tests/ui/needless_collect.rs:73:20 | LL | foo((0..10).collect::>()); | ^^^^^^^^^^^^^^^^^^^^ help: remove this call error: avoid using `collect()` when not needed - --> tests/ui/needless_collect.rs:73:49 + --> tests/ui/needless_collect.rs:74: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:74:37 + --> tests/ui/needless_collect.rs:75:37 | LL | baz((0..10), (), ('a'..='z').collect::>()) | ^^^^^^^^^^^^^^^^^^^^ help: remove this call diff --git a/tests/ui/search_is_some_fixable_none.fixed b/tests/ui/search_is_some_fixable_none.fixed index 86a937b4dae1..31aa072dd479 100644 --- a/tests/ui/search_is_some_fixable_none.fixed +++ b/tests/ui/search_is_some_fixable_none.fixed @@ -1,4 +1,4 @@ -#![allow(dead_code, clippy::explicit_auto_deref, clippy::useless_vec)] +#![allow(dead_code, clippy::explicit_auto_deref, clippy::useless_vec, clippy::slice_iter_any)] #![warn(clippy::search_is_some)] fn main() { diff --git a/tests/ui/search_is_some_fixable_none.rs b/tests/ui/search_is_some_fixable_none.rs index c0103a015097..776e62dcab78 100644 --- a/tests/ui/search_is_some_fixable_none.rs +++ b/tests/ui/search_is_some_fixable_none.rs @@ -1,4 +1,4 @@ -#![allow(dead_code, clippy::explicit_auto_deref, clippy::useless_vec)] +#![allow(dead_code, clippy::explicit_auto_deref, clippy::useless_vec, clippy::slice_iter_any)] #![warn(clippy::search_is_some)] fn main() { diff --git a/tests/ui/search_is_some_fixable_some.fixed b/tests/ui/search_is_some_fixable_some.fixed index ae3cbc3c4da2..484e2cc7f044 100644 --- a/tests/ui/search_is_some_fixable_some.fixed +++ b/tests/ui/search_is_some_fixable_some.fixed @@ -1,4 +1,4 @@ -#![allow(dead_code, clippy::explicit_auto_deref, clippy::useless_vec)] +#![allow(dead_code, clippy::explicit_auto_deref, clippy::useless_vec, clippy::slice_iter_any)] #![warn(clippy::search_is_some)] fn main() { diff --git a/tests/ui/search_is_some_fixable_some.rs b/tests/ui/search_is_some_fixable_some.rs index 19a44803fd54..b2dafbfd5ec7 100644 --- a/tests/ui/search_is_some_fixable_some.rs +++ b/tests/ui/search_is_some_fixable_some.rs @@ -1,4 +1,4 @@ -#![allow(dead_code, clippy::explicit_auto_deref, clippy::useless_vec)] +#![allow(dead_code, clippy::explicit_auto_deref, clippy::useless_vec, clippy::slice_iter_any)] #![warn(clippy::search_is_some)] fn main() { diff --git a/tests/ui/slice_iter_any.rs b/tests/ui/slice_iter_any.rs new file mode 100644 index 000000000000..c69c451f49bb --- /dev/null +++ b/tests/ui/slice_iter_any.rs @@ -0,0 +1,82 @@ +#![warn(clippy::slice_iter_any)] + +fn main() { + let vec: Vec = vec![1, 2, 3, 4, 5, 6]; + let values = &vec[..]; + let _ = values.iter().any(|&v| v == 4); + //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient + + let vec: Vec = vec![1, 2, 3, 4, 5, 6]; + let values = &vec[..]; + let _ = values.contains(&4); + // no error, because it uses `contains()` + + let vec: Vec = vec![1, 2, 3, 4, 5, 6]; + let values = &vec[..]; + let _ = values.iter().any(|&v| v == 4); + //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient + + let values: [u8; 6] = [3, 14, 15, 92, 6, 5]; + let _ = values.iter().any(|&v| v == 10); + //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient + + let values: [u8; 6] = [3, 14, 15, 92, 6, 5]; + let _ = values.iter().any(|&v| v > 10); + // no error, because this `any()` is not for equality + + let vec: Vec = vec![1, 2, 3, 4, 5, 6]; + let values = &vec[..]; + let _ = values.iter().any(|&v| v % 2 == 0); + // no error, because this `iter().any()` can't be replaced with `contains()` simply + + let num = 14; + let values: [u8; 6] = [3, 14, 15, 92, 6, 5]; + let _ = values.iter().any(|&v| v == num); + //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient + + let values: [u8; 6] = [3, 14, 15, 92, 6, 5]; + let _ = values.iter().any(|v| *v == 4); + //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient +} + +fn foo(values: &[u8]) -> bool { + values.iter().any(|&v| v == 10) + //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient +} + +fn bar(values: [u8; 3]) -> bool { + values.iter().any(|&v| v == 10) + //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient +} + +#[clippy::msrv = "1.83"] +fn u8_slice_and_msrv_is_1_83_0() { + let vec: Vec = vec![1, 2, 3, 4, 5, 6]; + let values = &vec[..]; + let _ = values.iter().any(|&v| v == 4); + //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient +} + +#[clippy::msrv = "1.83"] +fn u32_slice_and_msrv_is_1_83_0() { + let vec: Vec = vec![1, 2, 3, 4, 5, 6]; + let values = &vec[..]; + let _ = values.iter().any(|&v| v == 4); + // Shouldn't trigger the performance lint because the MSRV is 1.83.0 and this is a u32 slice +} + +#[clippy::msrv = "1.84"] +fn u8_slice_and_msrv_is_1_84_0() { + let vec: Vec = vec![1, 2, 3, 4, 5, 6]; + let values = &vec[..]; + let _ = values.iter().any(|&v| v == 4); + //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient +} + +#[clippy::msrv = "1.84"] +fn u32_slice_and_msrv_is_1_84_0() { + let vec: Vec = vec![1, 2, 3, 4, 5, 6]; + let values = &vec[..]; + let _ = values.iter().any(|&v| v == 4); + //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient +} diff --git a/tests/ui/slice_iter_any.stderr b/tests/ui/slice_iter_any.stderr new file mode 100644 index 000000000000..710b801f8996 --- /dev/null +++ b/tests/ui/slice_iter_any.stderr @@ -0,0 +1,65 @@ +error: using `contains()` instead of `iter().any()` 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()` is more efficient + --> tests/ui/slice_iter_any.rs:16:13 + | +LL | let _ = values.iter().any(|&v| v == 4); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: using `contains()` instead of `iter().any()` is more efficient + --> tests/ui/slice_iter_any.rs:20:13 + | +LL | let _ = values.iter().any(|&v| v == 10); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: using `contains()` instead of `iter().any()` is more efficient + --> tests/ui/slice_iter_any.rs:34:13 + | +LL | let _ = values.iter().any(|&v| v == num); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: using `contains()` instead of `iter().any()` is more efficient + --> tests/ui/slice_iter_any.rs:38:13 + | +LL | let _ = values.iter().any(|v| *v == 4); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: using `contains()` instead of `iter().any()` is more efficient + --> tests/ui/slice_iter_any.rs:43:5 + | +LL | values.iter().any(|&v| v == 10) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: using `contains()` instead of `iter().any()` is more efficient + --> tests/ui/slice_iter_any.rs:48:5 + | +LL | values.iter().any(|&v| v == 10) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: using `contains()` instead of `iter().any()` is more efficient + --> tests/ui/slice_iter_any.rs:56:13 + | +LL | let _ = values.iter().any(|&v| v == 4); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: using `contains()` instead of `iter().any()` is more efficient + --> tests/ui/slice_iter_any.rs:72:13 + | +LL | let _ = values.iter().any(|&v| v == 4); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: using `contains()` instead of `iter().any()` is more efficient + --> tests/ui/slice_iter_any.rs:80:13 + | +LL | let _ = values.iter().any(|&v| v == 4); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 10 previous errors +