Skip to content

Commit ea2ea62

Browse files
committed
Note that using enumerate() will swap the arguments
The autofix now: - includes the swapping of index and element in the closure in which the content will be consumed; - notes, with applicability `MaybeIncorrect` (because it will be), that the element and the index will be swapped.
1 parent ed143af commit ea2ea62

File tree

7 files changed

+172
-15
lines changed

7 files changed

+172
-15
lines changed
Lines changed: 87 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1-
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::source::snippet;
3-
use clippy_utils::{SpanlessEq, higher, is_integer_const, is_trait_method};
1+
use clippy_utils::diagnostics::span_lint_and_then;
2+
use clippy_utils::source::{SpanRangeExt as _, snippet_with_applicability};
3+
use clippy_utils::{SpanlessEq, get_parent_expr, higher, is_integer_const, is_trait_method, sym};
44
use rustc_errors::Applicability;
5-
use rustc_hir::{Expr, ExprKind, QPath};
5+
use rustc_hir::{Expr, ExprKind, Node, Pat, PatKind, QPath};
66
use rustc_lint::LateContext;
7-
use rustc_span::sym;
87

98
use super::RANGE_ZIP_WITH_LEN;
109

@@ -21,14 +20,93 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, recv: &'
2120
&& let ExprKind::Path(QPath::Resolved(_, len_path)) = len_recv.kind
2221
&& SpanlessEq::new(cx).eq_path_segments(iter_path.segments, len_path.segments)
2322
{
24-
span_lint_and_sugg(
23+
span_lint_and_then(
2524
cx,
2625
RANGE_ZIP_WITH_LEN,
2726
expr.span,
2827
"using `.zip()` with a range and `.len()`",
29-
"try",
30-
format!("{}.iter().enumerate()", snippet(cx, recv.span, "_")),
31-
Applicability::MachineApplicable,
28+
|diag| {
29+
// If the iterator content is consumed by a pattern with exactly two elements, swap
30+
// the order of those elements. Otherwise, the suggestion will be marked as
31+
// `Applicability::MaybeIncorrect` (because it will be), and a note will be added
32+
// to the diagnostic to underline the swapping of the index and the content.
33+
let pat = methods_pattern(cx, expr).or_else(|| for_loop_pattern(cx, expr));
34+
let invert_bindings = if let Some(pat) = pat
35+
&& pat.span.eq_ctxt(expr.span)
36+
&& let PatKind::Tuple([first, second], _) = pat.kind
37+
{
38+
Some((first.span, second.span))
39+
} else {
40+
None
41+
};
42+
let mut app = Applicability::MachineApplicable;
43+
let mut suggestions = vec![(
44+
expr.span,
45+
format!(
46+
"{}.iter().enumerate()",
47+
snippet_with_applicability(cx, recv.span, "_", &mut app)
48+
),
49+
)];
50+
if let Some((left, right)) = invert_bindings
51+
&& let Some(snip_left) = left.get_source_text(cx)
52+
&& let Some(snip_right) = right.get_source_text(cx)
53+
{
54+
suggestions.extend([(left, snip_right.to_string()), (right, snip_left.to_string())]);
55+
} else {
56+
app = Applicability::MaybeIncorrect;
57+
}
58+
diag.multipart_suggestion("use", suggestions, app);
59+
if app != Applicability::MachineApplicable {
60+
diag.note("the order of the element and the index will be swapped");
61+
}
62+
},
3263
);
3364
}
3465
}
66+
67+
/// If `expr` is the argument of a `for` loop, return the loop pattern.
68+
fn for_loop_pattern<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx Pat<'tcx>> {
69+
cx.tcx.hir_parent_iter(expr.hir_id).find_map(|(_, node)| {
70+
if let Node::Expr(ancestor_expr) = node
71+
&& let Some(for_loop) = higher::ForLoop::hir(ancestor_expr)
72+
&& for_loop.arg.hir_id == expr.hir_id
73+
{
74+
Some(for_loop.pat)
75+
} else {
76+
None
77+
}
78+
})
79+
}
80+
81+
/// If `expr` is the receiver of an `Iterator` method which consumes the iterator elements and feed
82+
/// them to a closure, return the pattern of the closure.
83+
fn methods_pattern<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx Pat<'tcx>> {
84+
if let Some(parent_expr) = get_parent_expr(cx, expr)
85+
&& is_trait_method(cx, expr, sym::Iterator)
86+
&& let ExprKind::MethodCall(method, recv, [arg], _) = parent_expr.kind
87+
&& recv.hir_id == expr.hir_id
88+
&& matches!(
89+
method.ident.name,
90+
sym::all
91+
| sym::any
92+
| sym::filter_map
93+
| sym::find_map
94+
| sym::flat_map
95+
| sym::for_each
96+
| sym::is_partitioned
97+
| sym::is_sorted_by_key
98+
| sym::map
99+
| sym::map_while
100+
| sym::position
101+
| sym::rposition
102+
| sym::try_for_each
103+
)
104+
&& let ExprKind::Closure(closure) = arg.kind
105+
&& let body = cx.tcx.hir_body(closure.body)
106+
&& let [param] = body.params
107+
{
108+
Some(param.pat)
109+
} else {
110+
None
111+
}
112+
}

