Fix borrow and deref

Author: rylev

compiler/rustc_lint/src/noop_method_call.rs

@@ -15,6 +15,7 @@ declare_lint! {
     ///
     /// ```rust
     /// # #![allow(unused)]
+    /// #![deny(noop_method_call)]
     /// struct Foo;
     /// let foo = &Foo;
     /// let clone: &Foo = foo.clone();
@@ -30,7 +31,7 @@ declare_lint! {
     /// calling `clone` on a `&T` where `T` does not implement clone, actually doesn't do anything
     /// as references are copy. This lint detects these calls and warns the user about them.
     pub NOOP_METHOD_CALL,
-    Warn,
+    Allow,
     "detects the use of well-known noop methods"
 }
 
@@ -50,7 +51,9 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
             Some((DefKind::AssocFn, did)) => match cx.tcx.trait_of_item(did) {
                 // Check that we're dealing with a trait method for one of the traits we care about.
                 Some(trait_id)
-                    if [sym::Clone].iter().any(|s| cx.tcx.is_diagnostic_item(*s, trait_id)) =>
+                    if [sym::Clone, sym::Deref, sym::Borrow]
+                        .iter()
+                        .any(|s| cx.tcx.is_diagnostic_item(*s, trait_id)) =>
                 {
                     (trait_id, did)
                 }
@@ -71,20 +74,11 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
             _ => return,
         };
         // (Re)check that it implements the noop diagnostic.
-        for (s, peel_ref) in [(sym::noop_method_clone, false)].iter() {
+        for s in [sym::noop_method_clone, sym::noop_method_deref, sym::noop_method_borrow].iter() {
             if cx.tcx.is_diagnostic_item(*s, i.def_id()) {
                 let method = &call.ident.name;
                 let receiver = &elements[0];
                 let receiver_ty = cx.typeck_results().expr_ty(receiver);
-                let receiver_ty = match receiver_ty.kind() {
-                    // Remove one borrow from the receiver if appropriate to positively verify that
-                    // the receiver `&self` type and the return type are the same, depending on the
-                    // involved trait being checked.
-                    ty::Ref(_, ty, _) if *peel_ref => ty,
-                    // When it comes to `Clone` we need to check the `receiver_ty` directly.
-                    // FIXME: we must come up with a better strategy for this.
-                    _ => receiver_ty,
-                };
                 let expr_ty = cx.typeck_results().expr_ty_adjusted(expr);
                 if receiver_ty != expr_ty {
                     // This lint will only trigger if the receiver type and resulting expression \

compiler/rustc_span/src/symbol.rs

@@ -142,6 +142,7 @@ symbols! {
         Decodable,
         Decoder,
         Default,
+        Deref,
         Encodable,
         Encoder,
         Eq,
@@ -790,7 +791,9 @@ symbols! {
         none_error,
         nontemporal_store,
         nontrapping_dash_fptoint: "nontrapping-fptoint",
+        noop_method_borrow,
         noop_method_clone,
+        noop_method_deref,
         noreturn,
         nostack,
         not,

library/core/src/borrow.rs

@@ -153,6 +153,7 @@
 /// [`HashMap<K, V>`]: ../../std/collections/struct.HashMap.html
 /// [`String`]: ../../std/string/struct.String.html
 #[stable(feature = "rust1", since = "1.0.0")]
+#[rustc_diagnostic_item = "Borrow"]
 pub trait Borrow<Borrowed: ?Sized> {
     /// Immutably borrows from an owned value.
     ///
@@ -205,6 +206,7 @@ pub trait BorrowMut<Borrowed: ?Sized>: Borrow<Borrowed> {
 
 #[stable(feature = "rust1", since = "1.0.0")]
 impl<T: ?Sized> Borrow<T> for T {
+    #[rustc_diagnostic_item = "noop_method_borrow"]
     fn borrow(&self) -> &T {
         self
     }

library/core/src/ops/deref.rs

@@ -60,6 +60,7 @@
 #[doc(alias = "*")]
 #[doc(alias = "&*")]
 #[stable(feature = "rust1", since = "1.0.0")]
+#[rustc_diagnostic_item = "Deref"]
 pub trait Deref {
     /// The resulting type after dereferencing.
     #[stable(feature = "rust1", since = "1.0.0")]
@@ -78,6 +79,7 @@ pub trait Deref {
 impl<T: ?Sized> Deref for &T {
     type Target = T;
 
+    #[rustc_diagnostic_item = "noop_method_deref"]
     fn deref(&self) -> &T {
         *self
     }

library/core/tests/clone.rs

@@ -1,5 +1,3 @@
-#![cfg_attr(not(bootstrap), allow(noop_method_call))]
-
 #[test]
 fn test_borrowed_clone() {
     let x = 5;

src/test/ui/issues/issue-11820.rs

@@ -1,8 +1,6 @@
 // run-pass
 // pretty-expanded FIXME #23616
 
-#![allow(noop_method_call)]
-
 struct NoClone;
 
 fn main() {

src/test/ui/lint/noop-method-call.rs

@@ -1,15 +1,19 @@
 // check-pass
 
 #![allow(unused)]
+#![warn(noop_method_call)]
 
-struct NonCloneType<T>(T);
+use std::borrow::Borrow;
+use std::ops::Deref;
+
+struct PlainType<T>(T);
 
 #[derive(Clone)]
 struct CloneType<T>(T);
 
 fn main() {
-    let non_clone_type_ref = &NonCloneType(1u32);
-    let non_clone_type_ref_clone: &NonCloneType<u32> = non_clone_type_ref.clone();
+    let non_clone_type_ref = &PlainType(1u32);
+    let non_clone_type_ref_clone: &PlainType<u32> = non_clone_type_ref.clone();
     //~^ WARNING call to `.clone()` on a reference in this situation does nothing
 
     let clone_type_ref = &CloneType(1u32);
@@ -20,15 +24,31 @@ fn main() {
     let clone_type_ref = &&CloneType(1u32);
     let clone_type_ref_clone: &CloneType<u32> = clone_type_ref.clone();
 
+    let non_deref_type = &PlainType(1u32);
+    let non_deref_type_deref: &PlainType<u32> = non_deref_type.deref();
+    //~^ WARNING call to `.deref()` on a reference in this situation does nothing
+
+    // Dereferencing a &&T does not warn since it has collapsed the double reference
+    let non_deref_type = &&PlainType(1u32);
+    let non_deref_type_deref: &PlainType<u32> = non_deref_type.deref();
+
+    let non_borrow_type = &PlainType(1u32);
+    let non_borrow_type_borrow: &PlainType<u32> = non_borrow_type.borrow();
+    //~^ WARNING call to `.borrow()` on a reference in this situation does nothing
+
+    // Borrowing a &&T does not warn since it has collapsed the double reference
+    let non_borrow_type = &&PlainType(1u32);
+    let non_borrow_type_borrow: &PlainType<u32> = non_borrow_type.borrow();
+
     let xs = ["a", "b", "c"];
     let _v: Vec<&str> = xs.iter().map(|x| x.clone()).collect(); // ok, but could use `*x` instead
 }
 
-fn generic<T>(non_clone_type: &NonCloneType<T>) {
+fn generic<T>(non_clone_type: &PlainType<T>) {
     non_clone_type.clone();
 }
 
-fn non_generic(non_clone_type: &NonCloneType<u32>) {
+fn non_generic(non_clone_type: &PlainType<u32>) {
     non_clone_type.clone();
     //~^ WARNING call to `.clone()` on a reference in this situation does nothing
 }

src/test/ui/lint/noop-method-call.stderr

@@ -1,19 +1,39 @@
 warning: call to `.clone()` on a reference in this situation does nothing
-  --> $DIR/noop-method-call.rs:12:74
+  --> $DIR/noop-method-call.rs:16:71
    |
-LL |     let non_clone_type_ref_clone: &NonCloneType<u32> = non_clone_type_ref.clone();
-   |                                                                          ^^^^^^^^ unnecessary method call
+LL |     let non_clone_type_ref_clone: &PlainType<u32> = non_clone_type_ref.clone();
+   |                                                                       ^^^^^^^^ unnecessary method call
    |
-   = note: `#[warn(noop_method_call)]` on by default
-   = note: the type `&NonCloneType<u32>` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed
+note: the lint level is defined here
+  --> $DIR/noop-method-call.rs:4:9
+   |
+LL | #![warn(noop_method_call)]
+   |         ^^^^^^^^^^^^^^^^
+   = note: the type `&PlainType<u32>` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed
+
+warning: call to `.deref()` on a reference in this situation does nothing
+  --> $DIR/noop-method-call.rs:28:63
+   |
+LL |     let non_deref_type_deref: &PlainType<u32> = non_deref_type.deref();
+   |                                                               ^^^^^^^^ unnecessary method call
+   |
+   = note: the type `&PlainType<u32>` which `deref` is being called on is the same as the type returned from `deref`, so the method call does not do anything and can be removed
+
+warning: call to `.borrow()` on a reference in this situation does nothing
+  --> $DIR/noop-method-call.rs:36:66
+   |
+LL |     let non_borrow_type_borrow: &PlainType<u32> = non_borrow_type.borrow();
+   |                                                                  ^^^^^^^^^ unnecessary method call
+   |
+   = note: the type `&PlainType<u32>` which `borrow` is being called on is the same as the type returned from `borrow`, so the method call does not do anything and can be removed
 
 warning: call to `.clone()` on a reference in this situation does nothing
-  --> $DIR/noop-method-call.rs:32:19
+  --> $DIR/noop-method-call.rs:52:19
    |
 LL |     non_clone_type.clone();
    |                   ^^^^^^^^ unnecessary method call
    |
-   = note: the type `&NonCloneType<u32>` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed
+   = note: the type `&PlainType<u32>` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed
 
-warning: 2 warnings emitted
+warning: 4 warnings emitted
 

src/test/ui/underscore-imports/cycle.rs

@@ -14,6 +14,5 @@ mod y {
 
 pub fn main() {
     use x::*;
-    #[allow(noop_method_call)]
     (&0).deref();
 }

src/test/ui/underscore-imports/macro-expanded.rs

@@ -3,7 +3,6 @@
 // check-pass
 
 #![feature(decl_macro, rustc_attrs)]
-#![allow(noop_method_call)]
 
 mod x {
     pub use std::ops::Not as _;

src/tools/clippy/tests/ui/unnecessary_clone.rs

@@ -1,7 +1,7 @@
 // does not test any rustfixable lints
 
 #![warn(clippy::clone_on_ref_ptr)]
-#![allow(unused, noop_method_call, clippy::redundant_clone, clippy::unnecessary_wraps)]
+#![allow(unused, clippy::redundant_clone, clippy::unnecessary_wraps)]
 
 use std::cell::RefCell;
 use std::rc::{self, Rc};