diff --git a/clippy_lints/src/swap.rs b/clippy_lints/src/swap.rs index e3ecd6508bf9..5ecbb56925ec 100644 --- a/clippy_lints/src/swap.rs +++ b/clippy_lints/src/swap.rs @@ -3,7 +3,7 @@ use clippy_utils::source::{snippet_indent, snippet_with_context}; use clippy_utils::sugg::Sugg; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{can_mut_borrow_both, eq_expr_value, is_in_const_context, std_or_core}; +use clippy_utils::{can_mut_borrow_both, eq_expr_value, is_in_const_context, path_to_local, std_or_core}; use itertools::Itertools; use rustc_data_structures::fx::FxIndexSet; @@ -350,12 +350,21 @@ 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: + // - Variable initialization is outside the suggestion span + // - Variable declaration is outside the suggestion span + // - Variable is not used as an index or elsewhere later + if !self.suggest_span.contains(init.span) + || path_to_local(expr) + .is_some_and(|hir_id| !self.suggest_span.contains(self.cx.tcx.hir_span(hir_id))) + || !self.is_used_other_than_swapping(first_segment.ident) + { return String::new(); } diff --git a/tests/ui/manual_swap_auto_fix.fixed b/tests/ui/manual_swap_auto_fix.fixed index 28466ff3f9b4..6cd81bafce8b 100644 --- a/tests/ui/manual_swap_auto_fix.fixed +++ b/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; + } +} diff --git a/tests/ui/manual_swap_auto_fix.rs b/tests/ui/manual_swap_auto_fix.rs index c9880e651cd7..19dabfd833f6 100644 --- a/tests/ui/manual_swap_auto_fix.rs +++ b/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; + } +} diff --git a/tests/ui/manual_swap_auto_fix.stderr b/tests/ui/manual_swap_auto_fix.stderr index 7ab898fcc726..a0bb32233e23 100644 --- a/tests/ui/manual_swap_auto_fix.stderr +++ b/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