Skip to content

Commit bd22913

Browse files
committed
unused_enumerate_index: move to loops lint pass
Move `unused_enumerate_index.rs` to `methods`.
1 parent 0226fa9 commit bd22913

File tree

4 files changed

+157
-167
lines changed

4 files changed

+157
-167
lines changed

clippy_lints/src/loops/mod.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -880,6 +880,16 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
880880
missing_spin_loop::check(cx, condition, body);
881881
manual_while_let_some::check(cx, condition, body, span);
882882
}
883+
884+
if let ExprKind::MethodCall(path, recv, [arg], _) = expr.kind
885+
&& matches!(
886+
path.ident.name,
887+
sym::all | sym::any | sym::filter_map | sym::find_map | sym::flat_map | sym::for_each | sym::map
888+
)
889+
&& !recv.span.from_expansion()
890+
{
891+
unused_enumerate_index::check_method(cx, expr, recv, arg);
892+
}
883893
}
884894
}
885895

@@ -908,7 +918,7 @@ impl Loops {
908918
same_item_push::check(cx, pat, arg, body, expr, self.msrv);
909919
manual_flatten::check(cx, pat, arg, body, span, self.msrv);
910920
manual_find::check(cx, pat, arg, body, span, expr);
911-
unused_enumerate_index::check(cx, pat, arg, body);
921+
unused_enumerate_index::check_loop(cx, pat, arg, body);
912922
char_indices_as_byte_indices::check(cx, pat, arg, body);
913923
}
914924

clippy_lints/src/loops/unused_enumerate_index.rs

Lines changed: 135 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
use super::UNUSED_ENUMERATE_INDEX;
2-
use clippy_utils::diagnostics::span_lint_and_then;
3-
use clippy_utils::res::MaybeDef;
4-
use clippy_utils::source::snippet;
5-
use clippy_utils::{pat_is_wild, sugg};
2+
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
3+
use clippy_utils::res::{MaybeDef, MaybeTypeckRes};
4+
use clippy_utils::source::{SpanRangeExt, snippet};
5+
use clippy_utils::{expr_or_init, pat_is_wild, sugg};
66
use rustc_errors::Applicability;
77
use rustc_hir::def::DefKind;
8-
use rustc_hir::{Expr, ExprKind, Pat, PatKind};
8+
use rustc_hir::{Expr, ExprKind, FnDecl, Pat, PatKind, TyKind};
99
use rustc_lint::LateContext;
10-
use rustc_span::sym;
10+
use rustc_span::{Span, sym};
1111

