New lint: unused_enumerate_value

CHANGELOG.md

@@ -6259,6 +6259,7 @@ Released 2018-09-13
 [`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_enumerate_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_enumerate_value
 [`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

clippy_lints/src/declared_lints.rs

@@ -308,6 +308,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::loops::SAME_ITEM_PUSH_INFO,
     crate::loops::SINGLE_ELEMENT_LOOP_INFO,
     crate::loops::UNUSED_ENUMERATE_INDEX_INFO,
+    crate::loops::UNUSED_ENUMERATE_VALUE_INFO,
     crate::loops::WHILE_FLOAT_INFO,
     crate::loops::WHILE_IMMUTABLE_CONDITION_INFO,
     crate::loops::WHILE_LET_LOOP_INFO,

clippy_lints/src/loops/mod.rs

@@ -18,6 +18,7 @@ mod never_loop;
 mod same_item_push;
 mod single_element_loop;
 mod unused_enumerate_index;
+mod unused_enumerate_value;
 mod utils;
 mod while_float;
 mod while_immutable_condition;
@@ -784,6 +785,37 @@ declare_clippy_lint! {
     "using the character position yielded by `.chars().enumerate()` in a context where a byte index is expected"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for uses of the `enumerate` method where the value is unused (`_`) on iterators
+    /// implementing `ExactSizeIterator`.
+    ///
+    /// ### Why is this bad?
+    /// Just iterating a range of indices is more idiomatic and is probably faster because it
+    /// avoids consuming the iterator.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// fn example(iter: impl ExactSizeIterator<Item = i32>) {
+    ///     for (i, _) in iter.enumerate() {
+    ///         ..;
+    ///     }
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// fn example(iter: impl ExactSizeIterator<Item = i32>) {
+    ///     for i in 0..iter.len() {
+    ///       ..;
+    ///     }
+    /// }
+    /// ```
+    #[clippy::version = "1.87.0"]
+    pub UNUSED_ENUMERATE_VALUE,
+    nursery,
+    "using `.enumerate()` and immediately dropping the value"
+}
+
 pub struct Loops {
     msrv: Msrv,
     enforce_iter_loop_reborrow: bool,
@@ -822,6 +854,7 @@ impl_lint_pass!(Loops => [
     INFINITE_LOOP,
     MANUAL_SLICE_FILL,
     CHAR_INDICES_AS_BYTE_INDICES,
+    UNUSED_ENUMERATE_VALUE,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Loops {
@@ -906,6 +939,7 @@ impl Loops {
         manual_find::check(cx, pat, arg, body, span, expr);
         unused_enumerate_index::check(cx, pat, arg, body);
         char_indices_as_byte_indices::check(cx, pat, arg, body);
+        unused_enumerate_value::check(cx, pat, arg, body);
     }
 
     fn check_for_loop_arg(&self, cx: &LateContext<'_>, _: &Pat<'_>, arg: &Expr<'_>) {

clippy_lints/src/loops/unused_enumerate_value.rs

@@ -0,0 +1,87 @@
+use super::UNUSED_ENUMERATE_VALUE;
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::source::snippet;
+use clippy_utils::sugg::Sugg;
+use clippy_utils::ty::{get_adt_inherent_method, implements_trait};
+use clippy_utils::{get_trait_def_id, pat_is_wild, paths};
+use rustc_errors::Applicability;
+use rustc_hir::def::DefKind;
+use rustc_hir::{Expr, ExprKind, Pat, PatKind};
+use rustc_lint::LateContext;
+use rustc_middle::ty::{self, Ty};
+use rustc_span::sym;
+
+/// Checks for the `UNUSED_ENUMERATE_VALUE` lint.
+///
+/// TODO: Extend this lint to cover iterator chains.
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>, arg: &'tcx Expr<'_>, body: &'tcx Expr<'tcx>) {
+    if let PatKind::Tuple([index, elem], _) = pat.kind
+        && let ExprKind::MethodCall(_method, recv, [], _) = arg.kind
+        && pat_is_wild(cx, &elem.kind, body)
+        && let arg_ty = cx.typeck_results().expr_ty(arg)
+        && let ty::Adt(base, _) = *arg_ty.kind()
+        && cx.tcx.is_diagnostic_item(sym::Enumerate, base.did())
+        && let Some((DefKind::AssocFn, call_id)) = cx.typeck_results().type_dependent_def(arg.hir_id)
+        && cx.tcx.is_diagnostic_item(sym::enumerate_method, call_id)
+        && let receiver_ty = cx.typeck_results().expr_ty(recv)
+        // TODO: Replace with `sym` when it's available
+        && let Some(exact_size_iter) = get_trait_def_id(cx.tcx, &paths::ITER_EXACT_SIZE_ITERATOR)
+        && implements_trait(cx, receiver_ty, exact_size_iter, &[])
+    {
+        let (recv, applicability) = remove_trailing_iter(cx, recv);
+        span_lint_and_then(
+            cx,
+            UNUSED_ENUMERATE_VALUE,
+            arg.span,
+            "you seem to use `.enumerate()` and immediately discard the value",
+            |diag| {
+                let range_end = Sugg::hir(cx, recv, "..");
+                if applicability != Applicability::MachineApplicable {
+                    diag.help(format!("consider using `0..{range_end}.len()` instead"));
+                    return;
+                }
+
+                diag.multipart_suggestion(
+                    format!("replace `{}` with `0..{range_end}.len()`", snippet(cx, arg.span, "..")),
+                    vec![
+                        (pat.span, snippet(cx, index.span, "..").into_owned()),
+                        (arg.span, format!("0..{range_end}.len()")),
+                    ],
+                    applicability,
+                );
+            },
+        );
+    }
+}
+
+/// Removes trailing `.iter()`, `.iter_mut()`, or `.into_iter()` calls from the given expression if
+/// `len` can be called directly on the receiver. Note that this may be incorrect if the receiver is
+/// a user-defined type whose `len` method has a different meaning than the standard library.
+fn remove_trailing_iter<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> (&'tcx Expr<'tcx>, Applicability) {
+    let mut app = Applicability::MachineApplicable;
+    if let ExprKind::MethodCall(iter_path, iter_recv, _, _) = expr.kind
+        && matches!(iter_path.ident.name, sym::iter | sym::iter_mut | sym::into_iter)
+        && let iter_recv_ty = cx.typeck_results().expr_ty(iter_recv).peel_refs()
+        && let ty_is_builtin_container = is_builtin_container(cx, iter_recv_ty)
+        && (ty_is_builtin_container || get_adt_inherent_method(cx, iter_recv_ty, sym::len).is_some())
+    {
+        if !ty_is_builtin_container {
+            app = Applicability::MaybeIncorrect;
+        }
+
+        return (iter_recv, app);
+    }
+
+    (expr, app)
+}
+
+/// Return `true` if the given type is a built-in container type (array, slice, or collection).
+fn is_builtin_container(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
+    ty.is_array()
+        || ty.is_slice()
+        || ty.is_array_slice()
+        || (matches!(*ty.kind(), ty::Adt(iter_base, _) 
+    if [sym::Vec, sym::VecDeque, sym::LinkedList, sym::BTreeMap, sym::BTreeSet, sym::HashMap, sym::HashSet, sym::BinaryHeap]
+        .iter()
+        .any(|sym| cx.tcx.is_diagnostic_item(*sym, iter_base.did()))))
+}

clippy_utils/src/paths.rs

@@ -31,6 +31,7 @@ pub const CHAR_IS_ASCII: [&str; 5] = ["core", "char", "methods", "<impl char>",
 pub const IO_ERROR_NEW: [&str; 5] = ["std", "io", "error", "Error", "new"];
 pub const IO_ERRORKIND_OTHER: [&str; 5] = ["std", "io", "error", "ErrorKind", "Other"];
 pub const ALIGN_OF: [&str; 3] = ["core", "mem", "align_of"];
+pub const ITER_EXACT_SIZE_ITERATOR: [&str; 3] = ["core", "iter", "ExactSizeIterator"];
 
 // Paths in clippy itself
 pub const MSRV_STACK: [&str; 3] = ["clippy_utils", "msrvs", "MsrvStack"];

tests/ui/unused_enumerate_value.fixed

@@ -0,0 +1,20 @@
+#![warn(clippy::unused_enumerate_value)]
+
+fn main() {
+    let mut array = [1, 2, 3];
+    for index in 0..array.len() {
+        //~^ unused_enumerate_value
+        todo!();
+    }
+
+    let my_iter = vec![1, 2, 3].into_iter();
+    for index in 0..my_iter.len() {
+        //~^ unused_enumerate_value
+        todo!();
+    }
+
+    let another_iter = vec![1, 2, 3].into_iter();
+    for (index, _) in another_iter.enumerate().map(|(index, x)| (index, x + 1)) {
+        todo!();
+    }
+}

tests/ui/unused_enumerate_value.rs

@@ -0,0 +1,20 @@
+#![warn(clippy::unused_enumerate_value)]
+
+fn main() {
+    let mut array = [1, 2, 3];
+    for (index, _) in array.iter_mut().enumerate() {
+        //~^ unused_enumerate_value
+        todo!();
+    }
+
+    let my_iter = vec![1, 2, 3].into_iter();
+    for (index, _) in my_iter.enumerate() {
+        //~^ unused_enumerate_value
+        todo!();
+    }
+
+    let another_iter = vec![1, 2, 3].into_iter();
+    for (index, _) in another_iter.enumerate().map(|(index, x)| (index, x + 1)) {
+        todo!();
+    }
+}

tests/ui/unused_enumerate_value.stderr

@@ -0,0 +1,28 @@
+error: you seem to use `.enumerate()` and immediately discard the value
+  --> tests/ui/unused_enumerate_value.rs:5:23
+   |
+LL |     for (index, _) in array.iter_mut().enumerate() {
+   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::unused-enumerate-value` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::unused_enumerate_value)]`
+help: replace `array.iter_mut().enumerate()` with `0..array.len()`
+   |
+LL -     for (index, _) in array.iter_mut().enumerate() {
+LL +     for index in 0..array.len() {
+   |
+
+error: you seem to use `.enumerate()` and immediately discard the value
+  --> tests/ui/unused_enumerate_value.rs:11:23
+   |
+LL |     for (index, _) in my_iter.enumerate() {
+   |                       ^^^^^^^^^^^^^^^^^^^
+   |
+help: replace `my_iter.enumerate()` with `0..my_iter.len()`
+   |
+LL -     for (index, _) in my_iter.enumerate() {
+LL +     for index in 0..my_iter.len() {
+   |
+
+error: aborting due to 2 previous errors
+

tests/ui/unused_enumerate_value_unfixable.rs

@@ -0,0 +1,27 @@
+//@no-rustfix
+#![warn(clippy::unused_enumerate_value)]
+
+fn main() {
+    struct Length(usize);
+
+    impl IntoIterator for Length {
+        type Item = usize;
+        type IntoIter = std::iter::Once<usize>;
+
+        fn into_iter(self) -> Self::IntoIter {
+            std::iter::once(self.0)
+        }
+    }
+
+    impl Length {
+        fn len(&self) -> usize {
+            self.0
+        }
+    }
+
+    let length = Length(3);
+    for (index, _) in length.into_iter().enumerate() {
+        //~^ unused_enumerate_value
+        todo!();
+    }
+}

tests/ui/unused_enumerate_value_unfixable.stderr

@@ -0,0 +1,12 @@
+error: you seem to use `.enumerate()` and immediately discard the value
+  --> tests/ui/unused_enumerate_value_unfixable.rs:23:23
+   |
+LL |     for (index, _) in length.into_iter().enumerate() {
+   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using `0..length.len()` instead
+   = note: `-D clippy::unused-enumerate-value` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::unused_enumerate_value)]`
+
+error: aborting due to 1 previous error
+