Skip to content

Commit 59d8345

Browse files
Fix suggestion-causes-error of manual_swap (#14978)
Fixes: rust-lang/rust-clippy#14931 changelog: Fix [`manual_swap`]'s suggestion-causes-error when the variable is mutable or as loop variable.
2 parents 180adb3 + 4389e17 commit 59d8345

File tree

4 files changed

+48
-5
lines changed

4 files changed

+48
-5
lines changed

clippy_lints/src/swap.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use clippy_utils::source::{snippet_indent, snippet_with_context};
33
use clippy_utils::sugg::Sugg;
44
use clippy_utils::ty::is_type_diagnostic_item;
55

6-
use clippy_utils::{can_mut_borrow_both, eq_expr_value, is_in_const_context, std_or_core};
6+
use clippy_utils::{can_mut_borrow_both, eq_expr_value, is_in_const_context, path_to_local, std_or_core};
77
use itertools::Itertools;
88

99
use rustc_data_structures::fx::FxIndexSet;
@@ -350,12 +350,21 @@ impl<'tcx> IndexBinding<'_, 'tcx> {
350350
format!("{lhs_snippet}{rhs_snippet}")
351351
},
352352
ExprKind::Path(QPath::Resolved(_, path)) => {
353-
let init = self.cx.expr_or_init(expr);
354-
355353
let Some(first_segment) = path.segments.first() else {
356354
return String::new();
357355
};
358-
if !self.suggest_span.contains(init.span) || !self.is_used_other_than_swapping(first_segment.ident) {
356+
357+
let init = self.cx.expr_or_init(expr);
358+
359+
// We skip suggesting a variable binding in any of these cases:
360+
// - Variable initialization is outside the suggestion span
361+
// - Variable declaration is outside the suggestion span
362+
// - Variable is not used as an index or elsewhere later
363+
if !self.suggest_span.contains(init.span)
364+
|| path_to_local(expr)
365+
.is_some_and(|hir_id| !self.suggest_span.contains(self.cx.tcx.hir_span(hir_id)))
366+
|| !self.is_used_other_than_swapping(first_segment.ident)
367+
{
359368
return String::new();
360369
}
361370

tests/ui/manual_swap_auto_fix.fixed

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,14 @@ fn swap8() {
5555
let i2 = 1;
5656
v.swap(i1 + i2, i2);
5757
}
58+
59+
fn issue_14931() {
60+
let mut v = [1, 2, 3, 4];
61+
62+
let mut i1 = 0;
63+
for i2 in 0..4 {
64+
v.swap(i1, i2);
65+
66+
i1 += 2;
67+
}
68+
}

tests/ui/manual_swap_auto_fix.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,17 @@ fn swap8() {
7878
v[i1 + i2] = v[i2];
7979
v[i2] = tmp;
8080
}
81+
82+
fn issue_14931() {
83+
let mut v = [1, 2, 3, 4];
84+
85+
let mut i1 = 0;
86+
for i2 in 0..4 {
87+
let tmp = v[i1];
88+
//~^ manual_swap
89+
v[i1] = v[i2];
90+
v[i2] = tmp;
91+
92+
i1 += 2;
93+
}
94+
}

tests/ui/manual_swap_auto_fix.stderr

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,5 +92,14 @@ LL | | v[i1 + i2] = v[i2];
9292
LL | | v[i2] = tmp;
9393
| |________________^ help: try: `v.swap(i1 + i2, i2);`
9494

95-
error: aborting due to 8 previous errors
95+
error: this looks like you are swapping elements of `v` manually
96+
--> tests/ui/manual_swap_auto_fix.rs:87:9
97+
|
98+
LL | / let tmp = v[i1];
99+
LL | |
100+
LL | | v[i1] = v[i2];
101+
LL | | v[i2] = tmp;
102+
| |____________________^ help: try: `v.swap(i1, i2);`
103+
104+
error: aborting due to 9 previous errors
96105

0 commit comments

Comments
 (0)