1212
/// Checks for the `UNUSED_ENUMERATE_INDEX` lint.
1313
///
1414
/// The lint is also partially implemented in `clippy_lints/src/methods/unused_enumerate_index.rs`.
15-
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>, arg: &Expr<'_>, body: &'tcx Expr<'tcx>) {
15+
pub(super) fn check_loop<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>, arg: &Expr<'_>, body: &'tcx Expr<'tcx>) {
1616
if let PatKind::Tuple([index, elem], _) = pat.kind
1717
&& let ExprKind::MethodCall(_method, self_arg, [], _) = arg.kind
1818
&& let ty = cx.typeck_results().expr_ty(arg)
@@ -40,3 +40,131 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>, arg: &Expr<'_
4040
);
4141
}
4242
}
43+
44+
/// Check for the `UNUSED_ENUMERATE_INDEX` lint outside of loops.
45+
///
46+
/// The lint is declared in `clippy_lints/src/loops/mod.rs`. There, the following pattern is
47+
/// checked:
48+
/// ```ignore
49+
/// for (_, x) in some_iter.enumerate() {
50+
/// // Index is ignored
51+
/// }
52+
/// ```
53+
///
54+
/// This `check` function checks for chained method calls constructs where we can detect that the
55+
/// index is unused. Currently, this checks only for the following patterns:
56+
/// ```ignore
57+
/// some_iter.enumerate().map_function(|(_, x)| ..)
58+
/// let x = some_iter.enumerate();
59+
/// x.map_function(|(_, x)| ..)
60+
/// ```
61+
/// where `map_function` is one of `all`, `any`, `filter_map`, `find_map`, `flat_map`, `for_each` or
62+
/// `map`.
63+
///
64+
/// # Preconditions
65+
/// This function must be called not on the `enumerate` call expression itself, but on any of the
66+
/// map functions listed above. It will ensure that `recv` is a `std::iter::Enumerate` instance and
67+
/// that the method call is one of the `std::iter::Iterator` trait.
68+
///
69+
/// * `call_expr`: The map function call expression
70+
/// * `recv`: The receiver of the call
71+
/// * `closure_arg`: The argument to the map function call containing the closure/function to apply
72+
pub(super) fn check_method(cx: &LateContext<'_>, call_expr: &Expr<'_>, recv: &Expr<'_>, closure_arg: &Expr<'_>) {
73+
let recv_ty = cx.typeck_results().expr_ty(recv);
74+
// If we call a method on a `std::iter::Enumerate` instance
75+
if recv_ty.is_diag_item(cx, sym::Enumerate)
76+
// If we are calling a method of the `Iterator` trait
77+
&& cx.ty_based_def(call_expr).opt_parent(cx).is_diag_item(cx, sym::Iterator)
78+
// And the map argument is a closure
79+
&& let ExprKind::Closure(closure) = closure_arg.kind
80+
&& let closure_body = cx.tcx.hir_body(closure.body)
81+
// And that closure has one argument ...
82+
&& let [closure_param] = closure_body.params
83+
// .. which is a tuple of 2 elements
84+
&& let PatKind::Tuple([index, elem], ..) = closure_param.pat.kind
85+
// And that the first element (the index) is either `_` or unused in the body
86+
&& pat_is_wild(cx, &index.kind, closure_body)
87+
// Try to find the initializer for `recv`. This is needed in case `recv` is a local_binding. In the
88+
// first example below, `expr_or_init` would return `recv`.
89+
// ```
90+
// iter.enumerate().map(|(_, x)| x)
91+
// ^^^^^^^^^^^^^^^^ `recv`, a call to `std::iter::Iterator::enumerate`
92+
//
93+
// let binding = iter.enumerate();
94+
// ^^^^^^^^^^^^^^^^ `recv_init_expr`
95+
// binding.map(|(_, x)| x)
96+
// ^^^^^^^ `recv`, not a call to `std::iter::Iterator::enumerate`
97+
// ```
98+
&& let recv_init_expr = expr_or_init(cx, recv)
99+
// Make sure the initializer is a method call. It may be that the `Enumerate` comes from something
100+
// that we cannot control.
101+
// This would for instance happen with:
102+
// ```
103+
// external_lib::some_function_returning_enumerate().map(|(_, x)| x)
104+
// ```
105+
&& let ExprKind::MethodCall(_, enumerate_recv, _, enumerate_span) = recv_init_expr.kind
106+
&& let Some(enumerate_defid) = cx.typeck_results().type_dependent_def_id(recv_init_expr.hir_id)
107+
// Make sure the method call is `std::iter::Iterator::enumerate`.
108+
&& cx.tcx.is_diagnostic_item(sym::enumerate_method, enumerate_defid)
109+
{
110+
// Check if the tuple type was explicit. It may be the type system _needs_ the type of the element
111+
// that would be explicitly in the closure.
112+
let new_closure_param = match find_elem_explicit_type_span(closure.fn_decl) {
113+
// We have an explicit type. Get its snippet, that of the binding name, and do `binding: ty`.
114+
// Fallback to `..` if we fail getting either snippet.
115+
Some(ty_span) => elem
116+
.span
117+
.get_source_text(cx)
118+
.and_then(|binding_name| {
119+
ty_span
120+
.get_source_text(cx)
121+
.map(|ty_name| format!("{binding_name}: {ty_name}"))
122+
})
123+
.unwrap_or_else(|| "..".to_string()),
124+
// Otherwise, we have no explicit type. We can replace with the binding name of the element.
125+
None => snippet(cx, elem.span, "..").into_owned(),
126+
};
127+
128+
// Suggest removing the tuple from the closure and the preceding call to `enumerate`, whose span we
129+
// can get from the `MethodCall`.
130+
span_lint_hir_and_then(
131+
cx,
132+
UNUSED_ENUMERATE_INDEX,
133+
recv_init_expr.hir_id,
134+
enumerate_span,
135+
"you seem to use `.enumerate()` and immediately discard the index",
136+
|diag| {
137+
diag.multipart_suggestion(
138+
"remove the `.enumerate()` call",
139+
vec![
140+
(closure_param.span, new_closure_param),
141+
(
142+
enumerate_span.with_lo(enumerate_recv.span.source_callsite().hi()),
143+
String::new(),
144+
),
145+
],
146+
Applicability::MachineApplicable,
147+
);
148+
},
149+
);
150+
}
151+
}
152+
153+
/// Find the span of the explicit type of the element.
154+
///
155+
/// # Returns
156+
/// If the tuple argument:
157+
/// * Has no explicit type, returns `None`
158+
/// * Has an explicit tuple type with an implicit element type (`(usize, _)`), returns `None`
159+
/// * Has an explicit tuple type with an explicit element type (`(_, i32)`), returns the span for
160+
/// the element type.
161+
fn find_elem_explicit_type_span(fn_decl: &FnDecl<'_>) -> Option<Span> {
162+
if let [tuple_ty] = fn_decl.inputs
163+
&& let TyKind::Tup([_idx_ty, elem_ty]) = tuple_ty.kind
164+
&& !matches!(elem_ty.kind, TyKind::Err(..) | TyKind::Infer(()))
165+
{
166+
Some(elem_ty.span)
167+
} else {
168+
None
169+
}
170+
}

