diff options
| author | bors <bors@rust-lang.org> | 2019-09-11 09:48:25 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2019-09-11 09:48:25 +0000 |
| commit | 6ca5b2053a6d48d0cf10af1dbbb9b79c9704707a (patch) | |
| tree | 26110a31a52ffbf3a2ffb37cb8a5b966f08f05a0 | |
| parent | dfa88c68419177c1fa55dba7dbd585564b794e12 (diff) | |
| parent | 9041856ab9785c8dd97aa47be8798e4126fbd504 (diff) | |
| download | rust-6ca5b2053a6d48d0cf10af1dbbb9b79c9704707a.tar.gz rust-6ca5b2053a6d48d0cf10af1dbbb9b79c9704707a.zip | |
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
| -rw-r--r-- | clippy_lints/src/swap.rs | 8 | ||||
| -rw-r--r-- | tests/ui/swap.rs | 20 | ||||
| -rw-r--r-- | tests/ui/swap.stderr | 14 |
3 files changed, 35 insertions, 7 deletions
diff --git a/clippy_lints/src/swap.rs b/clippy_lints/src/swap.rs index 9940269e5b3..cde49db2375 100644 --- a/clippy_lints/src/swap.rs +++ b/clippy_lints/src/swap.rs @@ -120,6 +120,14 @@ fn check_manual_swap(cx: &LateContext<'_, '_>, block: &Block) { None } + if let ExprKind::Field(ref lhs1, _) = lhs1.node { + if let ExprKind::Field(ref lhs2, _) = lhs2.node { + if lhs1.hir_id.owner_def_id() == lhs2.hir_id.owner_def_id() { + return; + } + } + } + let (replace, what, sugg) = if let Some((slice, idx1, idx2)) = check_for_slice(cx, lhs1, lhs2) { if let Some(slice) = Sugg::hir_opt(cx, slice) { (false, diff --git a/tests/ui/swap.rs b/tests/ui/swap.rs index 9db8dcbf75e..093cd7fd04a 100644 --- a/tests/ui/swap.rs +++ b/tests/ui/swap.rs @@ -3,6 +3,25 @@ struct Foo(u32); +#[derive(Clone)] +struct Bar { + a: u32, + b: u32, +} + +fn field() { + let mut bar = Bar { a: 1, b: 2 }; + + let temp = bar.a; + bar.a = bar.b; + bar.b = temp; + + let mut baz = vec![bar.clone(), bar.clone()]; + let temp = baz[0].a; + baz[0].a = baz[1].a; + baz[1].a = temp; +} + fn array() { let mut foo = [1, 2]; let temp = foo[0]; @@ -32,6 +51,7 @@ fn vec() { #[rustfmt::skip] fn main() { + field(); array(); slice(); vec(); diff --git a/tests/ui/swap.stderr b/tests/ui/swap.stderr index 5d818cf2056..bda0a0bf38b 100644 --- a/tests/ui/swap.stderr +++ b/tests/ui/swap.stderr @@ -1,5 +1,5 @@ error: this looks like you are swapping elements of `foo` manually - --> $DIR/swap.rs:8:5 + --> $DIR/swap.rs:27:5 | LL | / let temp = foo[0]; LL | | foo[0] = foo[1]; @@ -9,7 +9,7 @@ LL | | foo[1] = temp; = note: `-D clippy::manual-swap` implied by `-D warnings` error: this looks like you are swapping elements of `foo` manually - --> $DIR/swap.rs:17:5 + --> $DIR/swap.rs:36:5 | LL | / let temp = foo[0]; LL | | foo[0] = foo[1]; @@ -17,7 +17,7 @@ LL | | foo[1] = temp; | |_________________^ help: try: `foo.swap(0, 1)` error: this looks like you are swapping elements of `foo` manually - --> $DIR/swap.rs:26:5 + --> $DIR/swap.rs:45:5 | LL | / let temp = foo[0]; LL | | foo[0] = foo[1]; @@ -25,7 +25,7 @@ LL | | foo[1] = temp; | |_________________^ help: try: `foo.swap(0, 1)` error: this looks like you are swapping `a` and `b` manually - --> $DIR/swap.rs:45:7 + --> $DIR/swap.rs:65:7 | LL | ; let t = a; | _______^ @@ -36,7 +36,7 @@ LL | | b = t; = note: or maybe you should use `std::mem::replace`? error: this looks like you are swapping `c.0` and `a` manually - --> $DIR/swap.rs:54:7 + --> $DIR/swap.rs:74:7 | LL | ; let t = c.0; | _______^ @@ -47,7 +47,7 @@ LL | | a = t; = note: or maybe you should use `std::mem::replace`? error: this looks like you are trying to swap `a` and `b` - --> $DIR/swap.rs:42:5 + --> $DIR/swap.rs:62:5 | LL | / a = b; LL | | b = a; @@ -57,7 +57,7 @@ LL | | b = a; = note: or maybe you should use `std::mem::replace`? error: this looks like you are trying to swap `c.0` and `a` - --> $DIR/swap.rs:51:5 + --> $DIR/swap.rs:71:5 | LL | / c.0 = a; LL | | a = c.0; |
