-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
new lint to detect inefficient iter().any()
#13817
base: master
Are you sure you want to change the base?
Changes from 4 commits
c52ebf2
c1d5108
b3a1693
19357ca
e964cbc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
use clippy_utils::diagnostics::span_lint; | ||
use rustc_hir::{Expr, ExprKind}; | ||
use rustc_lint::LateContext; | ||
use rustc_middle::ty::{self}; | ||
|
||
use super::{SLICE_ITER_ANY, UNNECESSARY_ITER_ANY, method_call}; | ||
|
||
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) { | ||
if !expr.span.from_expansion() | ||
// any() | ||
&& let Some((name, recv, args, _, _)) = method_call(expr) | ||
&& name == "any" | ||
// check if the inner closure is a equality check | ||
&& args.len() == 1 | ||
&& let ExprKind::Closure(closure) = args[0].kind | ||
&& let body = cx.tcx.hir().body(closure.body) | ||
&& let ExprKind::Binary(op, _, _) = body.value.kind | ||
&& op.node == rustc_ast::ast::BinOpKind::Eq | ||
// iter() | ||
&& let Some((name, recv, _, _, _)) = method_call(recv) | ||
&& name == "iter" | ||
{ | ||
let ref_type = cx.typeck_results().expr_ty(recv); | ||
|
||
match ref_type.kind() { | ||
ty::Ref(_, inner_type, _) if inner_type.is_slice() => { | ||
// check if the receiver is a u8/i8 slice | ||
if let ty::Slice(slice_type) = inner_type.kind() | ||
&& (slice_type.to_string() == "u8" || slice_type.to_string() == "i8") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my environment, I could only see the speedup for the |
||
{ | ||
span_lint( | ||
cx, | ||
SLICE_ITER_ANY, | ||
expr.span, | ||
"using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient", | ||
); | ||
} else if let ty::Slice(slice_type) = inner_type.kind() | ||
&& slice_type.is_numeric() | ||
{ | ||
span_lint( | ||
cx, | ||
UNNECESSARY_ITER_ANY, | ||
expr.span, | ||
"using `contains()` instead of `iter().any()` is more readable", | ||
); | ||
} | ||
}, | ||
// if it's an array that uses `iter().any()` and its closure is an equality check, suggest using | ||
// `contains()` (currently only for numeric arrays because of the difficulty in determining whether | ||
// `contains()` can be replaced by `contains()` for arrays of general types) | ||
lapla-cogito marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ty::Array(array_type, _) if array_type.is_numeric() => span_lint( | ||
cx, | ||
UNNECESSARY_ITER_ANY, | ||
expr.span, | ||
"using `contains()` instead of `iter().any()` is more readable", | ||
), | ||
_ => {}, | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
#![warn(clippy::slice_iter_any)] | ||
|
||
fn main() { | ||
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()` on u8/i8 slices is more efficient | ||
|
||
let vec: Vec<u8> = vec![1, 2, 3, 4, 5, 6]; | ||
let values = &vec[..]; | ||
let _ = values.contains(&4); | ||
// no error, because it uses `contains()` | ||
|
||
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 readable | ||
|
||
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 readable | ||
|
||
let values: [u8; 6] = [3, 14, 15, 92, 6, 5]; | ||
let _ = values.iter().any(|&v| v > 10); | ||
// no error, because this `any()` is not for equality | ||
} | ||
|
||
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) | ||
//~^ ERROR: using `contains()` instead of `iter().any()` is more readable | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
error: using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient | ||
--> tests/ui/iter_any.rs:6:13 | ||
| | ||
LL | let _ = values.iter().any(|&v| v == 4); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= note: `-D clippy::slice-iter-any` implied by `-D warnings` | ||
= help: to override `-D warnings` add `#[allow(clippy::slice_iter_any)]` | ||
|
||
error: using `contains()` instead of `iter().any()` is more readable | ||
--> tests/ui/iter_any.rs:16:13 | ||
| | ||
LL | let _ = values.iter().any(|&v| v == 4); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
= note: `-D clippy::unnecessary-iter-any` implied by `-D warnings` | ||
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_iter_any)]` | ||
|
||
error: using `contains()` instead of `iter().any()` is more readable | ||
--> tests/ui/iter_any.rs:20:13 | ||
| | ||
LL | let _ = values.iter().any(|&v| v == 10); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
error: using `contains()` instead of `iter().any()` on u8/i8 slices is more efficient | ||
--> tests/ui/iter_any.rs:29:5 | ||
| | ||
LL | values.iter().any(|&v| v == 10) | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
error: using `contains()` instead of `iter().any()` is more readable | ||
--> tests/ui/iter_any.rs:34:5 | ||
| | ||
LL | values.iter().any(|&v| v == 10) | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
error: aborting due to 5 previous errors | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: Probably should be
expr_ty_adjusted
to handle autoderef. Add a test that ensures this works on a vectorvec.iter().any(...)
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I changed to use
expr_ty_adjusted
and added a test for this in e964cbc.