Skip to content

Commit

Permalink
new lint: manual_contains (#13817)
Browse files Browse the repository at this point in the history
close #13353

Using `contains()` for slices are more efficient than using
`iter().any()`.

changelog: [`manual_contains`]: new lint
  • Loading branch information
Manishearth authored Feb 15, 2025
2 parents a8b1782 + 1c0e120 commit 8cef0b6
Show file tree
Hide file tree
Showing 17 changed files with 459 additions and 29 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5743,6 +5743,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
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/booleans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ impl<'tcx> NonminimalBoolVisitor<'_, 'tcx> {
_ => simplified.push(Bool::Not(Box::new(simple.clone()))),
}
let simple_negated = simple_negate(simple);
if simplified.iter().any(|s| *s == simple_negated) {
if simplified.contains(&simple_negated) {
continue;
}
simplified.push(simple_negated);
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/casts/cast_sign_loss.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,11 @@ fn expr_sign<'cx, 'tcx>(cx: &LateContext<'cx>, mut expr: &'tcx Expr<'tcx>, ty: i
expr = recv;
}

if METHODS_POW.iter().any(|&name| method_name == name)
if METHODS_POW.contains(&method_name)
&& let [arg] = args
{
return pow_call_result_sign(cx, caller, arg);
} else if METHODS_RET_POSITIVE.iter().any(|&name| method_name == name) {
} else if METHODS_RET_POSITIVE.contains(&method_name) {
return Sign::ZeroOrPositive;
}
}
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,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,
Expand Down
113 changes: 113 additions & 0 deletions clippy_lints/src/methods/manual_contains.rs
Original file line number Diff line number Diff line change
@@ -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,
}
}
30 changes: 30 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -4434,6 +4435,31 @@ declare_clippy_lint! {
"calling .bytes() is very inefficient when data is not in memory"
}

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()`"
}

#[expect(clippy::struct_excessive_bools)]
pub struct Methods {
avoid_breaking_exported_api: bool,
Expand Down Expand Up @@ -4609,6 +4635,7 @@ impl_lint_pass!(Methods => [
SLICED_STRING_AS_BYTES,
RETURN_AND_THEN,
UNBUFFERED_BYTES,
MANUAL_CONTAINS,
]);

/// Extracts a method call name, args, and `Span` of the method name.
Expand Down Expand Up @@ -4866,6 +4893,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);
},
_ => {},
}
},
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/size_of_in_element_count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ fn get_pointee_ty_and_count_expr<'tcx>(
if let ExprKind::MethodCall(method_path, ptr_self, [.., count], _) = expr.kind
// Find calls to copy_{from,to}{,_nonoverlapping}
&& let method_ident = method_path.ident.as_str()
&& METHODS.iter().any(|m| *m == method_ident)
&& METHODS.contains(&method_ident)

// Get the pointee type
&& let ty::RawPtr(pointee_ty, _) =
Expand Down
98 changes: 98 additions & 0 deletions tests/ui/manual_contains.fixed
Original file line number Diff line number Diff line change
@@ -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);
//~^ manual_contains

let vec: Vec<u32> = vec![1, 2, 3, 4, 5, 6];
let values = &vec[..];
let _ = values.contains(&4);
//~^ manual_contains

let values: [u8; 6] = [3, 14, 15, 92, 6, 5];
let _ = values.contains(&10);
//~^ manual_contains

let num = 14;
let values: [u8; 6] = [3, 14, 15, 92, 6, 5];
let _ = values.contains(&num);
//~^ manual_contains

let num = 14;
let values: [u8; 6] = [3, 14, 15, 92, 6, 5];
let _ = values.contains(&num);
//~^ manual_contains

let values: [u8; 6] = [3, 14, 15, 92, 6, 5];
let _ = values.contains(&4);
//~^ manual_contains

let vec: Vec<u8> = vec![1, 2, 3, 4, 5, 6];
let values = &vec[..];
let _ = values.contains(&4);
//~^ manual_contains

let vec: Vec<u8> = vec![1, 2, 3, 4, 5, 6];
let values = &vec[..];
let a = &4;
let _ = values.contains(a);
//~^ manual_contains

let vec = vec!["1", "2", "3", "4", "5", "6"];
let values = &vec[..];
let _ = values.contains(&"4");
//~^ manual_contains

let vec: Vec<u32> = vec![1, 2, 3, 4, 5, 6];
let values = &vec[..];
let _ = values.contains(&(4 + 1));
//~^ manual_contains
}

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)
//~^ manual_contains
}

fn bar(values: [u8; 3]) -> bool {
values.contains(&10)
//~^ manual_contains
}
Loading

0 comments on commit 8cef0b6

Please sign in to comment.