Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 31 additions & 4 deletions clippy_lints/src/swap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -350,12 +350,24 @@ 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 initialization is inside the suggestion span but the variable is not used as an index
// or elsewhere later
// 3. Variable initialization is inside the suggestion span and the variable is 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();
}

Expand All @@ -371,6 +383,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)
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/manual_swap_auto_fix.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
14 changes: 14 additions & 0 deletions tests/ui/manual_swap_auto_fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
11 changes: 10 additions & 1 deletion tests/ui/manual_swap_auto_fix.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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