Note that using enumerate() will swap the arguments (#14969)

The autofix now:

- includes the swapping of index and element in the closure in which the
content will be consumed;
- notes, with applicability `MaybeIncorrect` (because it will be), that
the element and the index will be swapped.

changelog: [`range_zip_with_len`]: better suggestion, and better
applicability

Fixes #14929

Author: blyxyas

clippy_lints/src/methods/range_zip_with_len.rs

@@ -1,10 +1,9 @@
-use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::source::snippet;
-use clippy_utils::{SpanlessEq, higher, is_integer_const, is_trait_method};
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::source::{SpanRangeExt as _, snippet_with_applicability};
+use clippy_utils::{SpanlessEq, get_parent_expr, higher, is_integer_const, is_trait_method, sym};
 use rustc_errors::Applicability;
-use rustc_hir::{Expr, ExprKind, QPath};
+use rustc_hir::{Expr, ExprKind, Node, Pat, PatKind, QPath};
 use rustc_lint::LateContext;
-use rustc_span::sym;
 
 use super::RANGE_ZIP_WITH_LEN;
 
@@ -21,14 +20,93 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, recv: &'
         && let ExprKind::Path(QPath::Resolved(_, len_path)) = len_recv.kind
         && SpanlessEq::new(cx).eq_path_segments(iter_path.segments, len_path.segments)
     {
-        span_lint_and_sugg(
+        span_lint_and_then(
             cx,
             RANGE_ZIP_WITH_LEN,
             expr.span,
             "using `.zip()` with a range and `.len()`",
-            "try",
-            format!("{}.iter().enumerate()", snippet(cx, recv.span, "_")),
-            Applicability::MachineApplicable,
+            |diag| {
+                // If the iterator content is consumed by a pattern with exactly two elements, swap
+                // the order of those elements. Otherwise, the suggestion will be marked as
+                // `Applicability::MaybeIncorrect` (because it will be), and a note will be added
+                // to the diagnostic to underline the swapping of the index and the content.
+                let pat = methods_pattern(cx, expr).or_else(|| for_loop_pattern(cx, expr));
+                let invert_bindings = if let Some(pat) = pat
+                    && pat.span.eq_ctxt(expr.span)
+                    && let PatKind::Tuple([first, second], _) = pat.kind
+                {
+                    Some((first.span, second.span))
+                } else {
+                    None
+                };
+                let mut app = Applicability::MachineApplicable;
+                let mut suggestions = vec![(
+                    expr.span,
+                    format!(
+                        "{}.iter().enumerate()",
+                        snippet_with_applicability(cx, recv.span, "_", &mut app)
+                    ),
+                )];
+                if let Some((left, right)) = invert_bindings
+                    && let Some(snip_left) = left.get_source_text(cx)
+                    && let Some(snip_right) = right.get_source_text(cx)
+                {
+                    suggestions.extend([(left, snip_right.to_string()), (right, snip_left.to_string())]);
+                } else {
+                    app = Applicability::MaybeIncorrect;
+                }
+                diag.multipart_suggestion("use", suggestions, app);
+                if app != Applicability::MachineApplicable {
+                    diag.note("the order of the element and the index will be swapped");
+                }
+            },
         );
     }
 }
+
+/// If `expr` is the argument of a `for` loop, return the loop pattern.
+fn for_loop_pattern<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx Pat<'tcx>> {
+    cx.tcx.hir_parent_iter(expr.hir_id).find_map(|(_, node)| {
+        if let Node::Expr(ancestor_expr) = node
+            && let Some(for_loop) = higher::ForLoop::hir(ancestor_expr)
+            && for_loop.arg.hir_id == expr.hir_id
+        {
+            Some(for_loop.pat)
+        } else {
+            None
+        }
+    })
+}
+
+/// If `expr` is the receiver of an `Iterator` method which consumes the iterator elements and feed
+/// them to a closure, return the pattern of the closure.
+fn methods_pattern<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx Pat<'tcx>> {
+    if let Some(parent_expr) = get_parent_expr(cx, expr)
+        && is_trait_method(cx, expr, sym::Iterator)
+        && let ExprKind::MethodCall(method, recv, [arg], _) = parent_expr.kind
+        && recv.hir_id == expr.hir_id
+        && matches!(
+            method.ident.name,
+            sym::all
+                | sym::any
+                | sym::filter_map
+                | sym::find_map
+                | sym::flat_map
+                | sym::for_each
+                | sym::is_partitioned
+                | sym::is_sorted_by_key
+                | sym::map
+                | sym::map_while
+                | sym::position
+                | sym::rposition
+                | sym::try_for_each
+        )
+        && let ExprKind::Closure(closure) = arg.kind
+        && let body = cx.tcx.hir_body(closure.body)
+        && let [param] = body.params
+    {
+        Some(param.pat)
+    } else {
+        None
+    }
+}

clippy_utils/src/sym.rs

