Skip to content

Commit 7cdeac5

Browse files
committed
[unused_enumerate_index]: trigger on method calls
The lint used to check for patterns looking like: ```rs for (_, x) in some_iter.enumerate() { // Index is ignored } ``` This commit further checks for chained method calls constructs where we can detect that the index is unused. Currently, this checks only for the following patterns: ```rs some_iter.enumerate().map_function(|(_, x)| ..) let x = some_iter.enumerate(); x.map_function(|(_, x)| ..) ``` where `map_function` is one of `all`, `any`, `filter_map`, `find_map`, `flat_map`, `for_each` or `map`. Fixes #12411.
1 parent 43f4ec3 commit 7cdeac5

File tree

7 files changed

+253
-31
lines changed

7 files changed

+253
-31
lines changed

clippy_lints/src/loops/unused_enumerate_index.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ use rustc_lint::LateContext;
88
use rustc_middle::ty;
99

1010
/// Checks for the `UNUSED_ENUMERATE_INDEX` lint.
11+
///
12+
/// The lint is also partially implemented in `clippy_lints/src/methods/unused_enumerate_index.rs`.
1113
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>, arg: &Expr<'_>, body: &'tcx Expr<'tcx>) {
1214
if let PatKind::Tuple([index, elem], _) = pat.kind
1315
&& let ExprKind::MethodCall(_method, self_arg, [], _) = arg.kind

