From 0ef5dee3b8c8e564676faab09039cb8efd7b7e9c Mon Sep 17 00:00:00 2001
From: Sean Klein <seanmarionklein@gmail.com>
Date: Sun, 19 Apr 2020 10:49:12 -0400
Subject: [PATCH] Fix issue #2907.

Update the "borrow box" lint to avoid recommending the following
conversion:

```
  // Old
  pub fn f(&mut Box<T>) {...}

  // New
  pub fn f(&mut T) {...}
```

Given a mutable reference to a box, functions may want to change
"which" object the Box is pointing at.

This change avoids recommending removing the "Box" parameter
for mutable references.
---
 clippy_lints/src/types.rs  | 14 +++++++-------
 tests/ui/borrow_box.rs     | 18 ++++++++++++++++++
 tests/ui/borrow_box.stderr | 18 ++++++------------
 3 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs
index 31d8daa2d977..153a2b7249c1 100644
--- a/clippy_lints/src/types.rs
+++ b/clippy_lints/src/types.rs
@@ -531,11 +531,12 @@ impl Types {
                         } else {
                             format!("{} ", lt.name.ident().as_str())
                         };
-                        let mutopt = if mut_ty.mutbl == Mutability::Mut {
-                            "mut "
-                        } else {
-                            ""
-                        };
+
+                        if mut_ty.mutbl == Mutability::Mut {
+                            // Ignore `&mut Box<T>` types; see issue #2907 for
+                            // details.
+                            return;
+                        }
                         let mut applicability = Applicability::MachineApplicable;
                         span_lint_and_sugg(
                             cx,
@@ -544,9 +545,8 @@ impl Types {
                             "you seem to be trying to use `&Box<T>`. Consider using just `&T`",
                             "try",
                             format!(
-                                "&{}{}{}",
+                                "&{}{}",
                                 ltopt,
-                                mutopt,
                                 &snippet_with_applicability(cx, inner.span, "..", &mut applicability)
                             ),
                             Applicability::Unspecified,
diff --git a/tests/ui/borrow_box.rs b/tests/ui/borrow_box.rs
index b30f7290ffea..1901de46ca89 100644
--- a/tests/ui/borrow_box.rs
+++ b/tests/ui/borrow_box.rs
@@ -4,6 +4,14 @@
 #![allow(dead_code)]
 
 pub fn test1(foo: &mut Box<bool>) {
+    // Although this function could be changed to "&mut bool",
+    // avoiding the Box, mutable references to boxes are not
+    // flagged by this lint.
+    //
+    // This omission is intentional: By passing a mutable Box,
+    // the memory location of the pointed-to object could be
+    // modified. By passing a mutable reference, the contents
+    // could change, but not the location.
     println!("{:?}", foo)
 }
 
@@ -71,6 +79,16 @@ impl<'a> Test12 for Test11<'a> {
     }
 }
 
+pub fn test13(boxed_slice: &mut Box<[i32]>) {
+    // Unconditionally replaces the box pointer.
+    //
+    // This cannot be accomplished if "&mut [i32]" is passed,
+    // and provides a test case where passing a reference to
+    // a Box is valid.
+    let mut data = vec![12];
+    *boxed_slice = data.into_boxed_slice();
+}
+
 fn main() {
     test1(&mut Box::new(false));
     test2();
diff --git a/tests/ui/borrow_box.stderr b/tests/ui/borrow_box.stderr
index 89171ddd7c6a..b5db691f89f3 100644
--- a/tests/ui/borrow_box.stderr
+++ b/tests/ui/borrow_box.stderr
@@ -1,8 +1,8 @@
 error: you seem to be trying to use `&Box<T>`. Consider using just `&T`
-  --> $DIR/borrow_box.rs:6:19
+  --> $DIR/borrow_box.rs:19:14
    |
-LL | pub fn test1(foo: &mut Box<bool>) {
-   |                   ^^^^^^^^^^^^^^ help: try: `&mut bool`
+LL |     let foo: &Box<bool>;
+   |              ^^^^^^^^^^ help: try: `&bool`
    |
 note: the lint level is defined here
   --> $DIR/borrow_box.rs:1:9
@@ -11,22 +11,16 @@ LL | #![deny(clippy::borrowed_box)]
    |         ^^^^^^^^^^^^^^^^^^^^
 
 error: you seem to be trying to use `&Box<T>`. Consider using just `&T`
-  --> $DIR/borrow_box.rs:11:14
-   |
-LL |     let foo: &Box<bool>;
-   |              ^^^^^^^^^^ help: try: `&bool`
-
-error: you seem to be trying to use `&Box<T>`. Consider using just `&T`
-  --> $DIR/borrow_box.rs:15:10
+  --> $DIR/borrow_box.rs:23:10
    |
 LL |     foo: &'a Box<bool>,
    |          ^^^^^^^^^^^^^ help: try: `&'a bool`
 
 error: you seem to be trying to use `&Box<T>`. Consider using just `&T`
-  --> $DIR/borrow_box.rs:19:17
+  --> $DIR/borrow_box.rs:27:17
    |
 LL |     fn test4(a: &Box<bool>);
    |                 ^^^^^^^^^^ help: try: `&bool`
 
-error: aborting due to 4 previous errors
+error: aborting due to 3 previous errors