Skip to content

Commit 8cef0b6

Browse files
ManishearthManishearth
authored
new lint: manual_contains (#13817)
close #13353 Using `contains()` for slices are more efficient than using `iter().any()`. changelog: [`manual_contains`]: new lint
2 parents a8b1782 + 1c0e120 commit 8cef0b6

17 files changed

+459
-29
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5743,6 +5743,7 @@ Released 2018-09-13
57435743
[`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits
57445744
[`manual_c_str_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals
57455745
[`manual_clamp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_clamp
5746+
[`manual_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_contains
57465747
[`manual_div_ceil`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_div_ceil
57475748
[`manual_filter`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter
57485749
[`manual_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter_map

‎clippy_lints/src/booleans.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ impl<'tcx> NonminimalBoolVisitor<'_, 'tcx> {
555555
_ => simplified.push(Bool::Not(Box::new(simple.clone()))),
556556
}
557557
let simple_negated = simple_negate(simple);
558-
if simplified.iter().any(|s| *s == simple_negated) {
558+
if simplified.contains(&simple_negated) {
559559
continue;
560560
}
561561
simplified.push(simple_negated);

‎clippy_lints/src/casts/cast_sign_loss.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,11 @@ fn expr_sign<'cx, 'tcx>(cx: &LateContext<'cx>, mut expr: &'tcx Expr<'tcx>, ty: i
142142
expr = recv;
143143
}
144144

145-
if METHODS_POW.iter().any(|&name| method_name == name)
145+
if METHODS_POW.contains(&method_name)
146146
&& let [arg] = args
147147
{
148148
return pow_call_result_sign(cx, caller, arg);
149-
} else if METHODS_RET_POSITIVE.iter().any(|&name| method_name == name) {
149+
} else if METHODS_RET_POSITIVE.contains(&method_name) {
150150
return Sign::ZeroOrPositive;
151151
}
152152
}

‎clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
418418
crate::methods::ITER_SKIP_ZERO_INFO,
419419
crate::methods::ITER_WITH_DRAIN_INFO,
420420
crate::methods::JOIN_ABSOLUTE_PATHS_INFO,
421+
crate::methods::MANUAL_CONTAINS_INFO,
421422
crate::methods::MANUAL_C_STR_LITERALS_INFO,
422423
crate::methods::MANUAL_FILTER_MAP_INFO,
423424
crate::methods::MANUAL_FIND_MAP_INFO,

‎clippy_lints/src/methods/manual_contains.rs

+113
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::eager_or_lazy::switch_to_eager_eval;
3+
use clippy_utils::peel_hir_pat_refs;
4+
use clippy_utils::source::snippet_with_applicability;
5+
use clippy_utils::sugg::Sugg;
6+
use rustc_ast::UnOp;
7+
use rustc_errors::Applicability;
8+
use rustc_hir::def::Res;
9+
use rustc_hir::{BinOpKind, Body, Expr, ExprKind, HirId, QPath};
10+
use rustc_lint::LateContext;
11+
use rustc_middle::ty::{self};
12+
use rustc_span::source_map::Spanned;
13+
14+
use super::MANUAL_CONTAINS;
15+
16+
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, closure_arg: &Expr<'_>) {
17+
let mut app = Applicability::MachineApplicable;
18+
19+
if !expr.span.from_expansion()
20+
// check if `iter().any()` can be replaced with `contains()`
21+
&& let ExprKind::Closure(closure) = closure_arg.kind
22+
&& let Body{params: [param],value} = cx.tcx.hir().body(closure.body)
23+
&& let ExprKind::Binary(op, lhs, rhs) = value.kind
24+
&& let (peeled_ref_pat, _) = peel_hir_pat_refs(param.pat)
25+
&& let Some((snip,snip_expr)) = can_replace_with_contains(cx, op, lhs, rhs, peeled_ref_pat.hir_id, &mut app)
26+
&& let ref_type = cx.typeck_results().expr_ty_adjusted(recv)
27+
&& let ty::Ref(_, inner_type, _) = ref_type.kind()
28+
&& let ty::Slice(slice_type) = inner_type.kind()
29+
&& *slice_type == cx.typeck_results().expr_ty(snip_expr)
30+
{
31+
span_lint_and_sugg(
32+
cx,
33+
MANUAL_CONTAINS,
34+
expr.span,
35+
"using `contains()` instead of `iter().any()` is more efficient",
36+
"try",
37+
format!(
38+
"{}.contains({})",
39+
snippet_with_applicability(cx, recv.span, "_", &mut app),
40+
snip
41+
),
42+
app,
43+
);
44+
}
45+
}
46+
47+
enum EligibleArg {
48+
IsClosureArg,
49+
ContainsArg(String),
50+
}
51+
52+
fn try_get_eligible_arg<'tcx>(
53+
cx: &LateContext<'tcx>,
54+
expr: &'tcx Expr<'tcx>,
55+
closure_arg_id: HirId,
56+
applicability: &mut Applicability,
57+
) -> Option<(EligibleArg, &'tcx Expr<'tcx>)> {
58+
let mut get_snippet = |expr: &Expr<'_>, needs_borrow: bool| {
59+
let sugg = Sugg::hir_with_applicability(cx, expr, "_", applicability);
60+
EligibleArg::ContainsArg((if needs_borrow { sugg.addr() } else { sugg }).to_string())
61+
};
62+
63+
match expr.kind {
64+
ExprKind::Path(QPath::Resolved(_, path)) => {
65+
if path.res == Res::Local(closure_arg_id) {
66+
Some((EligibleArg::IsClosureArg, expr))
67+
} else {
68+
Some((get_snippet(expr, true), expr))
69+
}
70+
},
71+
ExprKind::Unary(UnOp::Deref, inner) => {
72+
if let ExprKind::Path(QPath::Resolved(_, path)) = inner.kind {
73+
if path.res == Res::Local(closure_arg_id) {
74+
Some((EligibleArg::IsClosureArg, expr))
75+
} else {
76+
Some((get_snippet(inner, false), expr))
77+
}
78+
} else {
79+
None
80+
}
81+
},
82+
_ => {
83+
if switch_to_eager_eval(cx, expr) {
84+
Some((get_snippet(expr, true), expr))
85+
} else {
86+
None
87+
}
88+
},
89+
}
90+
}
91+
92+
fn can_replace_with_contains<'tcx>(
93+
cx: &LateContext<'tcx>,
94+
bin_op: Spanned<BinOpKind>,
95+
left_expr: &'tcx Expr<'tcx>,
96+
right_expr: &'tcx Expr<'tcx>,
97+
closure_arg_id: HirId,
98+
applicability: &mut Applicability,
99+
) -> Option<(String, &'tcx Expr<'tcx>)> {
100+
if bin_op.node != BinOpKind::Eq {
101+
return None;
102+
}
103+
104+
let left_candidate = try_get_eligible_arg(cx, left_expr, closure_arg_id, applicability)?;
105+
let right_candidate = try_get_eligible_arg(cx, right_expr, closure_arg_id, applicability)?;
106+
match (left_candidate, right_candidate) {
107+
((EligibleArg::IsClosureArg, _), (EligibleArg::ContainsArg(snip), candidate_expr))
108+
| ((EligibleArg::ContainsArg(snip), candidate_expr), (EligibleArg::IsClosureArg, _)) => {
109+
Some((snip, candidate_expr))
110+
},
111+
_ => None,
112+
}
113+
}

‎clippy_lints/src/methods/mod.rs

+30
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ mod iter_with_drain;
5454
mod iterator_step_by_zero;
5555
mod join_absolute_paths;
5656
mod manual_c_str_literals;
57+
mod manual_contains;
5758
mod manual_inspect;
5859
mod manual_is_variant_and;
5960
mod manual_next_back;
@@ -4434,6 +4435,31 @@ declare_clippy_lint! {
44344435
"calling .bytes() is very inefficient when data is not in memory"
44354436
}
44364437

4438+
declare_clippy_lint! {
4439+
/// ### What it does
4440+
/// Checks for usage of `iter().any()` on slices when it can be replaced with `contains()` and suggests doing so.
4441+
///
4442+
/// ### Why is this bad?
4443+
/// `contains()` is more concise and idiomatic, sometimes more fast.
4444+
///
4445+
/// ### Example
4446+
/// ```no_run
4447+
/// fn foo(values: &[u8]) -> bool {
4448+
/// values.iter().any(|&v| v == 10)
4449+
/// }
4450+
/// ```
4451+
/// Use instead:
4452+
/// ```no_run
4453+
/// fn foo(values: &[u8]) -> bool {
4454+
/// values.contains(&10)
4455+
/// }
4456+
/// ```
4457+
#[clippy::version = "1.86.0"]
4458+
pub MANUAL_CONTAINS,
4459+
perf,
4460+
"unnecessary `iter().any()` on slices that can be replaced with `contains()`"
4461+
}
4462+
44374463
#[expect(clippy::struct_excessive_bools)]
44384464
pub struct Methods {
44394465
avoid_breaking_exported_api: bool,
@@ -4609,6 +4635,7 @@ impl_lint_pass!(Methods => [
46094635
SLICED_STRING_AS_BYTES,
46104636
RETURN_AND_THEN,
46114637
UNBUFFERED_BYTES,
4638+
MANUAL_CONTAINS,
46124639
]);
46134640

46144641
/// Extracts a method call name, args, and `Span` of the method name.
@@ -4866,6 +4893,9 @@ impl Methods {
48664893
Some(("map", _, [map_arg], _, map_call_span)) => {
48674894
map_all_any_identity::check(cx, expr, recv, map_call_span, map_arg, call_span, arg, "any");
48684895
},
4896+
Some(("iter", iter_recv, ..)) => {
4897+
manual_contains::check(cx, expr, iter_recv, arg);
4898+
},
48694899
_ => {},
48704900
}
48714901
},

‎clippy_lints/src/size_of_in_element_count.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ fn get_pointee_ty_and_count_expr<'tcx>(
9999
if let ExprKind::MethodCall(method_path, ptr_self, [.., count], _) = expr.kind
100100
// Find calls to copy_{from,to}{,_nonoverlapping}
101101
&& let method_ident = method_path.ident.as_str()
102-
&& METHODS.iter().any(|m| *m == method_ident)
102+
&& METHODS.contains(&method_ident)
103103

104104
// Get the pointee type
105105
&& let ty::RawPtr(pointee_ty, _) =

‎tests/ui/manual_contains.fixed

+98
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
#![warn(clippy::manual_contains)]
2+
#![allow(clippy::eq_op, clippy::useless_vec)]
3+
4+
fn should_lint() {
5+
let vec: Vec<u8> = vec![1, 2, 3, 4, 5, 6];
6+
let values = &vec[..];
7+
let _ = values.contains(&4);
8+
//~^ manual_contains
9+
10+
let vec: Vec<u32> = vec![1, 2, 3, 4, 5, 6];
11+
let values = &vec[..];
12+
let _ = values.contains(&4);
13+
//~^ manual_contains
14+
15+
let values: [u8; 6] = [3, 14, 15, 92, 6, 5];
16+
let _ = values.contains(&10);
17+
//~^ manual_contains
18+
19+
let num = 14;
20+
let values: [u8; 6] = [3, 14, 15, 92, 6, 5];
21+
let _ = values.contains(&num);
22+
//~^ manual_contains
23+
24+
let num = 14;
25+
let values: [u8; 6] = [3, 14, 15, 92, 6, 5];
26+
let _ = values.contains(&num);
27+
//~^ manual_contains
28+
29+
let values: [u8; 6] = [3, 14, 15, 92, 6, 5];
30+
let _ = values.contains(&4);
31+
//~^ manual_contains
32+
33+
let vec: Vec<u8> = vec![1, 2, 3, 4, 5, 6];
34+
let values = &vec[..];
35+
let _ = values.contains(&4);
36+
//~^ manual_contains
37+
38+
let vec: Vec<u8> = vec![1, 2, 3, 4, 5, 6];
39+
let values = &vec[..];
40+
let a = &4;
41+
let _ = values.contains(a);
42+
//~^ manual_contains
43+
44+
let vec = vec!["1", "2", "3", "4", "5", "6"];
45+
let values = &vec[..];
46+
let _ = values.contains(&"4");
47+
//~^ manual_contains
48+
49+
let vec: Vec<u32> = vec![1, 2, 3, 4, 5, 6];
50+
let values = &vec[..];
51+
let _ = values.contains(&(4 + 1));
52+
//~^ manual_contains
53+
}
54+
55+
fn should_not_lint() {
56+
let values: [u8; 6] = [3, 14, 15, 92, 6, 5];
57+
let _ = values.iter().any(|&v| v > 10);
58+
59+
let vec: Vec<u32> = vec![1, 2, 3, 4, 5, 6];
60+
let values = &vec[..];
61+
let _ = values.iter().any(|&v| v % 2 == 0);
62+
let _ = values.iter().any(|&v| v * 2 == 6);
63+
let _ = values.iter().any(|&v| v == v);
64+
let _ = values.iter().any(|&v| 4 == 4);
65+
let _ = values.contains(&4);
66+
67+
let a = 1;
68+
let b = 2;
69+
let _ = values.iter().any(|&v| a == b);
70+
let _ = values.iter().any(|&v| a == 4);
71+
72+
let vec: Vec<String> = vec!["1", "2", "3", "4", "5", "6"]
73+
.iter()
74+
.map(|&x| x.to_string())
75+
.collect();
76+
let values = &vec[..];
77+
let _ = values.iter().any(|v| v == "4");
78+
79+
let vec: Vec<u32> = vec![1, 2, 3, 4, 5, 6];
80+
let values = &vec[..];
81+
let mut counter = 0;
82+
let mut count = || {
83+
counter += 1;
84+
counter
85+
};
86+
let _ = values.iter().any(|&v| v == count());
87+
let _ = values.iter().any(|&v| v == v * 2);
88+
}
89+
90+
fn foo(values: &[u8]) -> bool {
91+
values.contains(&10)
92+
//~^ manual_contains
93+
}
94+
95+
fn bar(values: [u8; 3]) -> bool {
96+
values.contains(&10)
97+
//~^ manual_contains
98+
}

0 commit comments

Comments
 (0)