From 8eb9d35aa25033e5fe37e8d91a6390da86811f08 Mon Sep 17 00:00:00 2001 From: lapla-cogito Date: Thu, 12 Dec 2024 09:58:06 +0900 Subject: [PATCH] new lint to use contains() instead of iter().any() for u8 and i8 slices --- CHANGELOG.md | 1 + clippy_lints/src/contains_for_slice.rs | 65 ++++++++++++++++++++++++++ clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + tests/ui/contains_for_slice.rs | 32 +++++++++++++ tests/ui/contains_for_slice.stderr | 17 +++++++ 6 files changed, 118 insertions(+) create mode 100644 clippy_lints/src/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/contains_for_slice.rs b/clippy_lints/src/contains_for_slice.rs new file mode 100644 index 000000000000..5646d9417d09 --- /dev/null +++ b/clippy_lints/src/contains_for_slice.rs @@ -0,0 +1,65 @@ +use crate::methods::method_call; +use clippy_utils::diagnostics::span_lint; +use rustc_hir::Expr; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{self}; +use rustc_session::declare_lint_pass; + +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" +} + +declare_lint_pass!(ContainsForSlice => [CONTAINS_FOR_SLICE]); + +impl LateLintPass<'_> for ContainsForSlice { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + if !expr.span.from_expansion() + // any() + && let Some((name, recv, _, _, _)) = method_call(expr) + && name == "any" + // 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/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 304622afe530..c4f42896803e 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -107,6 +107,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::collapsible_if::COLLAPSIBLE_IF_INFO, crate::collection_is_never_read::COLLECTION_IS_NEVER_READ_INFO, crate::comparison_chain::COMPARISON_CHAIN_INFO, + crate::contains_for_slice::CONTAINS_FOR_SLICE_INFO, crate::copies::BRANCHES_SHARING_CODE_INFO, crate::copies::IFS_SAME_COND_INFO, crate::copies::IF_SAME_THEN_ELSE_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 017e6e2a1423..6c39b77aa31e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -100,6 +100,7 @@ mod cognitive_complexity; mod collapsible_if; mod collection_is_never_read; mod comparison_chain; +mod contains_for_slice; mod copies; mod copy_iterator; mod crate_in_macro_def; @@ -967,5 +968,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp)); store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound)); store.register_late_pass(move |_| Box::new(arbitrary_source_item_ordering::ArbitrarySourceItemOrdering::new(conf))); + store.register_late_pass(|_| Box::new(contains_for_slice::ContainsForSlice)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/tests/ui/contains_for_slice.rs b/tests/ui/contains_for_slice.rs new file mode 100644 index 000000000000..cb7ecfc75ede --- /dev/null +++ b/tests/ui/contains_for_slice.rs @@ -0,0 +1,32 @@ +#![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 +} + +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..38a74a76cd85 --- /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:25:5 + | +LL | values.iter().any(|&v| v == 10) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors +