Skip to content

Commit dadcd94

Browse files
committed
[unused_enumerate_index]: Keep explicit element type
Prior to this change, it might be that the lint would remove an explicit type that was necessary for the type system to keep track of types. This should be the last change that prevented this lint to be machine applicable.
1 parent 7cdeac5 commit dadcd94

File tree

4 files changed

+106
-9
lines changed

4 files changed

+106
-9
lines changed

clippy_lints/src/methods/unused_enumerate_index.rs

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
use clippy_utils::diagnostics::{multispan_sugg, span_lint_hir_and_then};
1+
use clippy_utils::diagnostics::{multispan_sugg_with_applicability, span_lint_hir_and_then};
22
use clippy_utils::paths::{CORE_ITER_ENUMERATE_METHOD, CORE_ITER_ENUMERATE_STRUCT};
3-
use clippy_utils::source::snippet;
3+
use clippy_utils::source::{snippet, snippet_opt};
44
use clippy_utils::{expr_or_init, is_trait_method, match_def_path, pat_is_wild};
5-
use rustc_hir::{Expr, ExprKind, PatKind};
5+
use rustc_errors::Applicability;
6+
use rustc_hir::{Expr, ExprKind, FnDecl, PatKind, TyKind};
67
use rustc_lint::LateContext;
78
use rustc_middle::ty::AdtDef;
8-
use rustc_span::sym;
9+
use rustc_span::{sym, Span};
910

1011
use crate::loops::UNUSED_ENUMERATE_INDEX;
1112

