add manual_contains lint

Author: lapla-cogito

CHANGELOG.md

@@ -5742,6 +5742,7 @@ Released 2018-09-13
 [`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits
 [`manual_c_str_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals
 [`manual_clamp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp
+[`manual_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_contains
 [`manual_div_ceil`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_div_ceil
 [`manual_filter`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter
 [`manual_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter_map

clippy_lints/src/declared_lints.rs

@@ -417,6 +417,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::methods::ITER_WITH_DRAIN_INFO,
     crate::methods::JOIN_ABSOLUTE_PATHS_INFO,
     crate::methods::MANUAL_C_STR_LITERALS_INFO,
+    crate::methods::MANUAL_CONTAINS_INFO,
     crate::methods::MANUAL_FILTER_MAP_INFO,
     crate::methods::MANUAL_FIND_MAP_INFO,
     crate::methods::MANUAL_INSPECT_INFO,

clippy_lints/src/methods/manual_contains.rs

@@ -0,0 +1,117 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::peel_hir_pat_refs;
+use clippy_utils::source::snippet_with_applicability;
+use rustc_ast::UnOp;
+use rustc_errors::Applicability;
+use rustc_hir::def::Res;
+use rustc_hir::{BinOpKind, Body, Expr, ExprKind, HirId, QPath};
+use rustc_lint::LateContext;
+use rustc_middle::ty::{self};
+use rustc_span::Span;
+use rustc_span::source_map::Spanned;
+
+use super::MANUAL_CONTAINS;
+
+pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, closure_arg: &Expr<'_>) {
+    let mut app = Applicability::MachineApplicable;
+
+    if !expr.span.from_expansion()
+        // check if `iter().any()` can be replaced with `contains()`
+        && let ExprKind::Closure(closure) = closure_arg.kind
+        && let Body{params: [param],value} = cx.tcx.hir().body(closure.body)
+        && let ExprKind::Binary(op, lhs, rhs) = value.kind
+        && let (peeled_ref_pat, _)=peel_hir_pat_refs(param.pat)
+        && let Some((snip,snip_expr))=can_replace_with_contains(cx, op, lhs, rhs, peeled_ref_pat.hir_id, &mut app)
+    {
+        let ref_type = cx.typeck_results().expr_ty_adjusted(recv);
+
+        match ref_type.kind() {
+            ty::Ref(_, inner_type, _) if inner_type.is_slice() => {
+                if let ty::Slice(slice_type) = inner_type.kind()
+                    && *slice_type == cx.typeck_results().expr_ty(snip_expr)
+                {
+                    span_lint_and_sugg(
+                        cx,
+                        MANUAL_CONTAINS,
+                        expr.span,
+                        "using `contains()` instead of `iter().any()` is more efficient",
+                        "try",
+                        format!(
+                            "{}.contains({})",
+                            snippet_with_applicability(cx, recv.span, "..", &mut app),
+                            snip
+                        ),
+                        app,
+                    );
+                }
+            },
+            _ => {},
+        }
+    }
+}
+
+enum EligibleArg {
+    IsClosureArg,
+    ContainsArg(String),
+}
+
+fn try_get_eligible_arg<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'tcx>,
+    closure_arg_id: HirId,
+    applicability: &mut Applicability,
+) -> Option<(EligibleArg, &'tcx Expr<'tcx>)> {
+    let mut get_snippet = |span: Span, needs_borrow: bool| {
+        let snippet = snippet_with_applicability(cx, span, "..", applicability).to_string();
+        if needs_borrow {
+            EligibleArg::ContainsArg(format!("&{snippet}"))
+        } else {
+            EligibleArg::ContainsArg(snippet)
+        }
+    };
+
+    match expr.kind {
+        ExprKind::Path(QPath::Resolved(_, path)) => {
+            if path.res == Res::Local(closure_arg_id) {
+                Some((EligibleArg::IsClosureArg, expr))
+            } else {
+                Some((get_snippet(expr.span, true), expr))
+            }
+        },
+        ExprKind::Unary(UnOp::Deref, inner) => {
+            if let ExprKind::Path(QPath::Resolved(_, path)) = inner.kind {
+                if path.res == Res::Local(closure_arg_id) {
+                    Some((EligibleArg::IsClosureArg, expr))
+                } else {
+                    Some((get_snippet(inner.span, false), expr))
+                }
+            } else {
+                None
+            }
+        },
+        _ => Some((get_snippet(expr.span, true), expr)),
+    }
+}
+
+fn can_replace_with_contains<'tcx>(
+    cx: &LateContext<'tcx>,
+    bin_op: Spanned<BinOpKind>,
+    left_expr: &'tcx Expr<'tcx>,
+    right_expr: &'tcx Expr<'tcx>,
+    closure_arg_id: HirId,
+    applicability: &mut Applicability,
+) -> Option<(String, &'tcx Expr<'tcx>)> {
+    if bin_op.node != BinOpKind::Eq {
+        return None;
+    }
+
+    let left_candidate = try_get_eligible_arg(cx, left_expr, closure_arg_id, applicability)?;
+    let right_candidate = try_get_eligible_arg(cx, right_expr, closure_arg_id, applicability)?;
+    match (left_candidate, right_candidate) {
+        ((EligibleArg::IsClosureArg, _), (EligibleArg::ContainsArg(snip), candidate_expr))
+        | ((EligibleArg::ContainsArg(snip), candidate_expr), (EligibleArg::IsClosureArg, _)) => {
+            Some((snip, candidate_expr))
+        },
+        _ => None,
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -54,6 +54,7 @@ mod iter_with_drain;
 mod iterator_step_by_zero;
 mod join_absolute_paths;
 mod manual_c_str_literals;
+mod manual_contains;
 mod manual_inspect;
 mod manual_is_variant_and;
 mod manual_next_back;
@@ -4406,6 +4407,31 @@ declare_clippy_lint! {
     "using `Option::and_then` or `Result::and_then` to chain a computation that returns an `Option` or a `Result`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of `iter().any()` on slices when it can be replaced with `contains()` and suggests doing so.
+    ///
+    /// ### Why is this bad?
+    /// `contains()` is more concise and idiomatic, sometimes more fast.
+    ///
+    /// ### 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.86.0"]
+    pub MANUAL_CONTAINS,
+    perf,
+    "unnecessary `iter().any()` on slices that can be replaced with `contains()`"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -4575,6 +4601,7 @@ impl_lint_pass!(Methods => [
     MANUAL_REPEAT_N,
     SLICED_STRING_AS_BYTES,
     RETURN_AND_THEN,
+    MANUAL_CONTAINS,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4832,6 +4859,9 @@ impl Methods {
                         Some(("map", _, [map_arg], _, map_call_span)) => {
                             map_all_any_identity::check(cx, expr, recv, map_call_span, map_arg, call_span, arg, "any");
                         },
+                        Some(("iter", iter_recv, ..)) => {
+                            manual_contains::check(cx, expr, iter_recv, arg);
+                        },
                         _ => {},
                     }
                 },

tests/ui/manual_contains.fixed

@@ -0,0 +1,115 @@
+#![warn(clippy::manual_contains)]
+#![allow(clippy::eq_op, clippy::useless_vec)]
+
+fn should_lint_numeric() {
+    let vec: Vec<u8> = vec![1, 2, 3, 4, 5, 6];
+    let values = &vec[..];
+    let _ = values.contains(&4);
+    //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient
+
+    let vec: Vec<u32> = vec![1, 2, 3, 4, 5, 6];
+    let values = &vec[..];
+    let _ = values.contains(&4);
+    //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient
+
+    let values: [u8; 6] = [3, 14, 15, 92, 6, 5];
+    let _ = values.contains(&10);
+    //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient
+
+    let num = 14;
+    let values: [u8; 6] = [3, 14, 15, 92, 6, 5];
+    let _ = values.contains(&num);
+    //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient
+
+    let num = 14;
+    let values: [u8; 6] = [3, 14, 15, 92, 6, 5];
+    let _ = values.contains(&num);
+    //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient
+
+    let values: [u8; 6] = [3, 14, 15, 92, 6, 5];
+    let _ = values.contains(&4);
+    //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient
+
+    let vec: Vec<u8> = vec![1, 2, 3, 4, 5, 6];
+    let values = &vec[..];
+    let _ = values.contains(&4);
+    //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient
+
+    let vec: Vec<u8> = vec![1, 2, 3, 4, 5, 6];
+    let values = &vec[..];
+    let a = &4;
+    let _ = values.contains(a);
+    //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient
+
+    let vec = vec!["1", "2", "3", "4", "5", "6"];
+    let values = &vec[..];
+    let _ = values.contains(&"4");
+    //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient
+}
+
+fn should_not_lint() {
+    let values: [u8; 6] = [3, 14, 15, 92, 6, 5];
+    let _ = values.iter().any(|&v| v > 10);
+
+    let vec: Vec<u32> = vec![1, 2, 3, 4, 5, 6];
+    let values = &vec[..];
+    let _ = values.iter().any(|&v| v % 2 == 0);
+    let _ = values.iter().any(|&v| v * 2 == 6);
+    let _ = values.iter().any(|&v| v == v);
+    let _ = values.iter().any(|&v| 4 == 4);
+    let _ = values.contains(&4);
+
+    let a = 1;
+    let b = 2;
+    let _ = values.iter().any(|&v| a == b);
+    let _ = values.iter().any(|&v| a == 4);
+
+    let vec: Vec<String> = vec!["1", "2", "3", "4", "5", "6"]
+        .iter()
+        .map(|&x| x.to_string())
+        .collect();
+    let values = &vec[..];
+    let _ = values.iter().any(|v| v == "4");
+}
+
+fn foo(values: &[u8]) -> bool {
+    values.contains(&10)
+    //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient
+}
+
+fn bar(values: [u8; 3]) -> bool {
+    values.contains(&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<u8> = vec![1, 2, 3, 4, 5, 6];
+    let values = &vec[..];
+    let _ = values.contains(&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<u32> = vec![1, 2, 3, 4, 5, 6];
+    let values = &vec[..];
+    let _ = values.contains(&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<u8> = vec![1, 2, 3, 4, 5, 6];
+    let values = &vec[..];
+    let _ = values.contains(&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<u32> = vec![1, 2, 3, 4, 5, 6];
+    let values = &vec[..];
+    let _ = values.contains(&4);
+    //~^ ERROR: using `contains()` instead of `iter().any()` is more efficient
+}