diff options
| author | Dinu Blanovschi <git@dnbln.dev> | 2023-10-12 19:05:22 +0200 |
|---|---|---|
| committer | Dinu Blanovschi <git@dnbln.dev> | 2023-10-31 18:21:34 +0100 |
| commit | 14b82909b02b14c5b0c475bfbb5e507ddbd8f7c0 (patch) | |
| tree | 256ed6390b63ac8142f893573770f9c28e939923 | |
| parent | 0b90f72064cba3ced487773679aba48ae78af007 (diff) | |
| download | rust-14b82909b02b14c5b0c475bfbb5e507ddbd8f7c0.tar.gz rust-14b82909b02b14c5b0c475bfbb5e507ddbd8f7c0.zip | |
Apply suggestions from code review
Co-authored-by: Alejandra González <blyxyas@gmail.com>
| -rw-r--r-- | clippy_lints/src/loops/mod.rs | 10 | ||||
| -rw-r--r-- | clippy_lints/src/loops/unused_enumerate_index.rs | 37 | ||||
| -rw-r--r-- | tests/ui/unused_enumerate_index.fixed | 52 | ||||
| -rw-r--r-- | tests/ui/unused_enumerate_index.rs | 52 | ||||
| -rw-r--r-- | tests/ui/unused_enumerate_index.stderr | 15 |
5 files changed, 135 insertions, 31 deletions
diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs index c2b07920932..b99e0fd814f 100644 --- a/clippy_lints/src/loops/mod.rs +++ b/clippy_lints/src/loops/mod.rs @@ -580,7 +580,7 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for `for (_, v) in a.iter().enumerate()` + /// Checks for uses of the `enumerate` method where the index is unused (`_`) /// /// ### Why is this bad? /// The index from `.enumerate()` is immediately dropped. @@ -589,20 +589,20 @@ declare_clippy_lint! { /// ```rust /// let v = vec![1, 2, 3, 4]; /// for (_, x) in v.iter().enumerate() { - /// print!("{x}") + /// println!("{x}"); /// } /// ``` /// Use instead: /// ```rust /// let v = vec![1, 2, 3, 4]; /// for x in v.iter() { - /// print!("{x}") + /// println!("{x}"); /// } /// ``` - #[clippy::version = "1.69.0"] + #[clippy::version = "1.75.0"] pub UNUSED_ENUMERATE_INDEX, style, - "using .enumerate() and immediately dropping the index" + "using `.enumerate()` and immediately dropping the index" } declare_clippy_lint! { diff --git a/clippy_lints/src/loops/unused_enumerate_index.rs b/clippy_lints/src/loops/unused_enumerate_index.rs index 33c29499ee9..18b73f77e81 100644 --- a/clippy_lints/src/loops/unused_enumerate_index.rs +++ b/clippy_lints/src/loops/unused_enumerate_index.rs @@ -3,40 +3,27 @@ use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then}; use clippy_utils::source::snippet; use clippy_utils::sugg; use clippy_utils::visitors::is_local_used; +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 pat_span = pat.span; - - let PatKind::Tuple(pat, _) = pat.kind else { + let PatKind::Tuple(tuple, _) = pat.kind else { return; }; - if pat.len() != 2 { - return; - } - - let arg_span = arg.span; - - let ExprKind::MethodCall(method, self_arg, [], _) = arg.kind else { + let ExprKind::MethodCall(_method, self_arg, [], _) = arg.kind else { return; }; - if method.ident.as_str() != "enumerate" { - return; - } - let ty = cx.typeck_results().expr_ty(arg); - if !pat_is_wild(cx, &pat[0].kind, body) { + if !pat_is_wild(cx, &tuple[0].kind, body) { return; } - let new_pat_span = pat[1].span; - let name = match *ty.kind() { ty::Adt(base, _substs) => cx.tcx.def_path_str(base.did()), _ => return, @@ -46,10 +33,20 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, arg: &'tcx 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, + arg.span, "you seem to use `.enumerate()` and immediately discard the index", |diag| { let base_iter = sugg::Sugg::hir(cx, self_arg, "base iter"); @@ -57,8 +54,8 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, arg: &'tcx diag, "remove the `.enumerate()` call", vec![ - (pat_span, snippet(cx, new_pat_span, "value").into_owned()), - (arg_span, base_iter.to_string()), + (pat.span, snippet(cx, tuple[1].span, "..").into_owned()), + (arg.span, base_iter.to_string()), ], ); }, diff --git a/tests/ui/unused_enumerate_index.fixed b/tests/ui/unused_enumerate_index.fixed index 3a9f89063fd..d079807ab58 100644 --- a/tests/ui/unused_enumerate_index.fixed +++ b/tests/ui/unused_enumerate_index.fixed @@ -1,10 +1,58 @@ -// run-rustfix #![allow(unused)] #![warn(clippy::unused_enumerate_index)] +use std::iter::Enumerate; + fn main() { let v = [1, 2, 3]; for x in v.iter() { - print!("{x}"); + 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}"); } } diff --git a/tests/ui/unused_enumerate_index.rs b/tests/ui/unused_enumerate_index.rs index d047371f0b9..2d524da7632 100644 --- a/tests/ui/unused_enumerate_index.rs +++ b/tests/ui/unused_enumerate_index.rs @@ -1,10 +1,58 @@ -// run-rustfix #![allow(unused)] #![warn(clippy::unused_enumerate_index)] +use std::iter::Enumerate; + fn main() { let v = [1, 2, 3]; for (_, x) in v.iter().enumerate() { - print!("{x}"); + 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}"); } } diff --git a/tests/ui/unused_enumerate_index.stderr b/tests/ui/unused_enumerate_index.stderr index 7bd9e374151..b575fbbc4e6 100644 --- a/tests/ui/unused_enumerate_index.stderr +++ b/tests/ui/unused_enumerate_index.stderr @@ -1,5 +1,5 @@ error: you seem to use `.enumerate()` and immediately discard the index - --> $DIR/unused_enumerate_index.rs:7:19 + --> $DIR/unused_enumerate_index.rs:8:19 | LL | for (_, x) in v.iter().enumerate() { | ^^^^^^^^^^^^^^^^^^^^ @@ -11,5 +11,16 @@ help: remove the `.enumerate()` call LL | for x in v.iter() { | ~ ~~~~~~~~ -error: aborting due to previous error +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 |
