Skip to content

Commit 29492da

Browse files
committed
Fix suggestion-causes-error of manual_swap
1 parent ed143af commit 29492da

File tree

4 files changed

+66
-5
lines changed

4 files changed

+66
-5
lines changed

clippy_lints/src/swap.rs

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use rustc_data_structures::fx::FxIndexSet;
1010
use rustc_hir::intravisit::{Visitor, walk_expr};
1111

1212
use rustc_errors::Applicability;
13-
use rustc_hir::{AssignOpKind, Block, Expr, ExprKind, LetStmt, PatKind, QPath, Stmt, StmtKind};
13+
use rustc_hir::{AssignOpKind, Block, Expr, ExprKind, LetStmt, PatKind, Path, QPath, Stmt, StmtKind};
1414
use rustc_lint::{LateContext, LateLintPass, LintContext};
1515
use rustc_middle::ty;
1616
use rustc_session::declare_lint_pass;
@@ -350,12 +350,24 @@ 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+
// 1. Variable initialization is outside the suggestion span
361+
// 2. Variable initialization is inside the suggestion span but the variable is not used as an index
362+
// or elsewhere later
363+
// 3. Variable initialization is inside the suggestion span and the variable is used as an
364+
// index/elsewhere later, but its declaration is outside the suggestion span
365+
if !self.suggest_span.contains(init.span)
366+
|| !self.is_used_other_than_swapping(first_segment.ident)
367+
|| self
368+
.get_res_span(expr)
369+
.is_some_and(|span| !self.suggest_span.contains(span))
370+
{
359371
return String::new();
360372
}
361373

@@ -371,6 +383,21 @@ impl<'tcx> IndexBinding<'_, 'tcx> {
371383
}
372384
}
373385

386+
fn get_res_span(&self, expr: &'tcx Expr<'tcx>) -> Option<Span> {
387+
if let ExprKind::Path(QPath::Resolved(
388+
_,
389+
Path {
390+
res: rustc_hir::def::Res::Local(hir_id),
391+
..
392+
},
393+
)) = expr.kind
394+
{
395+
Some(self.cx.tcx.hir_span(*hir_id))
396+
} else {
397+
None
398+
}
399+
}
400+
374401
fn is_used_other_than_swapping(&mut self, idx_ident: Ident) -> bool {
375402
if Self::is_used_slice_indexed(self.swap1_idx, idx_ident)
376403
|| Self::is_used_slice_indexed(self.swap2_idx, idx_ident)

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)