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

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

clippy_lints/src/methods/manual_contains.rs

@@ -0,0 +1,113 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::eager_or_lazy::switch_to_eager_eval;
+use clippy_utils::peel_hir_pat_refs;
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::sugg::Sugg;
+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::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)
+        && let ty::Ref(_, inner_type, _) = ref_type.kind()
+        && 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 = |expr: &Expr<'_>, needs_borrow: bool| {
+        let sugg = Sugg::hir_with_applicability(cx, expr, "_", applicability);
+        EligibleArg::ContainsArg((if needs_borrow { sugg.addr() } else { sugg }).to_string())
+    };
+
+    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, 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, false), expr))
+                }
+            } else {
+                None
+            }
+        },
+        _ => {
+            if switch_to_eager_eval(cx, expr) {
+                Some((get_snippet(expr, true), expr))
+            } else {
+                None
+            }
+        },
+    }
+}
+
+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,98 @@
+#![warn(clippy::manual_contains)]
+#![allow(clippy::eq_op, clippy::useless_vec)]
+
+fn should_lint() {
+    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
+
+    let vec: Vec<u32> = vec![1, 2, 3, 4, 5, 6];
+    let values = &vec[..];
+    let _ = values.contains(&(4 + 1));
+    //~^ 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");
+
+    let vec: Vec<u32> = vec![1, 2, 3, 4, 5, 6];
+    let values = &vec[..];
+    let mut counter = 0;
+    let mut count = || {
+        counter += 1;
+        counter
+    };
+    let _ = values.iter().any(|&v| v == count());
+    let _ = values.iter().any(|&v| v == v * 2);
+}
+
+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
+}

tests/ui/manual_contains.rs

@@ -0,0 +1,98 @@
+#![warn(clippy::manual_contains)]
+#![allow(clippy::eq_op, clippy::useless_vec)]
+
+fn should_lint() {
+    let vec: Vec<u8> = 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<u32> = 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 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 num = 14;
+    let values: [u8; 6] = [3, 14, 15, 92, 6, 5];
+    let _ = values.iter().any(|&v| num == v);
+    //~^ 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
+
+    let vec: Vec<u8> = vec![1, 2, 3, 4, 5, 6];
+    let values = &vec[..];
+    let _ = values.iter().any(|&v| 4 == v);
+    //~^ 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.iter().any(|v| *v == *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.iter().any(|&v| v == "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.iter().any(|&v| v == 4 + 1);
+    //~^ 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");
+
+    let vec: Vec<u32> = vec![1, 2, 3, 4, 5, 6];
+    let values = &vec[..];
+    let mut counter = 0;
+    let mut count = || {
+        counter += 1;
+        counter
+    };
+    let _ = values.iter().any(|&v| v == count());
+    let _ = values.iter().any(|&v| v == v * 2);
+}
+
+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
+}