about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEthiraric <ethiraric@gmail.com>2024-03-07 18:50:47 +0100
committerEthiraric <ethiraric@gmail.com>2024-03-13 20:28:01 +0100
commit7cdeac5773ac7664731d8cb850334559b4b9dab8 (patch)
treea889d5353b2621899aa0b9acf8c6bcd82cccba5c
parent43f4ec3b133262e6d23a0f5401aadaf60f5bc07a (diff)
downloadrust-7cdeac5773ac7664731d8cb850334559b4b9dab8.tar.gz
rust-7cdeac5773ac7664731d8cb850334559b4b9dab8.zip
[`unused_enumerate_index`]: trigger on method calls
The lint used to check for patterns looking like:
```rs
for (_, x) in some_iter.enumerate() {
    // Index is ignored
}
```

This commit further checks for chained method calls constructs where we
can detect that the index is unused. Currently, this checks only for the
following patterns:
```rs
some_iter.enumerate().map_function(|(_, x)| ..)
let x = some_iter.enumerate();
x.map_function(|(_, x)| ..)
```
where `map_function` is one of `all`, `any`, `filter_map`, `find_map`,
`flat_map`, `for_each` or `map`.

Fixes #12411.
-rw-r--r--clippy_lints/src/loops/unused_enumerate_index.rs2
-rw-r--r--clippy_lints/src/methods/mod.rs68
-rw-r--r--clippy_lints/src/methods/unused_enumerate_index.rs102
-rw-r--r--clippy_utils/src/paths.rs2
-rw-r--r--tests/ui/unused_enumerate_index.fixed34
-rw-r--r--tests/ui/unused_enumerate_index.rs34
-rw-r--r--tests/ui/unused_enumerate_index.stderr42
7 files changed, 253 insertions, 31 deletions
diff --git a/clippy_lints/src/loops/unused_enumerate_index.rs b/clippy_lints/src/loops/unused_enumerate_index.rs
index e86a63a2738..31f0f1cfeba 100644
--- a/clippy_lints/src/loops/unused_enumerate_index.rs
+++ b/clippy_lints/src/loops/unused_enumerate_index.rs
@@ -8,6 +8,8 @@ use rustc_lint::LateContext;
 use rustc_middle::ty;
 
 /// Checks for the `UNUSED_ENUMERATE_INDEX` lint.
+///
+/// The lint is also partially implemented in `clippy_lints/src/methods/unused_enumerate_index.rs`.
 pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>, arg: &Expr<'_>, body: &'tcx Expr<'tcx>) {
     if let PatKind::Tuple([index, elem], _) = pat.kind
         && let ExprKind::MethodCall(_method, self_arg, [], _) = arg.kind
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 8a24ccea3a1..d6d9425027e 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -118,6 +118,7 @@ mod unnecessary_literal_unwrap;
 mod unnecessary_result_map_or_else;
 mod unnecessary_sort_by;
 mod unnecessary_to_owned;
+mod unused_enumerate_index;
 mod unwrap_expect_used;
 mod useless_asref;
 mod utils;
