Auto merge of #9674 - smoelius:needless-borrow-fp, r=Jarcho

Fix `needless_borrow` false positive

The PR fixes the false positive exposed by `@BusyJay's` example in: https://github.com/rust-lang/rust-clippy/issues/9111#issuecomment-1277114280

The current approach is described in https://github.com/rust-lang/rust-clippy/pull/9674#issuecomment-1289294201 and https://github.com/rust-lang/rust-clippy/pull/9674#issuecomment-1292225232.

The original approach appears below.

---

The proposed fix is to flag only "simple" trait implementations involving references, a concept
that I introduce next.

Intuitively, a trait implementation is "simple" if all it does is dereference and apply the trait
implementation of a type named by a type parameter. `AsRef` provides a good example of a simple
implementation: https://doc.rust-lang.org/std/convert/trait.AsRef.html#impl-AsRef%3CU%3E-for-%26T

We can make this idea more precise as follows. Given a trait implementation, first determine
whether the implementation is "used defined." If so, then examine its nested obligations.
Consider the implementation simple if-and-only-if:
- there is at least one nested obligation for the same trait
- for each type `X` in the nested obligation's substitution, either `X` is the same as that of
  the original obligation's substitution, or the original type is `&X`

For example, the following implementation from `@BusyJay's` example is "complex" (i.e., not simple)
because it produces no nested obligations:

```rust
impl<'a> Extend<&'a u8> for A { ... }
```

On the other hand, the following slightly modified implementation is simple, because it produces
a nested obligation for `Extend<X>`:

```rust
impl<'a, X> Extend<&'a X> for A where A: Extend<X> { ... }
```

How does flagging only simple implementations help? One way of interpreting the false positive in
`@BusyJay's` example is that it separates a reference from a concrete type. Doing so turns a
successful type inference into a failing one. By flagging only simple implementations, we
separate references from type variables only, thereby eliminating this class of false positives.

Note that `Deref` is a special case, as the obligations generated for it already involve the
underlying type.

r? `@Jarcho` (Sorry to keep pinging you with `needless_borrow` stuff. But my impression is no one knows this code better than you.)

changelog: fix `needless_borrow` false positive

2 files changed, 46 insertions, 0 deletions

diff --git a/tests/ui/needless_borrow.fixed b/tests/ui/needless_borrow.fixed
index 340e89d2db1..57a682d62c0 100644
--- a/tests/ui/needless_borrow.fixed
+++ b/tests/ui/needless_borrow.fixed
@@ -385,3 +385,26 @@ mod used_more_than_once {
     fn use_x(_: impl AsRef<str>) {}
     fn use_x_again(_: impl AsRef<str>) {}
 }
+
+// https://github.com/rust-lang/rust-clippy/issues/9111#issuecomment-1277114280
+#[allow(dead_code)]
+mod issue_9111 {
+    struct A;
+
+    impl Extend<u8> for A {
+        fn extend<T: IntoIterator<Item = u8>>(&mut self, _: T) {
+            unimplemented!()
+        }
+    }
+
+    impl<'a> Extend<&'a u8> for A {
+        fn extend<T: IntoIterator<Item = &'a u8>>(&mut self, _: T) {
+            unimplemented!()
+        }
+    }
+
+    fn main() {
+        let mut a = A;
+        a.extend(&[]); // vs a.extend([]);
+    }
+}
diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs
index c93711ac8e2..0d325b48ab8 100644
--- a/tests/ui/needless_borrow.rs
+++ b/tests/ui/needless_borrow.rs
@@ -385,3 +385,26 @@ mod used_more_than_once {
     fn use_x(_: impl AsRef<str>) {}
     fn use_x_again(_: impl AsRef<str>) {}
 }
+
+// https://github.com/rust-lang/rust-clippy/issues/9111#issuecomment-1277114280
+#[allow(dead_code)]
+mod issue_9111 {
+    struct A;
+
+    impl Extend<u8> for A {
+        fn extend<T: IntoIterator<Item = u8>>(&mut self, _: T) {
+            unimplemented!()
+        }
+    }
+
+    impl<'a> Extend<&'a u8> for A {
+        fn extend<T: IntoIterator<Item = &'a u8>>(&mut self, _: T) {
+            unimplemented!()
+        }
+    }
+
+    fn main() {
+        let mut a = A;
+        a.extend(&[]); // vs a.extend([]);
+    }
+}