Auto merge of #10404 - dnbln:feat/unused_enumerate_index, r=blyxyas

Add `unused_enumerate_index` lint

A lint for unused `.enumerate()` indexes (`for (_, x) in iter.enumerate()`).

I wasn't able to find a `rustc_span::sym::Enumerate`, so the code for checking that it's the correct `Enumerate` iterator is a bit weird.

---

changelog: New lint: [`unused_enumerate_index`]: A new lint for checking that the indexes from `.enumerate()` calls are used.
[#10404](https://github.com/rust-lang/rust-clippy/pull/10404)
<!-- changelog_checked -->
This commit is contained in:
bors 2023-11-01 15:52:56 +00:00
commit 919f698da0
10 changed files with 252 additions and 24 deletions

View File

@ -5561,6 +5561,7 @@ Released 2018-09-13
[`unstable_as_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#unstable_as_slice [`unstable_as_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#unstable_as_slice
[`unused_async`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_async [`unused_async`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_async
[`unused_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_collect [`unused_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_collect
[`unused_enumerate_index`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_enumerate_index
[`unused_format_specs`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_format_specs [`unused_format_specs`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_format_specs
[`unused_io_amount`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_io_amount [`unused_io_amount`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_io_amount
[`unused_label`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_label [`unused_label`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_label

View File

@ -274,6 +274,7 @@
crate::loops::NEVER_LOOP_INFO, crate::loops::NEVER_LOOP_INFO,
crate::loops::SAME_ITEM_PUSH_INFO, crate::loops::SAME_ITEM_PUSH_INFO,
crate::loops::SINGLE_ELEMENT_LOOP_INFO, crate::loops::SINGLE_ELEMENT_LOOP_INFO,
crate::loops::UNUSED_ENUMERATE_INDEX_INFO,
crate::loops::WHILE_IMMUTABLE_CONDITION_INFO, crate::loops::WHILE_IMMUTABLE_CONDITION_INFO,
crate::loops::WHILE_LET_LOOP_INFO, crate::loops::WHILE_LET_LOOP_INFO,
crate::loops::WHILE_LET_ON_ITERATOR_INFO, crate::loops::WHILE_LET_ON_ITERATOR_INFO,

View File

@ -1,9 +1,8 @@
use super::FOR_KV_MAP; use super::FOR_KV_MAP;
use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then}; use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then};
use clippy_utils::source::snippet; use clippy_utils::source::snippet;
use clippy_utils::sugg;
use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::visitors::is_local_used; use clippy_utils::{pat_is_wild, sugg};
use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, Pat, PatKind}; use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, Pat, PatKind};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::ty; use rustc_middle::ty;
@ -55,12 +54,3 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, arg: &'tcx
} }
} }
} }
/// Returns `true` if the pattern is a `PatWild` or an ident prefixed with `_`.
fn pat_is_wild<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool {
match *pat {
PatKind::Wild => true,
PatKind::Binding(_, id, ident, None) if ident.as_str().starts_with('_') => !is_local_used(cx, body, id),
_ => false,
}
}

View File

@ -14,6 +14,7 @@
mod never_loop; mod never_loop;
mod same_item_push; mod same_item_push;
mod single_element_loop; mod single_element_loop;
mod unused_enumerate_index;
mod utils; mod utils;
mod while_immutable_condition; mod while_immutable_condition;
mod while_let_loop; mod while_let_loop;
@ -577,6 +578,33 @@
"manual implementation of `Iterator::find`" "manual implementation of `Iterator::find`"
} }
declare_clippy_lint! {
/// ### What it does
/// Checks for uses of the `enumerate` method where the index is unused (`_`)
///
/// ### Why is this bad?
/// The index from `.enumerate()` is immediately dropped.
///
/// ### Example
/// ```rust
/// let v = vec![1, 2, 3, 4];
/// for (_, x) in v.iter().enumerate() {
/// println!("{x}");
/// }
/// ```
/// Use instead:
/// ```rust
/// let v = vec![1, 2, 3, 4];
/// for x in v.iter() {
/// println!("{x}");
/// }
/// ```
#[clippy::version = "1.75.0"]
pub UNUSED_ENUMERATE_INDEX,
style,
"using `.enumerate()` and immediately dropping the index"
}
declare_clippy_lint! { declare_clippy_lint! {
/// ### What it does /// ### What it does
/// Looks for loops that check for emptiness of a `Vec` in the condition and pop an element /// Looks for loops that check for emptiness of a `Vec` in the condition and pop an element
@ -619,6 +647,7 @@ pub fn new(msrv: Msrv, enforce_iter_loop_reborrow: bool) -> Self {
} }
} }
} }
impl_lint_pass!(Loops => [ impl_lint_pass!(Loops => [
MANUAL_MEMCPY, MANUAL_MEMCPY,
MANUAL_FLATTEN, MANUAL_FLATTEN,
@ -638,7 +667,8 @@ pub fn new(msrv: Msrv, enforce_iter_loop_reborrow: bool) -> Self {
SINGLE_ELEMENT_LOOP, SINGLE_ELEMENT_LOOP,
MISSING_SPIN_LOOP, MISSING_SPIN_LOOP,
MANUAL_FIND, MANUAL_FIND,
MANUAL_WHILE_LET_SOME MANUAL_WHILE_LET_SOME,
UNUSED_ENUMERATE_INDEX,
]); ]);
impl<'tcx> LateLintPass<'tcx> for Loops { impl<'tcx> LateLintPass<'tcx> for Loops {
@ -717,6 +747,7 @@ fn check_for_loop<'tcx>(
same_item_push::check(cx, pat, arg, body, expr); same_item_push::check(cx, pat, arg, body, expr);
manual_flatten::check(cx, pat, arg, body, span); manual_flatten::check(cx, pat, arg, body, span);
manual_find::check(cx, pat, arg, body, span, expr); manual_find::check(cx, pat, arg, body, span, expr);
unused_enumerate_index::check(cx, pat, arg, body);
} }
fn check_for_loop_arg(&self, cx: &LateContext<'_>, _: &Pat<'_>, arg: &Expr<'_>) { fn check_for_loop_arg(&self, cx: &LateContext<'_>, _: &Pat<'_>, arg: &Expr<'_>) {

View File

@ -0,0 +1,62 @@
use super::UNUSED_ENUMERATE_INDEX;
use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then};
use clippy_utils::source::snippet;
use clippy_utils::{pat_is_wild, sugg};
use rustc_hir::def::DefKind;
use rustc_hir::{Expr, ExprKind, Pat, PatKind};
use rustc_lint::LateContext;
use rustc_middle::ty;
/// Checks for the `UNUSED_ENUMERATE_INDEX` lint.
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, arg: &'tcx Expr<'_>, body: &'tcx Expr<'_>) {
let PatKind::Tuple(tuple, _) = pat.kind else {
return;
};
let ExprKind::MethodCall(_method, self_arg, [], _) = arg.kind else {
return;
};
let ty = cx.typeck_results().expr_ty(arg);
if !pat_is_wild(cx, &tuple[0].kind, body) {
return;
}
let name = match *ty.kind() {
ty::Adt(base, _substs) => cx.tcx.def_path_str(base.did()),
_ => return,
};
if name != "std::iter::Enumerate" && name != "core::iter::Enumerate" {
return;
}
let Some((DefKind::AssocFn, call_id)) = cx.typeck_results().type_dependent_def(arg.hir_id) else {
return;
};
let call_name = cx.tcx.def_path_str(call_id);
if call_name != "std::iter::Iterator::enumerate" && call_name != "core::iter::Iterator::enumerate" {
return;
}
span_lint_and_then(
cx,
UNUSED_ENUMERATE_INDEX,
arg.span,
"you seem to use `.enumerate()` and immediately discard the index",
|diag| {
let base_iter = sugg::Sugg::hir(cx, self_arg, "base iter");
multispan_sugg(
diag,
"remove the `.enumerate()` call",
vec![
(pat.span, snippet(cx, tuple[1].span, "..").into_owned()),
(arg.span, base_iter.to_string()),
],
);
},
);
}