clippy_lints/src/methods/mod.rs

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ mod unnecessary_literal_unwrap;
118118
mod unnecessary_result_map_or_else;
119119
mod unnecessary_sort_by;
120120
mod unnecessary_to_owned;
121+
mod unused_enumerate_index;
121122
mod unwrap_expect_used;
122123
mod useless_asref;
123124
mod utils;
@@ -4403,6 +4404,7 @@ impl Methods {
44034404
zst_offset::check(cx, expr, recv);
44044405
},
44054406
("all", [arg]) => {
4407+
unused_enumerate_index::check(cx, expr, recv, arg);
44064408
if let Some(("cloned", recv2, [], _, _)) = method_call(recv) {
44074409
iter_overeager_cloned::check(
44084410
cx,
@@ -4421,23 +4423,26 @@ impl Methods {
44214423
unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
44224424
}
44234425
},
4424-
("any", [arg]) => match method_call(recv) {
4425-
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(
4426-
cx,
4427-
expr,
4428-
recv,
4429-
recv2,
4430-
iter_overeager_cloned::Op::NeedlessMove(arg),
4431-
false,
4432-
),
4433-
Some(("chars", recv, _, _, _))
4434-
if let ExprKind::Closure(arg) = arg.kind
4435-
&& let body = cx.tcx.hir().body(arg.body)
4436-
&& let [param] = body.params =>
4437-
{
4438-
string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv);
4439-
},
4440-
_ => {},
4426+
("any", [arg]) => {
4427+
unused_enumerate_index::check(cx, expr, recv, arg);
4428+
match method_call(recv) {
4429+
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(
4430+
cx,
4431+
expr,
4432+
recv,
4433+
recv2,
4434+
iter_overeager_cloned::Op::NeedlessMove(arg),
4435+
false,
4436+
),
4437+
Some(("chars", recv, _, _, _))
4438+
if let ExprKind::Closure(arg) = arg.kind
4439+
&& let body = cx.tcx.hir().body(arg.body)
4440+
&& let [param] = body.params =>
4441+
{
4442+
string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv);
4443+
},
4444+
_ => {},
4445+
}
44414446
},
44424447
("arg", [arg]) => {
44434448
suspicious_command_arg_space::check(cx, recv, arg, span);
@@ -4570,14 +4575,17 @@ impl Methods {
45704575
}
45714576
},
45724577
("filter_map", [arg]) => {
4578+
unused_enumerate_index::check(cx, expr, recv, arg);
45734579
unnecessary_filter_map::check(cx, expr, arg, name);
45744580
filter_map_bool_then::check(cx, expr, arg, call_span);
45754581
filter_map_identity::check(cx, expr, arg, span);
45764582
},
45774583
("find_map", [arg]) => {
4584+
unused_enumerate_index::check(cx, expr, recv, arg);
45784585
unnecessary_filter_map::check(cx, expr, arg, name);
45794586
},
45804587
("flat_map", [arg]) => {
4588+
unused_enumerate_index::check(cx, expr, recv, arg);
45814589
flat_map_identity::check(cx, expr, arg, span);
45824590
flat_map_option::check(cx, expr, arg, span);
45834591
},
@@ -4599,17 +4607,20 @@ impl Methods {
45994607
manual_try_fold::check(cx, expr, init, acc, call_span, &self.msrv);
46004608
unnecessary_fold::check(cx, expr, init, acc, span);
46014609
},
4602-
("for_each", [arg]) => match method_call(recv) {
4603-
Some(("inspect", _, [_], span2, _)) => inspect_for_each::check(cx, expr, span2),
4604-
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(
4605-
cx,
4606-
expr,
4607-
recv,
4608-
recv2,
4609-
iter_overeager_cloned::Op::NeedlessMove(arg),
4610-
false,
4611-
),
4612-
_ => {},
4610+
("for_each", [arg]) => {
4611+
unused_enumerate_index::check(cx, expr, recv, arg);
4612+
match method_call(recv) {
4613+
Some(("inspect", _, [_], span2, _)) => inspect_for_each::check(cx, expr, span2),
4614+
Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(
4615+
cx,
4616+
expr,
4617+
recv,
4618+
recv2,
4619+
iter_overeager_cloned::Op::NeedlessMove(arg),
4620+
false,
4621+
),
4622+
_ => {},
4623+
}
46134624
},
46144625
("get", [arg]) => {
46154626
get_first::check(cx, expr, recv, arg);
@@ -4650,6 +4661,7 @@ impl Methods {
46504661
},
46514662
(name @ ("map" | "map_err"), [m_arg]) => {
46524663
if name == "map" {
4664+
unused_enumerate_index::check(cx, expr, recv, m_arg);
46534665
map_clone::check(cx, expr, recv, m_arg, &self.msrv);
46544666
match method_call(recv) {
46554667
Some((map_name @ ("iter" | "into_iter"), recv2, _, _, _)) => {
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
use clippy_utils::diagnostics::{multispan_sugg, span_lint_hir_and_then};
2+
use clippy_utils::paths::{CORE_ITER_ENUMERATE_METHOD, CORE_ITER_ENUMERATE_STRUCT};
3+
use clippy_utils::source::snippet;
4+
use clippy_utils::{expr_or_init, is_trait_method, match_def_path, pat_is_wild};
5+
use rustc_hir::{Expr, ExprKind, PatKind};
6+
use rustc_lint::LateContext;
7+
use rustc_middle::ty::AdtDef;
8+
use rustc_span::sym;
9+
10+
use crate::loops::UNUSED_ENUMERATE_INDEX;
11+
12+
/// Check for the `UNUSED_ENUMERATE_INDEX` lint outside of loops.
13+
///
14+
/// The lint is declared in `clippy_lints/src/loops/mod.rs`. There, the following pattern is
15+
/// checked:
16+
/// ```ignore
17+
/// for (_, x) in some_iter.enumerate() {
18+
/// // Index is ignored
19+
/// }
20+
/// ```
21+
///
22+
/// This `check` function checks for chained method calls constructs where we can detect that the
23+
/// index is unused. Currently, this checks only for the following patterns:
24+
/// ```ignore
25+
/// some_iter.enumerate().map_function(|(_, x)| ..)
26+
/// let x = some_iter.enumerate();
27+
/// x.map_function(|(_, x)| ..)
28+
/// ```
29+
/// where `map_function` is one of `all`, `any`, `filter_map`, `find_map`, `flat_map`, `for_each` or
30+
/// `map`.
31+
///
32+
/// # Preconditions
33+
/// This function must be called not on the `enumerate` call expression itself, but on any of the
34+
/// map functions listed above. It will ensure that `recv` is a `std::iter::Enumerate` instance and
35+
/// that the method call is one of the `std::iter::Iterator` trait.
36+
///
37+
/// * `call_expr`: The map function call expression
38+
/// * `recv`: The receiver of the call
39+
/// * `closure_arg`: The argument to the map function call containing the closure/function to apply
40+
pub(super) fn check(cx: &LateContext<'_>, call_expr: &Expr<'_>, recv: &Expr<'_>, closure_arg: &Expr<'_>) {
41+
let recv_ty = cx.typeck_results().expr_ty(recv);
42+
if let Some(recv_ty_defid) = recv_ty.ty_adt_def().map(AdtDef::did)
43+
// If we call a method on a `std::iter::Enumerate` instance
44+
&& match_def_path(cx, recv_ty_defid, &CORE_ITER_ENUMERATE_STRUCT)
45+
// If we are calling a method of the `Iterator` trait
46+
&& is_trait_method(cx, call_expr, sym::Iterator)
47+
// And the map argument is a closure
48+
&& let ExprKind::Closure(closure) = closure_arg.kind
49+
&& let closure_body = cx.tcx.hir().body(closure.body)
50+
// And that closure has one argument ...
51+
&& let [closure_param] = closure_body.params
52+
// .. which is a tuple of 2 elements
53+
&& let PatKind::Tuple([index, elem], ..) = closure_param.pat.kind
54+
// And that the first element (the index) is either `_` or unused in the body
55+
&& pat_is_wild(cx, &index.kind, closure_body)
56+
// Try to find the initializer for `recv`. This is needed in case `recv` is a local_binding. In the
57+
// first example below, `expr_or_init` would return `recv`.
58+
// ```
59+
// iter.enumerate().map(|(_, x)| x)
60+
// ^^^^^^^^^^^^^^^^ `recv`, a call to `std::iter::Iterator::enumerate`
61+
//
62+
// let binding = iter.enumerate();
63+
// ^^^^^^^^^^^^^^^^ `recv_init_expr`
64+
// binding.map(|(_, x)| x)
65+
// ^^^^^^^ `recv`, not a call to `std::iter::Iterator::enumerate`
66+
// ```
67+
&& let recv_init_expr = expr_or_init(cx, recv)
68+
// Make sure the initializer is a method call. It may be that the `Enumerate` comes from something
69+
// that we cannot control.
70+
// This would for instance happen with:
71+
// ```
72+
// external_lib::some_function_returning_enumerate().map(|(_, x)| x)
73+
// ```
74+
&& let ExprKind::MethodCall(_, enumerate_recv, _, enumerate_span) = recv_init_expr.kind
75+
&& let Some(enumerate_defid) = cx.typeck_results().type_dependent_def_id(recv_init_expr.hir_id)
76+
// Make sure the method call is `std::iter::Iterator::enumerate`.
77+
&& match_def_path(cx, enumerate_defid, &CORE_ITER_ENUMERATE_METHOD)
78+
{
79+
// Suggest removing the tuple from the closure and the preceding call to `enumerate`, whose span we
80+
// can get from the `MethodCall`.
81+
span_lint_hir_and_then(
82+
cx,
83+
UNUSED_ENUMERATE_INDEX,
84+
recv_init_expr.hir_id,
85+
enumerate_span,
86+
"you seem to use `.enumerate()` and immediately discard the index",
87+
|diag| {
88+
multispan_sugg(
89+
diag,
90+
"remove the `.enumerate()` call",
91+
vec![
92+
(closure_param.span, snippet(cx, elem.span, "..").into_owned()),
93+
(
94+
enumerate_span.with_lo(enumerate_recv.span.source_callsite().hi()),
95+
String::new(),
96+
),
97+
],
98+
);
99+
},
100+
);
101+
}
102+
}

clippy_utils/src/paths.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ pub const BTREESET_ITER: [&str; 6] = ["alloc", "collections", "btree", "set", "B
1919
pub const CLONE_TRAIT_METHOD: [&str; 4] = ["core", "clone", "Clone", "clone"];
2020
pub const CORE_ITER_CLONED: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "cloned"];
2121
pub const CORE_ITER_COPIED: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "copied"];
22+
pub const CORE_ITER_ENUMERATE_METHOD: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "enumerate"];
23+
pub const CORE_ITER_ENUMERATE_STRUCT: [&str; 5] = ["core", "iter", "adapters", "enumerate", "Enumerate"];
2224
pub const CORE_ITER_FILTER: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "filter"];
2325
pub const CORE_RESULT_OK_METHOD: [&str; 4] = ["core", "result", "Result", "ok"];
2426
pub const CSTRING_AS_C_STR: [&str; 5] = ["alloc", "ffi", "c_str", "CString", "as_c_str"];

tests/ui/unused_enumerate_index.fixed

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33

44
use std::iter::Enumerate;
55

6+
fn get_enumerate() -> Enumerate<std::vec::IntoIter<i32>> {
7+
vec![1].into_iter().enumerate()
8+
}
9+
610
fn main() {
711
let v = [1, 2, 3];
812
for x in v.iter() {
@@ -55,4 +59,34 @@ fn main() {
5559
for x in dummy {
5660
println!("{x}");
5761
}
62+
63+
let _ = vec![1, 2, 3].into_iter().map(|x| println!("{x}"));
64+
65+
let p = vec![1, 2, 3].into_iter();
66+
p.map(|x| println!("{x}"));
67+
68+
// This shouldn't trigger the lint. `get_enumerate` may come from an external library on which we
69+
// have no control.
70+
let p = get_enumerate();
71+
p.map(|(_, x)| println!("{x}"));
72+
73+
// This shouldn't trigger the lint. The `enumerate` call is in a different context.
74+
macro_rules! mac {
75+
() => {
76+
[1].iter().enumerate()
77+
};
78+
}
79+
_ = mac!().map(|(_, v)| v);
80+
81+
macro_rules! mac2 {
82+
() => {
83+
[1].iter()
84+
};
85+
}
86+
_ = mac2!().map(|_v| {});
87+
88+
// This shouldn't trigger the lint because of the `allow`.
89+
#[allow(clippy::unused_enumerate_index)]
90+
let v = [1].iter().enumerate();
91+
v.map(|(_, _x)| {});
5892
}

tests/ui/unused_enumerate_index.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33

44
use std::iter::Enumerate;
55

6+
fn get_enumerate() -> Enumerate<std::vec::IntoIter<i32>> {
7+
vec![1].into_iter().enumerate()
8+
}
9+
610
fn main() {
711
let v = [1, 2, 3];
812
for (_, x) in v.iter().enumerate() {
@@ -55,4 +59,34 @@ fn main() {
5559
for (_, x) in dummy.enumerate() {
5660
println!("{x}");
5761
}
62+
63+
let _ = vec![1, 2, 3].into_iter().enumerate().map(|(_, x)| println!("{x}"));
64+
65+
let p = vec![1, 2, 3].into_iter().enumerate();
66+
p.map(|(_, x)| println!("{x}"));
67+
68+
// This shouldn't trigger the lint. `get_enumerate` may come from an external library on which we
69+
// have no control.
70+
let p = get_enumerate();
71+
p.map(|(_, x)| println!("{x}"));
72+
73+
// This shouldn't trigger the lint. The `enumerate` call is in a different context.
74+
macro_rules! mac {
75+
() => {
76+
[1].iter().enumerate()
77+
};
78+
}
79+
_ = mac!().map(|(_, v)| v);
80+
81+
macro_rules! mac2 {
82+
() => {
83+
[1].iter()
84+
};
85+
}
86+
_ = mac2!().enumerate().map(|(_, _v)| {});
87+
88+
// This shouldn't trigger the lint because of the `allow`.
89+
#[allow(clippy::unused_enumerate_index)]
90+
let v = [1].iter().enumerate();
91+
v.map(|(_, _x)| {});
5892
}

tests/ui/unused_enumerate_index.stderr

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: you seem to use `.enumerate()` and immediately discard the index
2-
--> tests/ui/unused_enumerate_index.rs:8:19
2+
--> tests/ui/unused_enumerate_index.rs:12:19
33
|
44
LL | for (_, x) in v.iter().enumerate() {
55
| ^^^^^^^^^^^^^^^^^^^^
@@ -12,7 +12,7 @@ LL | for x in v.iter() {
1212
| ~ ~~~~~~~~
1313

1414
error: you seem to use `.enumerate()` and immediately discard the index
15-
--> tests/ui/unused_enumerate_index.rs:55:19
15+
--> tests/ui/unused_enumerate_index.rs:59:19
1616
|
1717
LL | for (_, x) in dummy.enumerate() {
1818
| ^^^^^^^^^^^^^^^^^
@@ -22,5 +22,41 @@ help: remove the `.enumerate()` call
2222
LL | for x in dummy {
2323
| ~ ~~~~~
2424

25-
error: aborting due to 2 previous errors
25+
error: you seem to use `.enumerate()` and immediately discard the index
26+
--> tests/ui/unused_enumerate_index.rs:63:39
27+
|
28+
LL | let _ = vec![1, 2, 3].into_iter().enumerate().map(|(_, x)| println!("{x}"));
29+
| ^^^^^^^^^^^
30+
|
31+
help: remove the `.enumerate()` call
32+
|
33+
LL - let _ = vec![1, 2, 3].into_iter().enumerate().map(|(_, x)| println!("{x}"));
34+
LL + let _ = vec![1, 2, 3].into_iter().map(|x| println!("{x}"));
35+
|
36+
37+
error: you seem to use `.enumerate()` and immediately discard the index
38+
--> tests/ui/unused_enumerate_index.rs:65:39
39+
|
40+
LL | let p = vec![1, 2, 3].into_iter().enumerate();
41+
| ^^^^^^^^^^^
42+
|
43+
help: remove the `.enumerate()` call
44+
|
45+
LL ~ let p = vec![1, 2, 3].into_iter();
46+
LL ~ p.map(|x| println!("{x}"));
47+
|
48+
49+
error: you seem to use `.enumerate()` and immediately discard the index
50+
--> tests/ui/unused_enumerate_index.rs:86:17
51+
|
52+
LL | _ = mac2!().enumerate().map(|(_, _v)| {});
53+
| ^^^^^^^^^^^
54+
|
55+
help: remove the `.enumerate()` call
56+
|
57+
LL - _ = mac2!().enumerate().map(|(_, _v)| {});
58+
LL + _ = mac2!().map(|_v| {});
59+
|
60+
61+
error: aborting due to 5 previous errors
2662

0 commit comments

Comments
 (0)