Consider auto-deref when linting `manual_swap`

diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index 54d470ca738201c0a783cc6599de3bf42eb4c32b..8c9e229359781adafd94481841d64597608f3f4f 100644 (file)
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -621,6 +621,19 @@ fn projection_stack<'a, 'hir>(mut e: &'a Expr<'hir>) -> (Vec<&'a Expr<'hir>>, &'
     (result, root)
 }
 
+/// Gets the mutability of the custom deref adjustment, if any.
+pub fn expr_custom_deref_adjustment(cx: &LateContext<'_>, e: &Expr<'_>) -> Option<Mutability> {
+    cx.typeck_results()
+        .expr_adjustments(e)
+        .iter()
+        .find_map(|a| match a.kind {
+            Adjust::Deref(Some(d)) => Some(Some(d.mutbl)),
+            Adjust::Deref(None) => None,
+            _ => Some(None),
+        })
+        .and_then(|x| x)
+}
+
 /// Checks if two expressions can be mutably borrowed simultaneously
 /// and they aren't dependent on borrowing same thing twice
 pub fn can_mut_borrow_both(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>) -> bool {
@@ -629,7 +642,15 @@ pub fn can_mut_borrow_both(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>) -
     if !eq_expr_value(cx, r1, r2) {
         return true;
     }
+    if expr_custom_deref_adjustment(cx, r1).is_some() || expr_custom_deref_adjustment(cx, r2).is_some() {
+        return false;
+    }
+
     for (x1, x2) in s1.iter().zip(s2.iter()) {
+        if expr_custom_deref_adjustment(cx, x1).is_some() || expr_custom_deref_adjustment(cx, x2).is_some() {
+            return false;
+        }
+
         match (&x1.kind, &x2.kind) {
             (ExprKind::Field(_, i1), ExprKind::Field(_, i2)) => {
                 if i1 != i2 {

diff --git a/tests/ui/swap.fixed b/tests/ui/swap.fixed
index ef518359ec5f373614727442c65526c05a5c1899..3329efbd4ff429f1daabef797cf9793513b010c0 100644 (file)
--- a/tests/ui/swap.fixed
+++ b/tests/ui/swap.fixed
@@ -122,3 +122,36 @@ fn main() {
 
     ; std::mem::swap(&mut c.0, &mut a);
 }
+
+fn issue_8154() {
+    struct S1 {
+        x: i32,
+        y: i32,
+    }
+    struct S2(S1);
+    struct S3<'a, 'b>(&'a mut &'b mut S1);
+
+    impl std::ops::Deref for S2 {
+        type Target = S1;
+        fn deref(&self) -> &Self::Target {
+            &self.0
+        }
+    }
+    impl std::ops::DerefMut for S2 {
+        fn deref_mut(&mut self) -> &mut Self::Target {
+            &mut self.0
+        }
+    }
+
+    // Don't lint. `s.0` is mutably borrowed by `s.x` and `s.y` via the deref impl.
+    let mut s = S2(S1 { x: 0, y: 0 });
+    let t = s.x;
+    s.x = s.y;
+    s.y = t;
+
+    // Accessing through a mutable reference is fine
+    let mut s = S1 { x: 0, y: 0 };
+    let mut s = &mut s;
+    let s = S3(&mut s);
+    std::mem::swap(&mut s.0.x, &mut s.0.y);
+}

diff --git a/tests/ui/swap.rs b/tests/ui/swap.rs
index 8518659ccf316dbbf41ce2e9941b3874c5878092..8179ac1f2ab028a7a5d5eb26a0adcb6a24c811c7 100644 (file)
--- a/tests/ui/swap.rs
+++ b/tests/ui/swap.rs
@@ -144,3 +144,38 @@ fn main() {
     c.0 = a;
     a = t;
 }
+
+fn issue_8154() {
+    struct S1 {
+        x: i32,
+        y: i32,
+    }
+    struct S2(S1);
+    struct S3<'a, 'b>(&'a mut &'b mut S1);
+
+    impl std::ops::Deref for S2 {
+        type Target = S1;
+        fn deref(&self) -> &Self::Target {
+            &self.0
+        }
+    }
+    impl std::ops::DerefMut for S2 {
+        fn deref_mut(&mut self) -> &mut Self::Target {
+            &mut self.0
+        }
+    }
+
+    // Don't lint. `s.0` is mutably borrowed by `s.x` and `s.y` via the deref impl.
+    let mut s = S2(S1 { x: 0, y: 0 });
+    let t = s.x;
+    s.x = s.y;
+    s.y = t;
+
+    // Accessing through a mutable reference is fine
+    let mut s = S1 { x: 0, y: 0 };
+    let mut s = &mut s;
+    let s = S3(&mut s);
+    let t = s.0.x;
+    s.0.x = s.0.y;
+    s.0.y = t;
+}

diff --git a/tests/ui/swap.stderr b/tests/ui/swap.stderr
index 614d16ced40f1b5fd02cf899f86f5a0f38a9b6a5..2b556b475cee9da92ba55ebc79139ff980231ed0 100644 (file)
--- a/tests/ui/swap.stderr
+++ b/tests/ui/swap.stderr
@@ -108,5 +108,15 @@ LL | |     a = c.0;
    |
    = note: or maybe you should use `std::mem::replace`?
 
-error: aborting due to 12 previous errors
+error: this looks like you are swapping `s.0.x` and `s.0.y` manually
+  --> $DIR/swap.rs:178:5
+   |
+LL | /     let t = s.0.x;
+LL | |     s.0.x = s.0.y;
+LL | |     s.0.y = t;
+   | |_____________^ help: try: `std::mem::swap(&mut s.0.x, &mut s.0.y)`
+   |
+   = note: or maybe you should use `std::mem::replace`?
+
+error: aborting due to 13 previous errors