From 8bc1ded805ba25db8c926786946b71ea7a0c6ef8 Mon Sep 17 00:00:00 2001 From: Cristian Kubis Date: Sat, 31 Aug 2019 18:50:22 +0200 Subject: [PATCH 1/2] Fix incorrect swap suggestion Clippy suggests using swap on fields belonging to the same owner causing two mutable borrows of the owner Fixes #981 Signed-off-by: Cristian Kubis --- clippy_lints/src/swap.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/clippy_lints/src/swap.rs b/clippy_lints/src/swap.rs index 80c9a33c06a8..79ff183c9ca3 100644 --- a/clippy_lints/src/swap.rs +++ b/clippy_lints/src/swap.rs @@ -118,6 +118,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, From 9041856ab9785c8dd97aa47be8798e4126fbd504 Mon Sep 17 00:00:00 2001 From: Cristian Kubis Date: Mon, 2 Sep 2019 14:15:52 +0200 Subject: [PATCH 2/2] Add more UI tests for swap These tests make sure that the swap warning will not be triggered for expressions that will cause multiple mutable references of the same owner --- tests/ui/swap.rs | 20 ++++++++++++++++++++ tests/ui/swap.stderr | 14 +++++++------- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/tests/ui/swap.rs b/tests/ui/swap.rs index 9db8dcbf75e2..093cd7fd04af 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 5d818cf20566..bda0a0bf38b7 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;