Skip to content

Commit 301d293

Browse files
AlexendooAlexendoo
committed
Move iter_nth to style, add machine applicable suggestion
1 parent 5214ab5 commit 301d293

File tree

5 files changed

+139
-37
lines changed

5 files changed

+139
-37
lines changed

‎clippy_lints/src/methods/iter_nth.rs‎

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,44 @@
1-
use super::utils::derefs_to_slice;
2-
use crate::methods::iter_nth_zero;
3-
use clippy_utils::diagnostics::span_lint_and_help;
4-
use clippy_utils::ty::is_type_diagnostic_item;
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::ty::get_type_diagnostic_name;
3+
use rustc_errors::Applicability;
54
use rustc_hir as hir;
65
use rustc_lint::LateContext;
76
use rustc_span::symbol::sym;
7+
use rustc_span::Span;
88

99
use super::ITER_NTH;
1010

1111
pub(super) fn check<'tcx>(
1212
cx: &LateContext<'tcx>,
1313
expr: &hir::Expr<'_>,
1414
iter_recv: &'tcx hir::Expr<'tcx>,
15-
nth_recv: &hir::Expr<'_>,
16-
nth_arg: &hir::Expr<'_>,
17-
is_mut: bool,
18-
) {
19-
let mut_str = if is_mut { "_mut" } else { "" };
20-
let caller_type = if derefs_to_slice(cx, iter_recv, cx.typeck_results().expr_ty(iter_recv)).is_some() {
21-
"slice"
22-
} else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(iter_recv), sym::Vec) {
23-
"`Vec`"
24-
} else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(iter_recv), sym::VecDeque) {
25-
"`VecDeque`"
26-
} else {
27-
iter_nth_zero::check(cx, expr, nth_recv, nth_arg);
28-
return; // caller is not a type that we want to lint
15+
iter_method: &str,
16+
iter_span: Span,
17+
nth_span: Span,
18+
) -> bool {
19+
let caller_type = match get_type_diagnostic_name(cx, cx.typeck_results().expr_ty(iter_recv).peel_refs()) {
20+
Some(sym::Vec) => "`Vec`",
21+
Some(sym::VecDeque) => "`VecDeque`",
22+
_ if cx.typeck_results().expr_ty_adjusted(iter_recv).peel_refs().is_slice() => "slice",
23+
// caller is not a type that we want to lint
24+
_ => return false,
2925
};
3026

31-
span_lint_and_help(
27+
span_lint_and_then(
3228
cx,
3329
ITER_NTH,
3430
expr.span,
35-
&format!("called `.iter{mut_str}().nth()` on a {caller_type}"),
36-
None,
37-
&format!("calling `.get{mut_str}()` is both faster and more readable"),
31+
&format!("called `.{iter_method}().nth()` on a {caller_type}"),
32+
|diag| {
33+
let get_method = if iter_method == "iter_mut" { "get_mut" } else { "get" };
34+
diag.span_suggestion_verbose(
35+
iter_span.to(nth_span),
36+
format!("`{get_method}` is equivalent but more concise"),
37+
get_method,
38+
Applicability::MachineApplicable,
39+
);
40+
},
3841
);
42+
43+
true
3944
}

‎clippy_lints/src/methods/mod.rs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,12 +1235,11 @@ declare_clippy_lint! {
12351235

12361236
declare_clippy_lint! {
12371237
/// ### What it does
1238-
/// Checks for usage of `.iter().nth()` (and the related
1239-
/// `.iter_mut().nth()`) on standard library types with *O*(1) element access.
1238+
/// Checks for usage of `.iter().nth()`/`.iter_mut().nth()` on standard library types that have
1239+
/// equivalent `.get()`/`.get_mut()` methods.
12401240
///
12411241
/// ### Why is this bad?
1242-
/// `.get()` and `.get_mut()` are more efficient and more
1243-
/// readable.
1242+
/// `.get()` and `.get_mut()` are equivalent but more concise.
12441243
///
12451244
/// ### Example
12461245
/// ```no_run
@@ -1256,7 +1255,7 @@ declare_clippy_lint! {
12561255
/// ```
12571256
#[clippy::version = "pre 1.29.0"]
12581257
pub ITER_NTH,
1259-
perf,
1258+
style,
12601259
"using `.iter().nth()` on a standard library type with O(1) element access"
12611260
}
12621261

@@ -4723,8 +4722,11 @@ impl Methods {
47234722
iter_overeager_cloned::Op::LaterCloned,
47244723
false,
47254724
),
4726-
Some(("iter", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, false),
4727-
Some(("iter_mut", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, true),
4725+
Some((iter_method @ ("iter" | "iter_mut"), iter_recv, [], iter_span, _)) => {
4726+
if !iter_nth::check(cx, expr, iter_recv, iter_method, iter_span, span) {
4727+
iter_nth_zero::check(cx, expr, recv, n_arg);
4728+
}
4729+
},
47284730
_ => iter_nth_zero::check(cx, expr, recv, n_arg),
47294731
},
47304732
("ok_or_else", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or"),

‎tests/ui/iter_nth.fixed‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
//@aux-build:option_helpers.rs
2+
3+
#![warn(clippy::iter_nth)]
4+
#![allow(clippy::useless_vec)]
5+
6+
#[macro_use]
7+
extern crate option_helpers;
8+
9+
use option_helpers::IteratorFalsePositives;
10+
use std::collections::VecDeque;
11+
12+
/// Struct to generate false positives for things with `.iter()`.
13+
#[derive(Copy, Clone)]
14+
struct HasIter;
15+
16+
impl HasIter {
17+
fn iter(self) -> IteratorFalsePositives {
18+
IteratorFalsePositives { foo: 0 }
19+
}
20+
21+
fn iter_mut(self) -> IteratorFalsePositives {
22+
IteratorFalsePositives { foo: 0 }
23+
}
24+
}
25+
26+
/// Checks implementation of `ITER_NTH` lint.
27+
fn iter_nth() {
28+
let mut some_vec = vec![0, 1, 2, 3];
29+
let mut boxed_slice: Box<[u8]> = Box::new([0, 1, 2, 3]);
30+
let mut some_vec_deque: VecDeque<_> = some_vec.iter().cloned().collect();
31+
32+
{
33+
// Make sure we lint `.iter()` for relevant types.
34+
let bad_vec = some_vec.get(3);
35+
let bad_slice = &some_vec[..].get(3);
36+
let bad_boxed_slice = boxed_slice.get(3);
37+
let bad_vec_deque = some_vec_deque.get(3);
38+
}
39+
40+
{
41+
// Make sure we lint `.iter_mut()` for relevant types.
42+
let bad_vec = some_vec.get_mut(3);
43+
}
44+
{
45+
let bad_slice = &some_vec[..].get_mut(3);
46+
}
47+
{
48+
let bad_vec_deque = some_vec_deque.get_mut(3);
49+
}
50+
51+
let vec_ref = &Vec::<String>::new();
52+
vec_ref.get(3);
53+
54+
// Make sure we don't lint for non-relevant types.
55+
let false_positive = HasIter;
56+
let ok = false_positive.iter().nth(3);
57+
let ok_mut = false_positive.iter_mut().nth(3);
58+
}
59+
60+
fn main() {}

‎tests/ui/iter_nth.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ fn iter_nth() {
4848
let bad_vec_deque = some_vec_deque.iter_mut().nth(3);
4949
}
5050

51+
let vec_ref = &Vec::<String>::new();
52+
vec_ref.iter().nth(3);
53+
5154
// Make sure we don't lint for non-relevant types.
5255
let false_positive = HasIter;
5356
let ok = false_positive.iter().nth(3);

‎tests/ui/iter_nth.stderr‎

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,57 +4,89 @@ error: called `.iter().nth()` on a `Vec`
44
LL | let bad_vec = some_vec.iter().nth(3);
55
| ^^^^^^^^^^^^^^^^^^^^^^
66
|
7-
= help: calling `.get()` is both faster and more readable
87
= note: `-D clippy::iter-nth` implied by `-D warnings`
98
= help: to override `-D warnings` add `#[allow(clippy::iter_nth)]`
9+
help: `get` is equivalent but more concise
10+
|
11+
LL | let bad_vec = some_vec.get(3);
12+
| ~~~
1013

1114
error: called `.iter().nth()` on a slice
1215
--> tests/ui/iter_nth.rs:35:26
1316
|
1417
LL | let bad_slice = &some_vec[..].iter().nth(3);
1518
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
1619
|
17-
= help: calling `.get()` is both faster and more readable
20+
help: `get` is equivalent but more concise
21+
|
22+
LL | let bad_slice = &some_vec[..].get(3);
23+
| ~~~
1824

1925
error: called `.iter().nth()` on a slice
2026
--> tests/ui/iter_nth.rs:36:31
2127
|
2228
LL | let bad_boxed_slice = boxed_slice.iter().nth(3);
2329
| ^^^^^^^^^^^^^^^^^^^^^^^^^
2430
|
25-
= help: calling `.get()` is both faster and more readable
31+
help: `get` is equivalent but more concise
32+
|
33+
LL | let bad_boxed_slice = boxed_slice.get(3);
34+
| ~~~
2635

2736
error: called `.iter().nth()` on a `VecDeque`
2837
--> tests/ui/iter_nth.rs:37:29
2938
|
3039
LL | let bad_vec_deque = some_vec_deque.iter().nth(3);
3140
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3241
|
33-
= help: calling `.get()` is both faster and more readable
42+
help: `get` is equivalent but more concise
43+
|
44+
LL | let bad_vec_deque = some_vec_deque.get(3);
45+
| ~~~
3446

3547
error: called `.iter_mut().nth()` on a `Vec`
3648
--> tests/ui/iter_nth.rs:42:23
3749
|
3850
LL | let bad_vec = some_vec.iter_mut().nth(3);
3951
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
4052
|
41-
= help: calling `.get_mut()` is both faster and more readable
53+
help: `get_mut` is equivalent but more concise
54+
|
55+
LL | let bad_vec = some_vec.get_mut(3);
56+
| ~~~~~~~
4257

4358
error: called `.iter_mut().nth()` on a slice
4459
--> tests/ui/iter_nth.rs:45:26
4560
|
4661
LL | let bad_slice = &some_vec[..].iter_mut().nth(3);
4762
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4863
|
49-
= help: calling `.get_mut()` is both faster and more readable
64+
help: `get_mut` is equivalent but more concise
65+
|
66+
LL | let bad_slice = &some_vec[..].get_mut(3);
67+
| ~~~~~~~
5068

5169
error: called `.iter_mut().nth()` on a `VecDeque`
5270
--> tests/ui/iter_nth.rs:48:29
5371
|
5472
LL | let bad_vec_deque = some_vec_deque.iter_mut().nth(3);
5573
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5674
|
57-
= help: calling `.get_mut()` is both faster and more readable
75+
help: `get_mut` is equivalent but more concise
76+
|
77+
LL | let bad_vec_deque = some_vec_deque.get_mut(3);
78+
| ~~~~~~~
79+
80+
error: called `.iter().nth()` on a `Vec`
81+
--> tests/ui/iter_nth.rs:52:5
82+
|
83+
LL | vec_ref.iter().nth(3);
84+
| ^^^^^^^^^^^^^^^^^^^^^
85+
|
86+
help: `get` is equivalent but more concise
87+
|
88+
LL | vec_ref.get(3);
89+
| ~~~
5890

59-
error: aborting due to 7 previous errors
91+
error: aborting due to 8 previous errors
6092

0 commit comments

Comments
 (0)