mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #12417 - Alexendoo:iter-nth, r=flip1995

Browse Source 
Move `iter_nth` to `style`, add machine applicable suggestion

There's no `O(n)` involved with `.iter().nth()` on the linted types since the iterator implementations provide `nth` and/or `advance_by` that operate in `O(1)`

For slice iterators the codegen is equivalent, `VecDeque`'s iterator seems to codegen differently but that doesn't seem significant enough to keep it as a perf lint

changelog: [`iter_nth`] Move to `style`

r? `@flip1995`
This commit is contained in:
bors 2024-03-12 08:54:26 +00:00
commit a8a7371728
5 changed files with 139 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
clippy_lints/src/methods/iter_nth.rs
View File

@ -1,10 +1,10 @@
use super::utils::derefs_to_slice;
use crate::methods::iter_nth_zero;
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::ty::get_type_diagnostic_name;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_span::symbol::sym;
use rustc_span::Span;
use super::ITER_NTH;
@ -12,28 +12,33 @@ pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &hir::Expr<'_>,
iter_recv: &'tcx hir::Expr<'tcx>,
nth_recv: &hir::Expr<'_>,
nth_arg: &hir::Expr<'_>,
is_mut: bool,
) {
let mut_str = if is_mut { "_mut" } else { "" };
let caller_type = if derefs_to_slice(cx, iter_recv, cx.typeck_results().expr_ty(iter_recv)).is_some() {
"slice"
} else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(iter_recv), sym::Vec) {
"`Vec`"
} else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(iter_recv), sym::VecDeque) {
"`VecDeque`"
} else {
iter_nth_zero::check(cx, expr, nth_recv, nth_arg);
return; // caller is not a type that we want to lint
iter_method: &str,
iter_span: Span,
nth_span: Span,
) -> bool {
let caller_type = match get_type_diagnostic_name(cx, cx.typeck_results().expr_ty(iter_recv).peel_refs()) {
Some(sym::Vec) => "`Vec`",
Some(sym::VecDeque) => "`VecDeque`",
_ if cx.typeck_results().expr_ty_adjusted(iter_recv).peel_refs().is_slice() => "slice",
// caller is not a type that we want to lint
_ => return false,
};
span_lint_and_help(
span_lint_and_then(
cx,
ITER_NTH,
expr.span,
&format!("called `.iter{mut_str}().nth()` on a {caller_type}"),
None,
&format!("calling `.get{mut_str}()` is both faster and more readable"),
&format!("called `.{iter_method}().nth()` on a {caller_type}"),
|diag| {
let get_method = if iter_method == "iter_mut" { "get_mut" } else { "get" };
diag.span_suggestion_verbose(
iter_span.to(nth_span),
format!("`{get_method}` is equivalent but more concise"),
get_method,
Applicability::MachineApplicable,
);
},
);
true
}

16
clippy_lints/src/methods/mod.rs
View File

@ -1236,12 +1236,11 @@
declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `.iter().nth()` (and the related
/// `.iter_mut().nth()`) on standard library types with *O*(1) element access.
/// Checks for usage of `.iter().nth()`/`.iter_mut().nth()` on standard library types that have
/// equivalent `.get()`/`.get_mut()` methods.
///
/// ### Why is this bad?
/// `.get()` and `.get_mut()` are more efficient and more
/// readable.
/// `.get()` and `.get_mut()` are equivalent but more concise.
///
/// ### Example
/// ```no_run
@ -1257,7 +1256,7 @@
/// ```
#[clippy::version = "pre 1.29.0"]
pub ITER_NTH,
perf,
style,
"using `.iter().nth()` on a standard library type with O(1) element access"
}
@ -4756,8 +4755,11 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
iter_overeager_cloned::Op::LaterCloned,
false,
),
Some(("iter", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, false),
Some(("iter_mut", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, true),
Some((iter_method @ ("iter" | "iter_mut"), iter_recv, [], iter_span, _)) => {
if !iter_nth::check(cx, expr, iter_recv, iter_method, iter_span, span) {
iter_nth_zero::check(cx, expr, recv, n_arg);
}
},
_ => iter_nth_zero::check(cx, expr, recv, n_arg),
},
("ok_or_else", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or"),

60
tests/ui/iter_nth.fixed Normal file
View File

@ -0,0 +1,60 @@
//@aux-build:option_helpers.rs
#![warn(clippy::iter_nth)]
#![allow(clippy::useless_vec)]
#[macro_use]
extern crate option_helpers;
use option_helpers::IteratorFalsePositives;
use std::collections::VecDeque;
/// Struct to generate false positives for things with `.iter()`.
#[derive(Copy, Clone)]
struct HasIter;
impl HasIter {
fn iter(self) -> IteratorFalsePositives {
IteratorFalsePositives { foo: 0 }
}
fn iter_mut(self) -> IteratorFalsePositives {
IteratorFalsePositives { foo: 0 }
}
}
/// Checks implementation of `ITER_NTH` lint.
fn iter_nth() {
let mut some_vec = vec![0, 1, 2, 3];
let mut boxed_slice: Box<[u8]> = Box::new([0, 1, 2, 3]);
let mut some_vec_deque: VecDeque<_> = some_vec.iter().cloned().collect();
{
// Make sure we lint `.iter()` for relevant types.
let bad_vec = some_vec.get(3);
let bad_slice = &some_vec[..].get(3);
let bad_boxed_slice = boxed_slice.get(3);
let bad_vec_deque = some_vec_deque.get(3);
}
{
// Make sure we lint `.iter_mut()` for relevant types.
let bad_vec = some_vec.get_mut(3);
}
{
let bad_slice = &some_vec[..].get_mut(3);
}
{
let bad_vec_deque = some_vec_deque.get_mut(3);
}
let vec_ref = &Vec::<String>::new();
vec_ref.get(3);
// Make sure we don't lint for non-relevant types.
let false_positive = HasIter;
let ok = false_positive.iter().nth(3);
let ok_mut = false_positive.iter_mut().nth(3);
}
fn main() {}

3
tests/ui/iter_nth.rs
View File

@ -48,6 +48,9 @@ fn iter_nth() {
let bad_vec_deque = some_vec_deque.iter_mut().nth(3);
}
let vec_ref = &Vec::<String>::new();
vec_ref.iter().nth(3);
// Make sure we don't lint for non-relevant types.
let false_positive = HasIter;
let ok = false_positive.iter().nth(3);

48
tests/ui/iter_nth.stderr
View File

@ -4,9 +4,12 @@ error: called `.iter().nth()` on a `Vec`
LL | let bad_vec = some_vec.iter().nth(3);
| ^^^^^^^^^^^^^^^^^^^^^^
|
= help: calling `.get()` is both faster and more readable
= note: `-D clippy::iter-nth` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::iter_nth)]`
help: `get` is equivalent but more concise
|
LL | let bad_vec = some_vec.get(3);
| ~~~
error: called `.iter().nth()` on a slice
--> tests/ui/iter_nth.rs:35:26
@ -14,7 +17,10 @@ error: called `.iter().nth()` on a slice
LL | let bad_slice = &some_vec[..].iter().nth(3);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: calling `.get()` is both faster and more readable
help: `get` is equivalent but more concise
|
LL | let bad_slice = &some_vec[..].get(3);
| ~~~
error: called `.iter().nth()` on a slice
--> tests/ui/iter_nth.rs:36:31
@ -22,7 +28,10 @@ error: called `.iter().nth()` on a slice
LL | let bad_boxed_slice = boxed_slice.iter().nth(3);
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: calling `.get()` is both faster and more readable
help: `get` is equivalent but more concise
|
LL | let bad_boxed_slice = boxed_slice.get(3);
| ~~~
error: called `.iter().nth()` on a `VecDeque`
--> tests/ui/iter_nth.rs:37:29
@ -30,7 +39,10 @@ error: called `.iter().nth()` on a `VecDeque`
LL | let bad_vec_deque = some_vec_deque.iter().nth(3);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: calling `.get()` is both faster and more readable
help: `get` is equivalent but more concise
|
LL | let bad_vec_deque = some_vec_deque.get(3);
| ~~~
error: called `.iter_mut().nth()` on a `Vec`
--> tests/ui/iter_nth.rs:42:23
@ -38,7 +50,10 @@ error: called `.iter_mut().nth()` on a `Vec`
LL | let bad_vec = some_vec.iter_mut().nth(3);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: calling `.get_mut()` is both faster and more readable
help: `get_mut` is equivalent but more concise
|
LL | let bad_vec = some_vec.get_mut(3);
| ~~~~~~~
error: called `.iter_mut().nth()` on a slice
--> tests/ui/iter_nth.rs:45:26
@ -46,7 +61,10 @@ error: called `.iter_mut().nth()` on a slice
LL | let bad_slice = &some_vec[..].iter_mut().nth(3);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: calling `.get_mut()` is both faster and more readable
help: `get_mut` is equivalent but more concise
|
LL | let bad_slice = &some_vec[..].get_mut(3);
| ~~~~~~~
error: called `.iter_mut().nth()` on a `VecDeque`
--> tests/ui/iter_nth.rs:48:29
@ -54,7 +72,21 @@ error: called `.iter_mut().nth()` on a `VecDeque`
LL | let bad_vec_deque = some_vec_deque.iter_mut().nth(3);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: calling `.get_mut()` is both faster and more readable
help: `get_mut` is equivalent but more concise
|
LL | let bad_vec_deque = some_vec_deque.get_mut(3);
| ~~~~~~~
error: aborting due to 7 previous errors
error: called `.iter().nth()` on a `Vec`
--> tests/ui/iter_nth.rs:52:5
|
LL | vec_ref.iter().nth(3);
| ^^^^^^^^^^^^^^^^^^^^^
|
help: `get` is equivalent but more concise
|
LL | vec_ref.get(3);
| ~~~
error: aborting due to 8 previous errors