View File

@ -3,9 +3,8 @@
use super::ITER_KV_MAP; use super::ITER_KV_MAP;
use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_sugg, span_lint_and_then};
use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::source::{snippet, snippet_with_applicability};
use clippy_utils::sugg;
use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::visitors::is_local_used; use clippy_utils::{pat_is_wild, sugg};
use rustc_hir::{BindingAnnotation, Body, BorrowKind, ByRef, Expr, ExprKind, Mutability, Pat, PatKind}; use rustc_hir::{BindingAnnotation, Body, BorrowKind, ByRef, Expr, ExprKind, Mutability, Pat, PatKind};
use rustc_lint::{LateContext, LintContext}; use rustc_lint::{LateContext, LintContext};
use rustc_middle::ty; use rustc_middle::ty;
@ -84,13 +83,3 @@ pub(super) fn check<'tcx>(
} }
} }
} }
/// Returns `true` if the pattern is a `PatWild`, or is an ident prefixed with `_`
/// that is not locally used.
fn pat_is_wild<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool {
match *pat {
PatKind::Wild => true,
PatKind::Binding(_, id, ident, None) if ident.as_str().starts_with('_') => !is_local_used(cx, body, id),
_ => false,
}
}

View File

@ -2960,3 +2960,15 @@ pub fn binop_traits(kind: hir::BinOpKind) -> Option<(LangItem, LangItem)> {
Shl ShlAssign Shl ShlAssign
Shr ShrAssign Shr ShrAssign
} }
/// Returns `true` if the pattern is a `PatWild`, or is an ident prefixed with `_`
/// that is not locally used.
pub fn pat_is_wild<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx PatKind<'_>, body: impl Visitable<'tcx>) -> bool {
match *pat {
PatKind::Wild => true,
PatKind::Binding(_, id, ident, None) if ident.as_str().starts_with('_') => {
!visitors::is_local_used(cx, body, id)
},
_ => false,
}
}