@@ -76,6 +77,18 @@ pub(super) fn check(cx: &LateContext<'_>, call_expr: &Expr<'_>, recv: &Expr<'_>,
7677
// Make sure the method call is `std::iter::Iterator::enumerate`.
7778
&& match_def_path(cx, enumerate_defid, &CORE_ITER_ENUMERATE_METHOD)
7879
{
80+
// Check if the tuple type was explicit. It may be the type system _needs_ the type of the element
81+
// that would be explicited in the closure.
82+
let new_closure_param = match find_elem_explicit_type_span(closure.fn_decl) {
83+
// We have an explicit type. Get its snippet, that of the binding name, and do `binding: ty`.
84+
// Fallback to `..` if we fail getting either snippet.
85+
Some(ty_span) => snippet_opt(cx, elem.span)
86+
.and_then(|binding_name| snippet_opt(cx, ty_span).map(|ty_name| format!("{binding_name}: {ty_name}")))
87+
.unwrap_or_else(|| "..".to_string()),
88+
// Otherwise, we have no explicit type. We can replace with the binding name of the element.
89+
None => snippet(cx, elem.span, "..").into_owned(),
90+
};
91+
7992
// Suggest removing the tuple from the closure and the preceding call to `enumerate`, whose span we
8093
// can get from the `MethodCall`.
8194
span_lint_hir_and_then(
@@ -85,11 +98,12 @@ pub(super) fn check(cx: &LateContext<'_>, call_expr: &Expr<'_>, recv: &Expr<'_>,
8598
enumerate_span,
8699
"you seem to use `.enumerate()` and immediately discard the index",
87100
|diag| {
88-
multispan_sugg(
101+
multispan_sugg_with_applicability(
89102
diag,
90103
"remove the `.enumerate()` call",
104+
Applicability::MachineApplicable,
91105
vec![
92-
(closure_param.span, snippet(cx, elem.span, "..").into_owned()),
106+
(closure_param.span, new_closure_param),
93107
(
94108
enumerate_span.with_lo(enumerate_recv.span.source_callsite().hi()),
95109
String::new(),
@@ -100,3 +114,22 @@ pub(super) fn check(cx: &LateContext<'_>, call_expr: &Expr<'_>, recv: &Expr<'_>,
100114
);
101115
}
102116
}
117+
118+
/// Find the span of the explicit type of the element.
119+
///
120+
/// # Returns
121+
/// If the tuple argument:
122+
/// * Has no explicit type, returns `None`
123+
/// * Has an explicit tuple type with an implicit element type (`(usize, _)`), returns `None`
124+
/// * Has an explicit tuple type with an explicit element type (`(_, i32)`), returns the span for
125+
/// the element type.
126+
fn find_elem_explicit_type_span(fn_decl: &FnDecl<'_>) -> Option<Span> {
127+
if let [tuple_ty] = fn_decl.inputs
128+
&& let TyKind::Tup([_idx_ty, elem_ty]) = tuple_ty.kind
129+
&& !matches!(elem_ty.kind, TyKind::Err(..) | TyKind::Infer)
130+
{
131+
Some(elem_ty.span)
132+
} else {
133+
None
134+
}
135+
}

tests/ui/unused_enumerate_index.fixed

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#![allow(unused)]
1+
#![allow(unused, clippy::map_identity)]
22
#![warn(clippy::unused_enumerate_index)]
33

44
use std::iter::Enumerate;
@@ -89,4 +89,18 @@ fn main() {
8989
#[allow(clippy::unused_enumerate_index)]
9090
let v = [1].iter().enumerate();
9191
v.map(|(_, _x)| {});
92+
93+
// This should keep the explicit type of `x`.
94+
let v = [1, 2, 3].iter().copied();
95+
let x = v.map(|x: i32| x).sum::<i32>();
96+
assert_eq!(x, 6);
97+
98+
// This should keep the explicit type of `x`.
99+
let v = [1, 2, 3].iter().copied();
100+
let x = v.map(|x: i32| x).sum::<i32>();
101+
assert_eq!(x, 6);
102+
103+
let v = [1, 2, 3].iter().copied();
104+
let x = v.map(|x| x).sum::<i32>();
105+
assert_eq!(x, 6);
92106
}

tests/ui/unused_enumerate_index.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#![allow(unused)]
1+
#![allow(unused, clippy::map_identity)]
22
#![warn(clippy::unused_enumerate_index)]
33

44
use std::iter::Enumerate;
@@ -89,4 +89,18 @@ fn main() {
8989
#[allow(clippy::unused_enumerate_index)]
9090
let v = [1].iter().enumerate();
9191
v.map(|(_, _x)| {});
92+
93+
// This should keep the explicit type of `x`.
94+
let v = [1, 2, 3].iter().copied().enumerate();
95+
let x = v.map(|(_, x): (usize, i32)| x).sum::<i32>();
96+
assert_eq!(x, 6);
97+
98+
// This should keep the explicit type of `x`.
99+
let v = [1, 2, 3].iter().copied().enumerate();
100+
let x = v.map(|(_, x): (_, i32)| x).sum::<i32>();
101+
assert_eq!(x, 6);
102+
103+
let v = [1, 2, 3].iter().copied().enumerate();
104+
let x = v.map(|(_, x)| x).sum::<i32>();
105+
assert_eq!(x, 6);
92106
}

tests/ui/unused_enumerate_index.stderr

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,41 @@ LL - _ = mac2!().enumerate().map(|(_, _v)| {});
5858
LL + _ = mac2!().map(|_v| {});
5959
|
6060

61-
error: aborting due to 5 previous errors
61+
error: you seem to use `.enumerate()` and immediately discard the index
62+
--> tests/ui/unused_enumerate_index.rs:94:39
63+
|
64+
LL | let v = [1, 2, 3].iter().copied().enumerate();
65+
| ^^^^^^^^^^^
66+
|
67+
help: remove the `.enumerate()` call
68+
|
69+
LL ~ let v = [1, 2, 3].iter().copied();
70+
LL ~ let x = v.map(|x: i32| x).sum::<i32>();
71+
|
72+
73+
error: you seem to use `.enumerate()` and immediately discard the index
74+
--> tests/ui/unused_enumerate_index.rs:99:39
75+
|
76+
LL | let v = [1, 2, 3].iter().copied().enumerate();
77+
| ^^^^^^^^^^^
78+
|
79+
help: remove the `.enumerate()` call
80+
|
81+
LL ~ let v = [1, 2, 3].iter().copied();
82+
LL ~ let x = v.map(|x: i32| x).sum::<i32>();
83+
|
84+
85+
error: you seem to use `.enumerate()` and immediately discard the index
86+
--> tests/ui/unused_enumerate_index.rs:103:39
87+
|
88+
LL | let v = [1, 2, 3].iter().copied().enumerate();
89+
| ^^^^^^^^^^^
90+
|
91+
help: remove the `.enumerate()` call
92+
|
93+
LL ~ let v = [1, 2, 3].iter().copied();
94+
LL ~ let x = v.map(|x| x).sum::<i32>();
95+
|
96+
97+
error: aborting due to 8 previous errors
6298

0 commit comments

Comments
 (0)