Skip to content

Commit d1d5f79

Browse files
committed
Fix FP in manual_swap lint with slice-like types
1 parent d3232b0 commit d1d5f79

File tree

1 file changed

+55
-24
lines changed

1 file changed

+55
-24
lines changed

clippy_lints/src/swap.rs

Lines changed: 55 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -97,29 +97,6 @@ fn check_manual_swap(cx: &LateContext<'_, '_>, block: &Block) {
9797
if SpanlessEq::new(cx).ignore_fn().eq_expr(tmp_init, lhs1);
9898
if SpanlessEq::new(cx).ignore_fn().eq_expr(rhs1, lhs2);
9999
then {
100-
fn check_for_slice<'a>(
101-
cx: &LateContext<'_, '_>,
102-
lhs1: &'a Expr,
103-
lhs2: &'a Expr,
104-
) -> Option<(&'a Expr, &'a Expr, &'a Expr)> {
105-
if let ExprKind::Index(ref lhs1, ref idx1) = lhs1.kind {
106-
if let ExprKind::Index(ref lhs2, ref idx2) = lhs2.kind {
107-
if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, lhs2) {
108-
let ty = walk_ptrs_ty(cx.tables.expr_ty(lhs1));
109-
110-
if matches!(ty.kind, ty::Slice(_)) ||
111-
matches!(ty.kind, ty::Array(_, _)) ||
112-
is_type_diagnostic_item(cx, ty, Symbol::intern("vec_type")) ||
113-
match_type(cx, ty, &paths::VEC_DEQUE) {
114-
return Some((lhs1, idx1, idx2));
115-
}
116-
}
117-
}
118-
}
119-
120-
None
121-
}
122-
123100
if let ExprKind::Field(ref lhs1, _) = lhs1.kind {
124101
if let ExprKind::Field(ref lhs2, _) = lhs2.kind {
125102
if lhs1.hir_id.owner_def_id() == lhs2.hir_id.owner_def_id() {
@@ -128,7 +105,11 @@ fn check_manual_swap(cx: &LateContext<'_, '_>, block: &Block) {
128105
}
129106
}
130107

131-
let (replace, what, sugg) = if let Some((slice, idx1, idx2)) = check_for_slice(cx, lhs1, lhs2) {
108+
let slice = check_for_slice(cx, lhs1, lhs2);
109+
110+
let (replace, what, sugg) = if let Slice::NotSwappable = slice {
111+
return;
112+
} else if let Slice::Swappable(slice, idx1, idx2) = slice {
132113
if let Some(slice) = Sugg::hir_opt(cx, slice) {
133114
(false,
134115
format!(" elements of `{}`", slice),
@@ -171,6 +152,56 @@ fn check_manual_swap(cx: &LateContext<'_, '_>, block: &Block) {
171152
}
172153
}
173154

155+
enum Slice<'a> {
156+
/// `slice.swap(idx1, idx2)` can be used
157+
///
158+
/// ## Example
159+
///
160+
/// ```rust
161+
/// let t = a[1];
162+
/// a[1] = a[0];
163+
/// a[0] = t;
164+
/// // can be written as
165+
/// a.swap(0, 1);
166+
/// ```
167+
Swappable(&'a Expr, &'a Expr, &'a Expr),
168+
/// The `swap` function cannot be used.
169+
///
170+
/// ## Example
171+
///
172+
/// ```rust
173+
/// let t = a[0][1];
174+
/// a[0][1] = a[1][0];
175+
/// a[1][0] = t;
176+
/// ```
177+
NotSwappable,
178+
/// Not a slice
179+
None,
180+
}
181+
182+
/// Checks if both expressions are index operations into "slice-like" types.
183+
fn check_for_slice<'a>(cx: &LateContext<'_, '_>, lhs1: &'a Expr, lhs2: &'a Expr) -> Slice<'a> {
184+
if let ExprKind::Index(ref lhs1, ref idx1) = lhs1.kind {
185+
if let ExprKind::Index(ref lhs2, ref idx2) = lhs2.kind {
186+
if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs1, lhs2) {
187+
let ty = walk_ptrs_ty(cx.tables.expr_ty(lhs1));
188+
189+
if matches!(ty.kind, ty::Slice(_))
190+
|| matches!(ty.kind, ty::Array(_, _))
191+
|| is_type_diagnostic_item(cx, ty, Symbol::intern("vec_type"))
192+
|| match_type(cx, ty, &paths::VEC_DEQUE)
193+
{
194+
return Slice::Swappable(lhs1, idx1, idx2);
195+
}
196+
} else {
197+
return Slice::NotSwappable;
198+
}
199+
}
200+
}
201+
202+
Slice::None
203+
}
204+
174205
/// Implementation of the `ALMOST_SWAPPED` lint.
175206
fn check_suspicious_swap(cx: &LateContext<'_, '_>, block: &Block) {
176207
for w in block.stmts.windows(2) {

0 commit comments

Comments
 (0)