@@ -4403,6 +4404,7 @@ impl Methods {
                     zst_offset::check(cx, expr, recv);
                 },
                 ("all", [arg]) => {
+                    unused_enumerate_index::check(cx, expr, recv, arg);
                     if let Some(("cloned", recv2, [], _, _)) = method_call(recv) {
                         iter_overeager_cloned::check(
                             cx,
@@ -4421,23 +4423,26 @@ impl Methods {
                         unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
                     }
                 },
-                ("any", [arg]) => match method_call(recv) {
-                    Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(
-                        cx,
-                        expr,
-                        recv,
-                        recv2,
-                        iter_overeager_cloned::Op::NeedlessMove(arg),
-                        false,
-                    ),
-                    Some(("chars", recv, _, _, _))
-                        if let ExprKind::Closure(arg) = arg.kind
-                            && let body = cx.tcx.hir().body(arg.body)
-                            && let [param] = body.params =>
-                    {
-                        string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv);
-                    },
-                    _ => {},
+                ("any", [arg]) => {
+                    unused_enumerate_index::check(cx, expr, recv, arg);
+                    match method_call(recv) {
+                        Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(
+                            cx,
+                            expr,
+                            recv,
+                            recv2,
+                            iter_overeager_cloned::Op::NeedlessMove(arg),
+                            false,
+                        ),
+                        Some(("chars", recv, _, _, _))
+                            if let ExprKind::Closure(arg) = arg.kind
+                                && let body = cx.tcx.hir().body(arg.body)
+                                && let [param] = body.params =>
+                        {
+                            string_lit_chars_any::check(cx, expr, recv, param, peel_blocks(body.value), &self.msrv);
+                        },
+                        _ => {},
+                    }
                 },
                 ("arg", [arg]) => {
                     suspicious_command_arg_space::check(cx, recv, arg, span);
@@ -4570,14 +4575,17 @@ impl Methods {
                     }
                 },
                 ("filter_map", [arg]) => {
+                    unused_enumerate_index::check(cx, expr, recv, arg);
                     unnecessary_filter_map::check(cx, expr, arg, name);
                     filter_map_bool_then::check(cx, expr, arg, call_span);
                     filter_map_identity::check(cx, expr, arg, span);
                 },
                 ("find_map", [arg]) => {
+                    unused_enumerate_index::check(cx, expr, recv, arg);
                     unnecessary_filter_map::check(cx, expr, arg, name);
                 },
                 ("flat_map", [arg]) => {
+                    unused_enumerate_index::check(cx, expr, recv, arg);
                     flat_map_identity::check(cx, expr, arg, span);
                     flat_map_option::check(cx, expr, arg, span);
                 },
@@ -4599,17 +4607,20 @@ impl Methods {
                     manual_try_fold::check(cx, expr, init, acc, call_span, &self.msrv);
                     unnecessary_fold::check(cx, expr, init, acc, span);
                 },
-                ("for_each", [arg]) => match method_call(recv) {
-                    Some(("inspect", _, [_], span2, _)) => inspect_for_each::check(cx, expr, span2),
-                    Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(
-                        cx,
-                        expr,
-                        recv,
-                        recv2,
-                        iter_overeager_cloned::Op::NeedlessMove(arg),
-                        false,
-                    ),
-                    _ => {},
+                ("for_each", [arg]) => {
+                    unused_enumerate_index::check(cx, expr, recv, arg);
+                    match method_call(recv) {
+                        Some(("inspect", _, [_], span2, _)) => inspect_for_each::check(cx, expr, span2),
+                        Some(("cloned", recv2, [], _, _)) => iter_overeager_cloned::check(
+                            cx,
+                            expr,
+                            recv,
+                            recv2,
+                            iter_overeager_cloned::Op::NeedlessMove(arg),
+                            false,
+                        ),
+                        _ => {},
+                    }
                 },
                 ("get", [arg]) => {
                     get_first::check(cx, expr, recv, arg);
@@ -4650,6 +4661,7 @@ impl Methods {
                 },
                 (name @ ("map" | "map_err"), [m_arg]) => {
                     if name == "map" {
+                        unused_enumerate_index::check(cx, expr, recv, m_arg);
                         map_clone::check(cx, expr, recv, m_arg, &self.msrv);
                         match method_call(recv) {
                             Some((map_name @ ("iter" | "into_iter"), recv2, _, _, _)) => {
diff --git a/clippy_lints/src/methods/unused_enumerate_index.rs b/clippy_lints/src/methods/unused_enumerate_index.rs
new file mode 100644
index 00000000000..506a0f4304a
--- /dev/null
+++ b/clippy_lints/src/methods/unused_enumerate_index.rs
@@ -0,0 +1,102 @@
+use clippy_utils::diagnostics::{multispan_sugg, span_lint_hir_and_then};
+use clippy_utils::paths::{CORE_ITER_ENUMERATE_METHOD, CORE_ITER_ENUMERATE_STRUCT};
+use clippy_utils::source::snippet;
+use clippy_utils::{expr_or_init, is_trait_method, match_def_path, pat_is_wild};
+use rustc_hir::{Expr, ExprKind, PatKind};
+use rustc_lint::LateContext;
+use rustc_middle::ty::AdtDef;
+use rustc_span::sym;
+
+use crate::loops::UNUSED_ENUMERATE_INDEX;
+
+/// Check for the `UNUSED_ENUMERATE_INDEX` lint outside of loops.
+///
+/// The lint is declared in `clippy_lints/src/loops/mod.rs`. There, the following pattern is
+/// checked:
+/// ```ignore
+/// for (_, x) in some_iter.enumerate() {
+///     // Index is ignored
+/// }
+/// ```
+///
+/// This `check` function checks for chained method calls constructs where we can detect that the
+/// index is unused. Currently, this checks only for the following patterns:
+/// ```ignore
+/// some_iter.enumerate().map_function(|(_, x)| ..)
+/// let x = some_iter.enumerate();
+/// x.map_function(|(_, x)| ..)
+/// ```
+/// where `map_function` is one of `all`, `any`, `filter_map`, `find_map`, `flat_map`, `for_each` or
+/// `map`.
+///
+/// # Preconditions
+/// This function must be called not on the `enumerate` call expression itself, but on any of the
+/// map functions listed above. It will ensure that `recv` is a `std::iter::Enumerate` instance and
+/// that the method call is one of the `std::iter::Iterator` trait.
+///
+/// * `call_expr`: The map function call expression
+/// * `recv`: The receiver of the call
+/// * `closure_arg`: The argument to the map function call containing the closure/function to apply
+pub(super) fn check(cx: &LateContext<'_>, call_expr: &Expr<'_>, recv: &Expr<'_>, closure_arg: &Expr<'_>) {
+    let recv_ty = cx.typeck_results().expr_ty(recv);
+    if let Some(recv_ty_defid) = recv_ty.ty_adt_def().map(AdtDef::did)
+        // If we call a method on a `std::iter::Enumerate` instance
+        && match_def_path(cx, recv_ty_defid, &CORE_ITER_ENUMERATE_STRUCT)
+        // If we are calling a method of the `Iterator` trait
+        && is_trait_method(cx, call_expr, sym::Iterator)
+        // And the map argument is a closure
+        && let ExprKind::Closure(closure) = closure_arg.kind
+        && let closure_body = cx.tcx.hir().body(closure.body)
+        // And that closure has one argument ...
+        && let [closure_param] = closure_body.params
+        // .. which is a tuple of 2 elements
+        && let PatKind::Tuple([index, elem], ..) = closure_param.pat.kind
+        // And that the first element (the index) is either `_` or unused in the body
+        && pat_is_wild(cx, &index.kind, closure_body)
+        // Try to find the initializer for `recv`. This is needed in case `recv` is a local_binding. In the
+        // first example below, `expr_or_init` would return `recv`.
+        // ```
+        // iter.enumerate().map(|(_, x)| x)
+        // ^^^^^^^^^^^^^^^^ `recv`, a call to `std::iter::Iterator::enumerate`
+        //
+        // let binding = iter.enumerate();
+        //               ^^^^^^^^^^^^^^^^ `recv_init_expr`
+        // binding.map(|(_, x)| x)
+        // ^^^^^^^ `recv`, not a call to `std::iter::Iterator::enumerate`
+        // ```
+        && let recv_init_expr = expr_or_init(cx, recv)
+        // Make sure the initializer is a method call. It may be that the `Enumerate` comes from something
+        // that we cannot control.
+        // This would for instance happen with:
+        // ```
+        // external_lib::some_function_returning_enumerate().map(|(_, x)| x)
+        // ```
+        && let ExprKind::MethodCall(_, enumerate_recv, _, enumerate_span) = recv_init_expr.kind
+        && let Some(enumerate_defid) = cx.typeck_results().type_dependent_def_id(recv_init_expr.hir_id)
+        // Make sure the method call is `std::iter::Iterator::enumerate`.
+        && match_def_path(cx, enumerate_defid, &CORE_ITER_ENUMERATE_METHOD)
+    {
+        // Suggest removing the tuple from the closure and the preceding call to `enumerate`, whose span we
+        // can get from the `MethodCall`.
+        span_lint_hir_and_then(
+            cx,
+            UNUSED_ENUMERATE_INDEX,
+            recv_init_expr.hir_id,
+            enumerate_span,
+            "you seem to use `.enumerate()` and immediately discard the index",
+            |diag| {
+                multispan_sugg(
+                    diag,
+                    "remove the `.enumerate()` call",
+                    vec![
+                        (closure_param.span, snippet(cx, elem.span, "..").into_owned()),
+                        (
+                            enumerate_span.with_lo(enumerate_recv.span.source_callsite().hi()),
+                            String::new(),
+                        ),
+                    ],
+                );
+            },
+        );
+    }
+}
diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs
index 987f28192a8..456b8019e95 100644
--- a/clippy_utils/src/paths.rs
+++ b/clippy_utils/src/paths.rs
@@ -19,6 +19,8 @@ pub const BTREESET_ITER: [&str; 6] = ["alloc", "collections", "btree", "set", "B
 pub const CLONE_TRAIT_METHOD: [&str; 4] = ["core", "clone", "Clone", "clone"];
 pub const CORE_ITER_CLONED: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "cloned"];
 pub const CORE_ITER_COPIED: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "copied"];
+pub const CORE_ITER_ENUMERATE_METHOD: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "enumerate"];
+pub const CORE_ITER_ENUMERATE_STRUCT: [&str; 5] = ["core", "iter", "adapters", "enumerate", "Enumerate"];
 pub const CORE_ITER_FILTER: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "filter"];
 pub const CORE_RESULT_OK_METHOD: [&str; 4] = ["core", "result", "Result", "ok"];
 pub const CSTRING_AS_C_STR: [&str; 5] = ["alloc", "ffi", "c_str", "CString", "as_c_str"];
diff --git a/tests/ui/unused_enumerate_index.fixed b/tests/ui/unused_enumerate_index.fixed
index d079807ab58..1224eb54ba5 100644
--- a/tests/ui/unused_enumerate_index.fixed
+++ b/tests/ui/unused_enumerate_index.fixed
@@ -3,6 +3,10 @@
 
 use std::iter::Enumerate;
 
+fn get_enumerate() -> Enumerate<std::vec::IntoIter<i32>> {
+    vec![1].into_iter().enumerate()
+}
+
 fn main() {
     let v = [1, 2, 3];
     for x in v.iter() {
@@ -55,4 +59,34 @@ fn main() {
     for x in dummy {
         println!("{x}");
     }
+
+    let _ = vec![1, 2, 3].into_iter().map(|x| println!("{x}"));
+
+    let p = vec![1, 2, 3].into_iter();
+    p.map(|x| println!("{x}"));
+
+    // This shouldn't trigger the lint. `get_enumerate` may come from an external library on which we
+    // have no control.
+    let p = get_enumerate();
+    p.map(|(_, x)| println!("{x}"));
+
+    // This shouldn't trigger the lint. The `enumerate` call is in a different context.
+    macro_rules! mac {
+        () => {
+            [1].iter().enumerate()
+        };
+    }
+    _ = mac!().map(|(_, v)| v);
+
+    macro_rules! mac2 {
+        () => {
+            [1].iter()
+        };
+    }
+    _ = mac2!().map(|_v| {});
+
+    // This shouldn't trigger the lint because of the `allow`.
+    #[allow(clippy::unused_enumerate_index)]
+    let v = [1].iter().enumerate();
+    v.map(|(_, _x)| {});
 }
diff --git a/tests/ui/unused_enumerate_index.rs b/tests/ui/unused_enumerate_index.rs
index 2d524da7632..66f98b690cb 100644
--- a/tests/ui/unused_enumerate_index.rs
+++ b/tests/ui/unused_enumerate_index.rs
@@ -3,6 +3,10 @@
 
 use std::iter::Enumerate;
 
+fn get_enumerate() -> Enumerate<std::vec::IntoIter<i32>> {
+    vec![1].into_iter().enumerate()
+}
+
 fn main() {
     let v = [1, 2, 3];
     for (_, x) in v.iter().enumerate() {
@@ -55,4 +59,34 @@ fn main() {
     for (_, x) in dummy.enumerate() {
         println!("{x}");
     }
+
+    let _ = vec![1, 2, 3].into_iter().enumerate().map(|(_, x)| println!("{x}"));
+
+    let p = vec![1, 2, 3].into_iter().enumerate();
+    p.map(|(_, x)| println!("{x}"));
+
+    // This shouldn't trigger the lint. `get_enumerate` may come from an external library on which we
+    // have no control.
+    let p = get_enumerate();
+    p.map(|(_, x)| println!("{x}"));
+
+    // This shouldn't trigger the lint. The `enumerate` call is in a different context.
+    macro_rules! mac {
+        () => {
+            [1].iter().enumerate()
+        };
+    }
+    _ = mac!().map(|(_, v)| v);
+
+    macro_rules! mac2 {
+        () => {
+            [1].iter()
+        };
+    }
+    _ = mac2!().enumerate().map(|(_, _v)| {});
+
+    // This shouldn't trigger the lint because of the `allow`.
+    #[allow(clippy::unused_enumerate_index)]
+    let v = [1].iter().enumerate();
+    v.map(|(_, _x)| {});
 }
diff --git a/tests/ui/unused_enumerate_index.stderr b/tests/ui/unused_enumerate_index.stderr
index 7bd7d29117e..f8babf428c0 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
-  --> tests/ui/unused_enumerate_index.rs:8:19
+  --> tests/ui/unused_enumerate_index.rs:12:19
    |
 LL |     for (_, x) in v.iter().enumerate() {
    |                   ^^^^^^^^^^^^^^^^^^^^
@@ -12,7 +12,7 @@ LL |     for x in v.iter() {
    |         ~    ~~~~~~~~
 
 error: you seem to use `.enumerate()` and immediately discard the index
-  --> tests/ui/unused_enumerate_index.rs:55:19
+  --> tests/ui/unused_enumerate_index.rs:59:19
    |
 LL |     for (_, x) in dummy.enumerate() {
    |                   ^^^^^^^^^^^^^^^^^
@@ -22,5 +22,41 @@ help: remove the `.enumerate()` call
 LL |     for x in dummy {
    |         ~    ~~~~~
 
-error: aborting due to 2 previous errors
+error: you seem to use `.enumerate()` and immediately discard the index
+  --> tests/ui/unused_enumerate_index.rs:63:39
+   |
+LL |     let _ = vec![1, 2, 3].into_iter().enumerate().map(|(_, x)| println!("{x}"));
+   |                                       ^^^^^^^^^^^
+   |
+help: remove the `.enumerate()` call
+   |
+LL -     let _ = vec![1, 2, 3].into_iter().enumerate().map(|(_, x)| println!("{x}"));
+LL +     let _ = vec![1, 2, 3].into_iter().map(|x| println!("{x}"));
+   |
+
+error: you seem to use `.enumerate()` and immediately discard the index
+  --> tests/ui/unused_enumerate_index.rs:65:39
+   |
+LL |     let p = vec![1, 2, 3].into_iter().enumerate();
+   |                                       ^^^^^^^^^^^
+   |
+help: remove the `.enumerate()` call
+   |
+LL ~     let p = vec![1, 2, 3].into_iter();
+LL ~     p.map(|x| println!("{x}"));
+   |
+
+error: you seem to use `.enumerate()` and immediately discard the index
+  --> tests/ui/unused_enumerate_index.rs:86:17
+   |
+LL |     _ = mac2!().enumerate().map(|(_, _v)| {});
+   |                 ^^^^^^^^^^^
+   |
+help: remove the `.enumerate()` call
+   |
+LL -     _ = mac2!().enumerate().map(|(_, _v)| {});
+LL +     _ = mac2!().map(|_v| {});
+   |
+
+error: aborting due to 5 previous errors