clippy_utils/src/sym.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,10 @@ generate! {
191191
is_none,
192192
is_none_or,
193193
is_ok,
194+
is_partitioned,
194195
is_some,
195196
is_some_and,
197+
is_sorted_by_key,
196198
isqrt,
197199
itertools,
198200
join,
@@ -210,6 +212,7 @@ generate! {
210212
map_continue,
211213
map_or,
212214
map_or_else,
215+
map_while,
213216
match_indices,
214217
matches,
215218
max,
@@ -349,6 +352,7 @@ generate! {
349352
trim_start,
350353
trim_start_matches,
351354
truncate,
355+
try_for_each,
352356
unreachable_pub,
353357
unsafe_removed_from_name,
354358
unused,

tests/ui/range.fixed

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,23 @@
11
#![allow(clippy::useless_vec)]
22
#[warn(clippy::range_zip_with_len)]
33
fn main() {
4-
let v1 = vec![1, 2, 3];
5-
let v2 = vec![4, 5];
4+
let v1: Vec<u64> = vec![1, 2, 3];
5+
let v2: Vec<u64> = vec![4, 5];
66
let _x = v1.iter().enumerate();
77
//~^ range_zip_with_len
88

9+
//~v range_zip_with_len
10+
for (i, e) in v1.iter().enumerate() {
11+
let _: &u64 = e;
12+
let _: usize = i;
13+
}
14+
15+
//~v range_zip_with_len
16+
v1.iter().enumerate().for_each(|(i, e)| {
17+
let _: &u64 = e;
18+
let _: usize = i;
19+
});
20+
921
let _y = v1.iter().zip(0..v2.len()); // No error
1022
}
1123

tests/ui/range.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,23 @@
11
#![allow(clippy::useless_vec)]
22
#[warn(clippy::range_zip_with_len)]
33
fn main() {
4-
let v1 = vec![1, 2, 3];
5-
let v2 = vec![4, 5];
4+
let v1: Vec<u64> = vec![1, 2, 3];
5+
let v2: Vec<u64> = vec![4, 5];
66
let _x = v1.iter().zip(0..v1.len());
77
//~^ range_zip_with_len
88

9+
//~v range_zip_with_len
10+
for (e, i) in v1.iter().zip(0..v1.len()) {
11+
let _: &u64 = e;
12+
let _: usize = i;
13+
}
14+
15+
//~v range_zip_with_len
16+
v1.iter().zip(0..v1.len()).for_each(|(e, i)| {
17+
let _: &u64 = e;
18+
let _: usize = i;
19+
});
20+
921
let _y = v1.iter().zip(0..v2.len()); // No error
1022
}
1123

tests/ui/range.stderr

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,35 @@ error: using `.zip()` with a range and `.len()`
22
--> tests/ui/range.rs:6:14
33
|
44
LL | let _x = v1.iter().zip(0..v1.len());
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `v1.iter().enumerate()`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `v1.iter().enumerate()`
66
|
7+
= note: the order of the element and the index will be swapped
78
= note: `-D clippy::range-zip-with-len` implied by `-D warnings`
89
= help: to override `-D warnings` add `#[allow(clippy::range_zip_with_len)]`
910

10-
error: aborting due to 1 previous error
11+
error: using `.zip()` with a range and `.len()`
12+
--> tests/ui/range.rs:10:19
13+
|
14+
LL | for (e, i) in v1.iter().zip(0..v1.len()) {
15+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
16+
|
17+
help: use
18+
|
19+
LL - for (e, i) in v1.iter().zip(0..v1.len()) {
20+
LL + for (i, e) in v1.iter().enumerate() {
21+
|
22+
23+
error: using `.zip()` with a range and `.len()`
24+
--> tests/ui/range.rs:16:5
25+
|
26+
LL | v1.iter().zip(0..v1.len()).for_each(|(e, i)| {
27+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
28+
|
29+
help: use
30+
|
31+
LL - v1.iter().zip(0..v1.len()).for_each(|(e, i)| {
32+
LL + v1.iter().enumerate().for_each(|(i, e)| {
33+
|
34+
35+
error: aborting due to 3 previous errors
1136

tests/ui/range_unfixable.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
//@no-rustfix
2+
#![allow(clippy::useless_vec)]
3+
#[warn(clippy::range_zip_with_len)]
4+
fn main() {
5+
let v1: Vec<u64> = vec![1, 2, 3];
6+
let v2: Vec<u64> = vec![4, 5];
7+
8+
// Do not autofix, `filter()` would not consume the iterator.
9+
//~v range_zip_with_len
10+
v1.iter().zip(0..v1.len()).filter(|(_, i)| *i < 2).for_each(|(e, i)| {
11+
let _: &u64 = e;
12+
let _: usize = i;
13+
});
14+
}

tests/ui/range_unfixable.stderr

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
error: using `.zip()` with a range and `.len()`
2+
--> tests/ui/range_unfixable.rs:10:5
3+
|
4+
LL | v1.iter().zip(0..v1.len()).filter(|(_, i)| *i < 2).for_each(|(e, i)| {
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `v1.iter().enumerate()`
6+
|
7+
= note: the order of the element and the index will be swapped
8+
= note: `-D clippy::range-zip-with-len` implied by `-D warnings`
9+
= help: to override `-D warnings` add `#[allow(clippy::range_zip_with_len)]`
10+
11+
error: aborting due to 1 previous error
12+

0 commit comments

Comments
 (0)