about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-11-01 15:52:56 +0000
committerbors <bors@rust-lang.org>2023-11-01 15:52:56 +0000
commit919f698da0f5917d9f9620e510b69a81234249bc (patch)
tree41e1ffdccecb4e70cc4ff7b21b0aad043ccbafb2
parent7d34406015fce08031d0986dec0a4689a3cc407e (diff)
parentbb9cc6d47c11d33087d5776ecc261b95c6dace74 (diff)
downloadrust-919f698da0f5917d9f9620e510b69a81234249bc.tar.gz
rust-919f698da0f5917d9f9620e510b69a81234249bc.zip
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 -->
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/declared_lints.rs1
-rw-r--r--clippy_lints/src/loops/for_kv_map.rs12
-rw-r--r--clippy_lints/src/loops/mod.rs33
-rw-r--r--clippy_lints/src/loops/unused_enumerate_index.rs62
-rw-r--r--clippy_lints/src/methods/iter_kv_map.rs13
-rw-r--r--clippy_utils/src/lib.rs12
-rw-r--r--tests/ui/unused_enumerate_index.fixed58
-rw-r--r--tests/ui/unused_enumerate_index.rs58
-rw-r--r--tests/ui/unused_enumerate_index.stderr26
10 files changed, 252 insertions, 24 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 993406b692c..87a96bdeba6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5561,6 +5561,7 @@ Released 2018-09-13
 [`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_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_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
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index d9c97a8cc97..1a646ba38c3 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -274,6 +274,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::loops::NEVER_LOOP_INFO,
     crate::loops::SAME_ITEM_PUSH_INFO,
     crate::loops::SINGLE_ELEMENT_LOOP_INFO,
+    crate::loops::UNUSED_ENUMERATE_INDEX_INFO,
     crate::loops::WHILE_IMMUTABLE_CONDITION_INFO,
     crate::loops::WHILE_LET_LOOP_INFO,
     crate::loops::WHILE_LET_ON_ITERATOR_INFO,
diff --git a/clippy_lints/src/loops/for_kv_map.rs b/clippy_lints/src/loops/for_kv_map.rs
index ed620460dbe..94c951fc10a 100644
--- a/clippy_lints/src/loops/for_kv_map.rs
+++ b/clippy_lints/src/loops/for_kv_map.rs
@@ -1,9 +1,8 @@
 use super::FOR_KV_MAP;
 use clippy_utils::diagnostics::{multispan_sugg, span_lint_and_then};
 use clippy_utils::source::snippet;
-use clippy_utils::sugg;
 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_lint::LateContext;
 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,
-    }
-}
diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs
index 575a84c8c63..b99e0fd814f 100644
--- a/clippy_lints/src/loops/mod.rs
+++ b/clippy_lints/src/loops/mod.rs
@@ -14,6 +14,7 @@ mod needless_range_loop;
 mod never_loop;
 mod same_item_push;
 mod single_element_loop;
+mod unused_enumerate_index;
 mod utils;
 mod while_immutable_condition;
 mod while_let_loop;
@@ -579,6 +580,33 @@ declare_clippy_lint! {
 
 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! {
+    /// ### What it does
     /// Looks for loops that check for emptiness of a `Vec` in the condition and pop an element
     /// in the body as a separate operation.
     ///
@@ -619,6 +647,7 @@ impl Loops {
         }
     }
 }
+
 impl_lint_pass!(Loops => [
     MANUAL_MEMCPY,
     MANUAL_FLATTEN,
@@ -638,7 +667,8 @@ impl_lint_pass!(Loops => [
     SINGLE_ELEMENT_LOOP,
     MISSING_SPIN_LOOP,
     MANUAL_FIND,
-    MANUAL_WHILE_LET_SOME
+    MANUAL_WHILE_LET_SOME,
+    UNUSED_ENUMERATE_INDEX,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Loops {
@@ -717,6 +747,7 @@ impl Loops {
         same_item_push::check(cx, pat, arg, body, expr);
         manual_flatten::check(cx, pat, arg, body, span);
         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<'_>) {
diff --git a/clippy_lints/src/loops/unused_enumerate_index.rs b/clippy_lints/src/loops/unused_enumerate_index.rs
new file mode 100644
index 00000000000..62a2ab1ccb4
--- /dev/null
+++ b/clippy_lints/src/loops/unused_enumerate_index.rs
@@ -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()),
+                ],
+            );
+        },
+    );
+}
diff --git a/clippy_lints/src/methods/iter_kv_map.rs b/clippy_lints/src/methods/iter_kv_map.rs
index b44a2716dde..625325d4cf5 100644
--- a/clippy_lints/src/methods/iter_kv_map.rs
+++ b/clippy_lints/src/methods/iter_kv_map.rs
@@ -3,9 +3,8 @@
 use super::ITER_KV_MAP;
 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::sugg;
 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_lint::{LateContext, LintContext};
 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,
-    }
-}
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index 11467138a58..7497d4b2cf1 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -2960,3 +2960,15 @@ op_utils! {
     Shl    ShlAssign
     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,
+    }
+}
diff --git a/tests/ui/unused_enumerate_index.fixed b/tests/ui/unused_enumerate_index.fixed
new file mode 100644
index 00000000000..d079807ab58
--- /dev/null
+++ b/tests/ui/unused_enumerate_index.fixed
@@ -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}");
+    }
+}
diff --git a/tests/ui/unused_enumerate_index.rs b/tests/ui/unused_enumerate_index.rs
new file mode 100644
index 00000000000..2d524da7632
--- /dev/null
+++ b/tests/ui/unused_enumerate_index.rs
@@ -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}");
+    }
+}
diff --git a/tests/ui/unused_enumerate_index.stderr b/tests/ui/unused_enumerate_index.stderr
new file mode 100644
index 00000000000..b575fbbc4e6
--- /dev/null
+++ b/tests/ui/unused_enumerate_index.stderr
@@ -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
+