about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2019-09-11 09:48:25 +0000
committerbors <bors@rust-lang.org>2019-09-11 09:48:25 +0000
commit6ca5b2053a6d48d0cf10af1dbbb9b79c9704707a (patch)
tree26110a31a52ffbf3a2ffb37cb8a5b966f08f05a0
parentdfa88c68419177c1fa55dba7dbd585564b794e12 (diff)
parent9041856ab9785c8dd97aa47be8798e4126fbd504 (diff)
downloadrust-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.rs8
-rw-r--r--tests/ui/swap.rs20
-rw-r--r--tests/ui/swap.stderr14
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;