View File

@ -0,0 +1,58 @@
#![allow(unused)]
#![warn(clippy::unused_enumerate_index)]
use std::iter::Enumerate;
fn main() {
let v = [1, 2, 3];
for x in v.iter() {
println!("{x}");
}
struct Dummy1;
impl Dummy1 {
fn enumerate(self) -> Vec<usize> {
vec![]
}
}
let dummy = Dummy1;
for x in dummy.enumerate() {
println!("{x}");
}
struct Dummy2;
impl Dummy2 {
fn enumerate(self) -> Enumerate<std::vec::IntoIter<usize>> {
vec![1, 2].into_iter().enumerate()
}
}
let dummy = Dummy2;
for (_, x) in dummy.enumerate() {
println!("{x}");
}
let mut with_used_iterator = [1, 2, 3].into_iter().enumerate();
with_used_iterator.next();
for (_, x) in with_used_iterator {
println!("{x}");
}
struct Dummy3(std::vec::IntoIter<usize>);
impl Iterator for Dummy3 {
type Item = usize;
fn next(&mut self) -> Option<Self::Item> {
self.0.next()
}
fn size_hint(&self) -> (usize, Option<usize>) {
self.0.size_hint()
}
}
let dummy = Dummy3(vec![1, 2, 3].into_iter());
for x in dummy {
println!("{x}");
}
}

View File

@ -0,0 +1,58 @@
#![allow(unused)]
#![warn(clippy::unused_enumerate_index)]
use std::iter::Enumerate;
fn main() {
let v = [1, 2, 3];
for (_, x) in v.iter().enumerate() {
println!("{x}");
}
struct Dummy1;
impl Dummy1 {
fn enumerate(self) -> Vec<usize> {
vec![]
}
}
let dummy = Dummy1;
for x in dummy.enumerate() {
println!("{x}");
}
struct Dummy2;
impl Dummy2 {
fn enumerate(self) -> Enumerate<std::vec::IntoIter<usize>> {
vec![1, 2].into_iter().enumerate()
}
}
let dummy = Dummy2;
for (_, x) in dummy.enumerate() {
println!("{x}");
}
let mut with_used_iterator = [1, 2, 3].into_iter().enumerate();
with_used_iterator.next();
for (_, x) in with_used_iterator {
println!("{x}");
}
struct Dummy3(std::vec::IntoIter<usize>);
impl Iterator for Dummy3 {
type Item = usize;
fn next(&mut self) -> Option<Self::Item> {
self.0.next()
}
fn size_hint(&self) -> (usize, Option<usize>) {
self.0.size_hint()
}
}
let dummy = Dummy3(vec![1, 2, 3].into_iter());
for (_, x) in dummy.enumerate() {
println!("{x}");
}
}

View File

@ -0,0 +1,26 @@
error: you seem to use `.enumerate()` and immediately discard the index
--> $DIR/unused_enumerate_index.rs:8:19
|
LL | for (_, x) in v.iter().enumerate() {
| ^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::unused-enumerate-index` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unused_enumerate_index)]`
help: remove the `.enumerate()` call
|
LL | for x in v.iter() {
| ~ ~~~~~~~~
error: you seem to use `.enumerate()` and immediately discard the index
--> $DIR/unused_enumerate_index.rs:55:19
|
LL | for (_, x) in dummy.enumerate() {
| ^^^^^^^^^^^^^^^^^
|
help: remove the `.enumerate()` call
|
LL | for x in dummy {
| ~ ~~~~~
error: aborting due to 2 previous errors