Skip to content

Commit 6ca5b20

Browse files
committed
Auto merge of #4478 - tsurai:master, r=flip1995
Fix incorrect swap suggestion Clippy suggests using swap on fields belonging to the same owner causing two mutable borrows of the owner. Disclosure: This is my first time working with clippy and rusts HIR. I'm very grateful for assistance and suggestions to improve the PR. fixes #981 changelog: Fix false positive in `manual_swap` lint
2 parents dfa88c6 + 9041856 commit 6ca5b20

File tree

3 files changed

+35
-7
lines changed

3 files changed

+35
-7
lines changed

clippy_lints/src/swap.rs

+8
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,14 @@ fn check_manual_swap(cx: &LateContext<'_, '_>, block: &Block) {
120120
None
121121
}
122122

123+
if let ExprKind::Field(ref lhs1, _) = lhs1.node {
124+
if let ExprKind::Field(ref lhs2, _) = lhs2.node {
125+
if lhs1.hir_id.owner_def_id() == lhs2.hir_id.owner_def_id() {
126+
return;
127+
}
128+
}
129+
}
130+
123131
let (replace, what, sugg) = if let Some((slice, idx1, idx2)) = check_for_slice(cx, lhs1, lhs2) {
124132
if let Some(slice) = Sugg::hir_opt(cx, slice) {
125133
(false,

tests/ui/swap.rs

+20
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,25 @@
33

44
struct Foo(u32);
55

6+
#[derive(Clone)]
7+
struct Bar {
8+
a: u32,
9+
b: u32,
10+
}
11+
12+
fn field() {
13+
let mut bar = Bar { a: 1, b: 2 };
14+
15+
let temp = bar.a;
16+
bar.a = bar.b;
17+
bar.b = temp;
18+
19+
let mut baz = vec![bar.clone(), bar.clone()];
20+
let temp = baz[0].a;
21+
baz[0].a = baz[1].a;
22+
baz[1].a = temp;
23+
}
24+
625
fn array() {
726
let mut foo = [1, 2];
827
let temp = foo[0];
@@ -32,6 +51,7 @@ fn vec() {
3251

3352
#[rustfmt::skip]
3453
fn main() {
54+
field();
3555
array();
3656
slice();
3757
vec();

tests/ui/swap.stderr

+7-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this looks like you are swapping elements of `foo` manually
2-
--> $DIR/swap.rs:8:5
2+
--> $DIR/swap.rs:27:5
33
|
44
LL | / let temp = foo[0];
55
LL | | foo[0] = foo[1];
@@ -9,23 +9,23 @@ LL | | foo[1] = temp;
99
= note: `-D clippy::manual-swap` implied by `-D warnings`
1010

1111
error: this looks like you are swapping elements of `foo` manually
12-
--> $DIR/swap.rs:17:5
12+
--> $DIR/swap.rs:36:5
1313
|
1414
LL | / let temp = foo[0];
1515
LL | | foo[0] = foo[1];
1616
LL | | foo[1] = temp;
1717
| |_________________^ help: try: `foo.swap(0, 1)`
1818

1919
error: this looks like you are swapping elements of `foo` manually
20-
--> $DIR/swap.rs:26:5
20+
--> $DIR/swap.rs:45:5
2121
|
2222
LL | / let temp = foo[0];
2323
LL | | foo[0] = foo[1];
2424
LL | | foo[1] = temp;
2525
| |_________________^ help: try: `foo.swap(0, 1)`
2626

2727
error: this looks like you are swapping `a` and `b` manually
28-
--> $DIR/swap.rs:45:7
28+
--> $DIR/swap.rs:65:7
2929
|
3030
LL | ; let t = a;
3131
| _______^
@@ -36,7 +36,7 @@ LL | | b = t;
3636
= note: or maybe you should use `std::mem::replace`?
3737

3838
error: this looks like you are swapping `c.0` and `a` manually
39-
--> $DIR/swap.rs:54:7
39+
--> $DIR/swap.rs:74:7
4040
|
4141
LL | ; let t = c.0;
4242
| _______^
@@ -47,7 +47,7 @@ LL | | a = t;
4747
= note: or maybe you should use `std::mem::replace`?
4848

4949
error: this looks like you are trying to swap `a` and `b`
50-
--> $DIR/swap.rs:42:5
50+
--> $DIR/swap.rs:62:5
5151
|
5252
LL | / a = b;
5353
LL | | b = a;
@@ -57,7 +57,7 @@ LL | | b = a;
5757
= note: or maybe you should use `std::mem::replace`?
5858

5959
error: this looks like you are trying to swap `c.0` and `a`
60-
--> $DIR/swap.rs:51:5
60+
--> $DIR/swap.rs:71:5
6161
|
6262
LL | / c.0 = a;
6363
LL | | a = c.0;

0 commit comments

Comments
 (0)