Fix suggestion-causes-error of manual_swap

Author: relaxcn

clippy_lints/src/swap.rs

@@ -10,7 +10,7 @@ use rustc_data_structures::fx::FxIndexSet;
 use rustc_hir::intravisit::{Visitor, walk_expr};
 
 use rustc_errors::Applicability;
-use rustc_hir::{AssignOpKind, Block, Expr, ExprKind, LetStmt, PatKind, QPath, Stmt, StmtKind};
+use rustc_hir::{AssignOpKind, Block, Expr, ExprKind, LetStmt, PatKind, Path, QPath, Stmt, StmtKind};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::ty;
 use rustc_session::declare_lint_pass;
@@ -350,12 +350,23 @@ impl<'tcx> IndexBinding<'_, 'tcx> {
                 format!("{lhs_snippet}{rhs_snippet}")
             },
             ExprKind::Path(QPath::Resolved(_, path)) => {
-                let init = self.cx.expr_or_init(expr);
-
                 let Some(first_segment) = path.segments.first() else {
                     return String::new();
                 };
-                if !self.suggest_span.contains(init.span) || !self.is_used_other_than_swapping(first_segment.ident) {
+
+                let init = self.cx.expr_or_init(expr);
+
+                // We skip suggesting a variable binding in any of these cases:
+                // 1. Variable initialization is outside the suggestion span
+                // 2. Variable is inside the suggestion span but not used as an index or elsewhere later
+                // 3. Variable is inside the suggestion span and used as an index/elsewhere later, but its
+                //    declaration is outside the suggestion span
+                if !self.suggest_span.contains(init.span)
+                    || !self.is_used_other_than_swapping(first_segment.ident)
+                    || self
+                        .get_res_span(expr)
+                        .is_some_and(|span| !self.suggest_span.contains(span))
+                {
                     return String::new();
                 }
 
@@ -371,6 +382,21 @@ impl<'tcx> IndexBinding<'_, 'tcx> {
         }
     }
 
+    fn get_res_span(&self, expr: &'tcx Expr<'tcx>) -> Option<Span> {
+        if let ExprKind::Path(QPath::Resolved(
+            _,
+            Path {
+                res: rustc_hir::def::Res::Local(hir_id),
+                ..
+            },
+        )) = expr.kind
+        {
+            Some(self.cx.tcx.hir_span(*hir_id))
+        } else {
+            None
+        }
+    }
+
     fn is_used_other_than_swapping(&mut self, idx_ident: Ident) -> bool {
         if Self::is_used_slice_indexed(self.swap1_idx, idx_ident)
             || Self::is_used_slice_indexed(self.swap2_idx, idx_ident)

tests/ui/manual_swap_auto_fix.fixed

@@ -55,3 +55,14 @@ fn swap8() {
     let i2 = 1;
     v.swap(i1 + i2, i2);
 }
+
+fn issue_14931() {
+    let mut v = [1, 2, 3, 4];
+
+    let mut i1 = 0;
+    for i2 in 0..4 {
+        v.swap(i1, i2);
+
+        i1 += 2;
+    }
+}

tests/ui/manual_swap_auto_fix.rs

@@ -78,3 +78,17 @@ fn swap8() {
     v[i1 + i2] = v[i2];
     v[i2] = tmp;
 }
+
+fn issue_14931() {
+    let mut v = [1, 2, 3, 4];
+
+    let mut i1 = 0;
+    for i2 in 0..4 {
+        let tmp = v[i1];
+        //~^ manual_swap
+        v[i1] = v[i2];
+        v[i2] = tmp;
+
+        i1 += 2;
+    }
+}

tests/ui/manual_swap_auto_fix.stderr

@@ -92,5 +92,14 @@ LL | |     v[i1 + i2] = v[i2];
 LL | |     v[i2] = tmp;
    | |________________^ help: try: `v.swap(i1 + i2, i2);`
 
-error: aborting due to 8 previous errors
+error: this looks like you are swapping elements of `v` manually
+  --> tests/ui/manual_swap_auto_fix.rs:87:9
+   |
+LL | /         let tmp = v[i1];
+LL | |
+LL | |         v[i1] = v[i2];
+LL | |         v[i2] = tmp;
+   | |____________________^ help: try: `v.swap(i1, i2);`
+
+error: aborting due to 9 previous errors