clippy_lints/src/methods/mod.rs

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,6 @@ mod unnecessary_option_map_or_else;
138138
mod unnecessary_result_map_or_else;
139139
mod unnecessary_sort_by;
140140
mod unnecessary_to_owned;
141-
mod unused_enumerate_index;
142141
mod unwrap_expect_used;
143142
mod useless_asref;
144143
mod useless_nonzero_new_unchecked;
@@ -5026,7 +5025,6 @@ impl Methods {
50265025
zst_offset::check(cx, expr, recv);
50275026
},
50285027
(sym::all, [arg]) => {
5029-
unused_enumerate_index::check(cx, expr, recv, arg);
50305028
needless_character_iteration::check(cx, expr, recv, arg, true);
50315029
match method_call(recv) {
50325030
Some((sym::cloned, recv2, [], _, _)) => {
@@ -5056,7 +5054,6 @@ impl Methods {
50565054
}
50575055
},
50585056
(sym::any, [arg]) => {
5059-
unused_enumerate_index::check(cx, expr, recv, arg);
50605057
needless_character_iteration::check(cx, expr, recv, arg, false);
50615058
match method_call(recv) {
50625059
Some((sym::cloned, recv2, [], _, _)) => iter_overeager_cloned::check(
@@ -5216,7 +5213,6 @@ impl Methods {
52165213
}
52175214
},
52185215
(sym::filter_map, [arg]) => {
5219-
unused_enumerate_index::check(cx, expr, recv, arg);
52205216
unnecessary_filter_map::check(cx, expr, arg, call_span, unnecessary_filter_map::Kind::FilterMap);
52215217
filter_map_bool_then::check(cx, expr, arg, call_span);
52225218
filter_map_identity::check(cx, expr, arg, span);
@@ -5231,11 +5227,9 @@ impl Methods {
52315227
);
52325228
},
52335229
(sym::find_map, [arg]) => {
5234-
unused_enumerate_index::check(cx, expr, recv, arg);
52355230
unnecessary_filter_map::check(cx, expr, arg, call_span, unnecessary_filter_map::Kind::FindMap);
52365231
},
52375232
(sym::flat_map, [arg]) => {
5238-
unused_enumerate_index::check(cx, expr, recv, arg);
52395233
flat_map_identity::check(cx, expr, arg, span);
52405234
flat_map_option::check(cx, expr, arg, span);
52415235
lines_filter_map_ok::check_filter_or_flat_map(
@@ -5263,20 +5257,17 @@ impl Methods {
52635257
manual_try_fold::check(cx, expr, init, acc, call_span, self.msrv);
52645258
unnecessary_fold::check(cx, expr, init, acc, span);
52655259
},
5266-
(sym::for_each, [arg]) => {
5267-
unused_enumerate_index::check(cx, expr, recv, arg);
5268-
match method_call(recv) {
5269-
Some((sym::inspect, _, [_], span2, _)) => inspect_for_each::check(cx, expr, span2),
5270-
Some((sym::cloned, recv2, [], _, _)) => iter_overeager_cloned::check(
5271-
cx,
5272-
expr,
5273-
recv,
5274-
recv2,
5275-
iter_overeager_cloned::Op::NeedlessMove(arg),
5276-
false,
5277-
),
5278-
_ => {},
5279-
}
5260+
(sym::for_each, [arg]) => match method_call(recv) {
5261+
Some((sym::inspect, _, [_], span2, _)) => inspect_for_each::check(cx, expr, span2),
5262+
Some((sym::cloned, recv2, [], _, _)) => iter_overeager_cloned::check(
5263+
cx,
5264+
expr,
5265+
recv,
5266+
recv2,
5267+
iter_overeager_cloned::Op::NeedlessMove(arg),
5268+
false,
5269+
),
5270+
_ => {},
52805271
},
52815272
(sym::get, [arg]) => {
52825273
get_first::check(cx, expr, recv, arg);
@@ -5337,7 +5328,6 @@ impl Methods {
53375328
},
53385329
(name @ (sym::map | sym::map_err), [m_arg]) => {
53395330
if name == sym::map {
5340-
unused_enumerate_index::check(cx, expr, recv, m_arg);
53415331
map_clone::check(cx, expr, recv, m_arg, self.msrv);
53425332
map_with_unused_argument_over_ranges::check(cx, expr, recv, m_arg, self.msrv, span);
53435333
manual_is_variant_and::check_map(cx, expr);

0 commit comments

Comments
 (0)