@@ -189,8 +189,10 @@ generate! {
     is_none,
     is_none_or,
     is_ok,
+    is_partitioned,
     is_some,
     is_some_and,
+    is_sorted_by_key,
     isqrt,
     itertools,
     join,
@@ -208,6 +210,7 @@ generate! {
     map_continue,
     map_or,
     map_or_else,
+    map_while,
     match_indices,
     matches,
     max,
@@ -344,6 +347,7 @@ generate! {
     trim_start,
     trim_start_matches,
     truncate,
+    try_for_each,
     unreachable_pub,
     unsafe_removed_from_name,
     unused,

tests/ui/range.fixed

@@ -1,11 +1,23 @@
 #![allow(clippy::useless_vec)]
 #[warn(clippy::range_zip_with_len)]
 fn main() {
-    let v1 = vec![1, 2, 3];
-    let v2 = vec![4, 5];
+    let v1: Vec<u64> = vec![1, 2, 3];
+    let v2: Vec<u64> = vec![4, 5];
     let _x = v1.iter().enumerate();
     //~^ range_zip_with_len
 
+    //~v range_zip_with_len
+    for (i, e) in v1.iter().enumerate() {
+        let _: &u64 = e;
+        let _: usize = i;
+    }
+
+    //~v range_zip_with_len
+    v1.iter().enumerate().for_each(|(i, e)| {
+        let _: &u64 = e;
+        let _: usize = i;
+    });
+
     let _y = v1.iter().zip(0..v2.len()); // No error
 }
 

tests/ui/range.rs

@@ -1,11 +1,23 @@
 #![allow(clippy::useless_vec)]
 #[warn(clippy::range_zip_with_len)]
 fn main() {
-    let v1 = vec![1, 2, 3];
-    let v2 = vec![4, 5];
+    let v1: Vec<u64> = vec![1, 2, 3];
+    let v2: Vec<u64> = vec![4, 5];
     let _x = v1.iter().zip(0..v1.len());
     //~^ range_zip_with_len
 
+    //~v range_zip_with_len
+    for (e, i) in v1.iter().zip(0..v1.len()) {
+        let _: &u64 = e;
+        let _: usize = i;
+    }
+
+    //~v range_zip_with_len
+    v1.iter().zip(0..v1.len()).for_each(|(e, i)| {
+        let _: &u64 = e;
+        let _: usize = i;
+    });
+
     let _y = v1.iter().zip(0..v2.len()); // No error
 }
 

tests/ui/range.stderr

@@ -2,10 +2,35 @@ error: using `.zip()` with a range and `.len()`
   --> tests/ui/range.rs:6:14
    |
 LL |     let _x = v1.iter().zip(0..v1.len());
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `v1.iter().enumerate()`
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `v1.iter().enumerate()`
    |
+   = note: the order of the element and the index will be swapped
    = note: `-D clippy::range-zip-with-len` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::range_zip_with_len)]`
 
-error: aborting due to 1 previous error
+error: using `.zip()` with a range and `.len()`
+  --> tests/ui/range.rs:10:19
+   |
+LL |     for (e, i) in v1.iter().zip(0..v1.len()) {
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use
+   |
+LL -     for (e, i) in v1.iter().zip(0..v1.len()) {
+LL +     for (i, e) in v1.iter().enumerate() {
+   |
+
+error: using `.zip()` with a range and `.len()`
+  --> tests/ui/range.rs:16:5
+   |
+LL |     v1.iter().zip(0..v1.len()).for_each(|(e, i)| {
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use
+   |
+LL -     v1.iter().zip(0..v1.len()).for_each(|(e, i)| {
+LL +     v1.iter().enumerate().for_each(|(i, e)| {
+   |
+
+error: aborting due to 3 previous errors
 

tests/ui/range_unfixable.rs

@@ -0,0 +1,14 @@
+//@no-rustfix
+#![allow(clippy::useless_vec)]
+#[warn(clippy::range_zip_with_len)]
+fn main() {
+    let v1: Vec<u64> = vec![1, 2, 3];
+    let v2: Vec<u64> = vec![4, 5];
+
+    // Do not autofix, `filter()` would not consume the iterator.
+    //~v range_zip_with_len
+    v1.iter().zip(0..v1.len()).filter(|(_, i)| *i < 2).for_each(|(e, i)| {
+        let _: &u64 = e;
+        let _: usize = i;
+    });
+}

tests/ui/range_unfixable.stderr

@@ -0,0 +1,12 @@
+error: using `.zip()` with a range and `.len()`
+  --> tests/ui/range_unfixable.rs:10:5
+   |
+LL |     v1.iter().zip(0..v1.len()).filter(|(_, i)| *i < 2).for_each(|(e, i)| {
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `v1.iter().enumerate()`
+   |
+   = note: the order of the element and the index will be swapped
+   = note: `-D clippy::range-zip-with-len` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::range_zip_with_len)]`
+
+error: aborting due to 1 previous error
+