diff --git a/CHANGELOG.md b/CHANGELOG.md index cc966972939a..e271a655859e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6020,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 304622afe530..6789d9e75006 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -464,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 810287fa5416..392c6fe7576f 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -100,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; @@ -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 numeric slices when it can be replaced with `contains()` and suggests doing so. + /// + /// ### Why is this bad? + /// `contains()` on numeric slices is faster than `iter().any()`. + /// + /// ### 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 SLICE_ITER_ANY, + perf, + "detect use of `iter().any()` on numeric slices and suggest using `contains()`" +} + 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, + SLICE_ITER_ANY, ]); /// 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); + 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/needless_collect.fixed b/tests/ui/needless_collect.fixed index bd83581bdd97..636dc492a04f 100644 --- a/tests/ui/needless_collect.fixed +++ b/tests/ui/needless_collect.fixed @@ -1,4 +1,10 @@ -#![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}; diff --git a/tests/ui/needless_collect.rs b/tests/ui/needless_collect.rs index 6a81a767bbb6..39e241070000 100644 --- a/tests/ui/needless_collect.rs +++ b/tests/ui/needless_collect.rs @@ -1,4 +1,10 @@ -#![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}; diff --git a/tests/ui/needless_collect.stderr b/tests/ui/needless_collect.stderr index ea317896d368..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:9: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:10: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:13: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:18: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:19: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:26: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:27: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:28: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:29: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:32: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:33: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:38: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:39: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:61: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:62: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:66: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:67: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:68: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:69: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 +