Implement NOOP_METHOD_CALL lint

compiler/rustc_codegen_ssa/src/back/rpath.rs

@@ -24,7 +24,7 @@ pub fn get_rpath_flags(config: &mut RPathConfig<'_>) -> Vec<String> {
 
     debug!("preparing the RPATH!");
 
-    let libs = config.used_crates.clone();
+    let libs = config.used_crates;
     let libs = libs.iter().filter_map(|&(_, ref l)| l.option()).collect::<Vec<_>>();
     let rpaths = get_rpaths(config, &libs);
     let mut flags = rpaths_to_flags(&rpaths);

compiler/rustc_lint/src/lib.rs

@@ -57,6 +57,7 @@ mod methods;
 mod non_ascii_idents;
 mod non_fmt_panic;
 mod nonstandard_style;
+mod noop_method_call;
 mod passes;
 mod redundant_semicolon;
 mod traits;
@@ -83,6 +84,7 @@ use methods::*;
 use non_ascii_idents::*;
 use non_fmt_panic::NonPanicFmt;
 use nonstandard_style::*;
+use noop_method_call::*;
 use redundant_semicolon::*;
 use traits::*;
 use types::*;
@@ -170,6 +172,7 @@ macro_rules! late_lint_passes {
                 DropTraitConstraints: DropTraitConstraints,
                 TemporaryCStringAsPtr: TemporaryCStringAsPtr,
                 NonPanicFmt: NonPanicFmt,
+                NoopMethodCall: NoopMethodCall,
             ]
         );
     };

compiler/rustc_lint/src/noop_method_call.rs

@@ -0,0 +1,111 @@
+use crate::context::LintContext;
+use crate::rustc_middle::ty::TypeFoldable;
+use crate::LateContext;
+use crate::LateLintPass;
+use rustc_hir::def::DefKind;
+use rustc_hir::{Expr, ExprKind};
+use rustc_middle::ty;
+use rustc_span::symbol::sym;
+
+declare_lint! {
+    /// The `noop_method_call` lint detects specific calls to noop methods
+    /// such as a calling `<&T as Clone>::clone` where `T: !Clone`.
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// # #![allow(unused)]
+    /// #![warn(noop_method_call)]
+    /// struct Foo;
+    /// let foo = &Foo;
+    /// let clone: &Foo = foo.clone();
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Some method calls are noops meaning that they do nothing. Usually such methods
+    /// are the result of blanket implementations that happen to create some method invocations
+    /// that end up not doing anything. For instance, `Clone` is implemented on all `&T`, but
+    /// 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,
+    Allow,
+    "detects the use of well-known noop methods"
+}
+
+declare_lint_pass!(NoopMethodCall => [NOOP_METHOD_CALL]);
+
+impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        // We only care about method calls.
+        let (call, elements) = match expr.kind {
+            ExprKind::MethodCall(call, _, elements, _) => (call, elements),
+            _ => return,
+        };
+        // We only care about method calls corresponding to the `Clone`, `Deref` and `Borrow`
+        // traits and ignore any other method call.
+        let (trait_id, did) = match cx.typeck_results().type_dependent_def(expr.hir_id) {
+            // Verify we are dealing with a method/associated function.
+            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, sym::Deref, sym::Borrow]
+                        .iter()
+                        .any(|s| cx.tcx.is_diagnostic_item(*s, trait_id)) =>
+                {
+                    (trait_id, did)
+                }
+                _ => return,
+            },
+            _ => return,
+        };
+        let substs = cx.typeck_results().node_substs(expr.hir_id);
+        if substs.needs_subst() {
+            // We can't resolve on types that require monomorphization, so we don't handle them if
+            // we need to perfom substitution.
+            return;
+        }
+        let param_env = cx.tcx.param_env(trait_id);
+        // Resolve the trait method instance.
+        let i = match ty::Instance::resolve(cx.tcx, param_env, did, substs) {
+            Ok(Some(i)) => i,
+            _ => return,
+        };
+        // (Re)check that it implements the noop diagnostic.
+        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 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 \
+                    // type are the same, implying that the method call is unnecessary.
+                    return;
+                }
+                let expr_span = expr.span;
+                let note = format!(
+                    "the type `{:?}` which `{}` is being called on is the same as \
+                     the type returned from `{}`, so the method call does not do \
+                     anything and can be removed",
+                    receiver_ty, method, method,
+                );
+
+                let span = expr_span.with_lo(receiver.span.hi());
+                cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| {
+                    let method = &call.ident.name;
+                    let message = format!(
+                        "call to `.{}()` on a reference in this situation does nothing",
+                        &method,
+                    );
+                    lint.build(&message)
+                        .span_label(span, "unnecessary method call")
+                        .note(&note)
+                        .emit()
+                });
+            }
+        }
+    }
+}

compiler/rustc_middle/src/ty/error.rs

@@ -12,7 +12,6 @@ use rustc_target::spec::abi;
 
 use std::borrow::Cow;
 use std::fmt;
-use std::ops::Deref;
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, TypeFoldable)]
 pub struct ExpectedFound<T> {
@@ -548,7 +547,6 @@ impl<T> Trait<T> for X {
             TargetFeatureCast(def_id) => {
                 let attrs = self.get_attrs(*def_id);
                 let target_spans = attrs
-                    .deref()
                     .iter()
                     .filter(|attr| attr.has_name(sym::target_feature))
                     .map(|attr| attr.span);

compiler/rustc_mir/src/borrow_check/invalidation.rs

@@ -165,7 +165,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> {
                 self.consume_operand(location, value);
 
                 // Invalidate all borrows of local places
-                let borrow_set = self.borrow_set.clone();
+                let borrow_set = self.borrow_set;
                 let resume = self.location_table.start_index(resume.start_location());
                 for (i, data) in borrow_set.iter_enumerated() {
                     if borrow_of_local_data(data.borrowed_place) {
@@ -177,7 +177,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> {
             }
             TerminatorKind::Resume | TerminatorKind::Return | TerminatorKind::GeneratorDrop => {
                 // Invalidate all borrows of local places
-                let borrow_set = self.borrow_set.clone();
+                let borrow_set = self.borrow_set;
                 let start = self.location_table.start_index(location);
                 for (i, data) in borrow_set.iter_enumerated() {
                     if borrow_of_local_data(data.borrowed_place) {
@@ -369,15 +369,15 @@ impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> {
         );
         let tcx = self.tcx;
         let body = self.body;
-        let borrow_set = self.borrow_set.clone();
+        let borrow_set = self.borrow_set;
         let indices = self.borrow_set.indices();
         each_borrow_involving_path(
             self,
             tcx,
             body,
             location,
             (sd, place),
-            &borrow_set.clone(),
+            borrow_set,
             indices,
             |this, borrow_index, borrow| {
                 match (rw, borrow.kind) {

compiler/rustc_mir_build/src/build/matches/test.rs

@@ -51,7 +51,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
 
             PatKind::Constant { value } => Test {
                 span: match_pair.pattern.span,
-                kind: TestKind::Eq { value, ty: match_pair.pattern.ty.clone() },
+                kind: TestKind::Eq { value, ty: match_pair.pattern.ty },
             },
 
             PatKind::Range(range) => {

compiler/rustc_span/src/symbol.rs

@@ -129,6 +129,7 @@ symbols! {
         BTreeMap,
         BTreeSet,
         BinaryHeap,
+        Borrow,
         C,
         CString,
         Center,
@@ -141,6 +142,7 @@ symbols! {
         Decodable,
         Decoder,
         Default,
+        Deref,
         Encodable,
         Encoder,
         Eq,
@@ -789,6 +791,9 @@ symbols! {
         none_error,
         nontemporal_store,
         nontrapping_dash_fptoint: "nontrapping-fptoint",
+        noop_method_borrow,
+        noop_method_clone,
+        noop_method_deref,
         noreturn,
         nostack,
         not,

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

@@ -819,7 +819,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                 sig.decl
                     .inputs
                     .iter()
-                    .map(|arg| match arg.clone().kind {
+                    .map(|arg| match arg.kind {
                         hir::TyKind::Tup(ref tys) => ArgKind::Tuple(
                             Some(arg.span),
                             vec![("_".to_owned(), "_".to_owned()); tys.len()],

compiler/rustc_traits/src/chalk/mod.rs

@@ -165,7 +165,7 @@ crate fn evaluate_goal<'tcx>(
                         // let's just ignore that
                         let sol = Canonical {
                             max_universe: ty::UniverseIndex::from_usize(0),
-                            variables: obligation.variables.clone(),
+                            variables: obligation.variables,
                             value: QueryResponse {
                                 var_values: CanonicalVarValues { var_values: IndexVec::new() }
                                     .make_identity(tcx),

compiler/rustc_typeck/src/check/callee.rs

@@ -465,7 +465,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let expected_arg_tys = self.expected_inputs_for_expected_output(
             call_expr.span,
             expected,
-            fn_sig.output().clone(),
+            fn_sig.output(),
             fn_sig.inputs(),
         );
 

compiler/rustc_typeck/src/check/expr.rs

@@ -711,7 +711,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         });
 
         let ret_ty = ret_coercion.borrow().expected_ty();
-        let return_expr_ty = self.check_expr_with_hint(return_expr, ret_ty.clone());
+        let return_expr_ty = self.check_expr_with_hint(return_expr, ret_ty);
         ret_coercion.borrow_mut().coerce(
             self,
             &self.cause(return_expr.span, ObligationCauseCode::ReturnValue(return_expr.hir_id)),

library/alloc/src/collections/btree/map/tests.rs

@@ -1801,11 +1801,11 @@ fn test_occupied_entry_key() {
     let key = "hello there";
     let value = "value goes here";
     assert!(a.is_empty());
-    a.insert(key.clone(), value.clone());
+    a.insert(key, value);
     assert_eq!(a.len(), 1);
     assert_eq!(a[key], value);
 
-    match a.entry(key.clone()) {
+    match a.entry(key) {
         Vacant(_) => panic!(),
         Occupied(e) => assert_eq!(key, *e.key()),
     }
@@ -1821,11 +1821,11 @@ fn test_vacant_entry_key() {
     let value = "value goes here";
 
     assert!(a.is_empty());
-    match a.entry(key.clone()) {
+    match a.entry(key) {
         Occupied(_) => panic!(),
         Vacant(e) => {
             assert_eq!(key, *e.key());
-            e.insert(value.clone());
+            e.insert(value);
         }
     }
     assert_eq!(a.len(), 1);

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/clone.rs

@@ -104,12 +104,14 @@
 /// [impls]: #implementors
 #[stable(feature = "rust1", since = "1.0.0")]
 #[lang = "clone"]
+#[rustc_diagnostic_item = "Clone"]
 pub trait Clone: Sized {
     /// Returns a copy of the value.
     ///
     /// # Examples
     ///
     /// ```
+    /// # #![allow(noop_method_call)]
     /// let hello = "Hello"; // &str implements Clone
     ///
     /// assert_eq!("Hello", hello.clone());
@@ -221,6 +223,7 @@ mod impls {
     #[stable(feature = "rust1", since = "1.0.0")]
     impl<T: ?Sized> Clone for &T {
         #[inline]
+        #[rustc_diagnostic_item = "noop_method_clone"]
         fn clone(&self) -> Self {
             *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/iter/adapters/intersperse.rs

@@ -9,7 +9,7 @@ fn test_intersperse() {
     assert_eq!(v, vec![1]);
 
     let xs = ["a", "", "b", "c"];
-    let v: Vec<&str> = xs.iter().map(|x| x.clone()).intersperse(", ").collect();
+    let v: Vec<&str> = xs.iter().map(|x| *x).intersperse(", ").collect();
     let text: String = v.concat();
     assert_eq!(text, "a, , b, c".to_string());
 
@@ -24,7 +24,7 @@ fn test_intersperse_size_hint() {
     assert_eq!(iter.size_hint(), (0, Some(0)));
 
     let xs = ["a", "", "b", "c"];
-    let mut iter = xs.iter().map(|x| x.clone()).intersperse(", ");
+    let mut iter = xs.iter().map(|x| *x).intersperse(", ");
     assert_eq!(iter.size_hint(), (7, Some(7)));
 
     assert_eq!(iter.next(), Some("a"));

library/std/src/collections/hash/map/tests.rs

@@ -774,11 +774,11 @@ fn test_occupied_entry_key() {
     let key = "hello there";
     let value = "value goes here";
     assert!(a.is_empty());
-    a.insert(key.clone(), value.clone());
+    a.insert(key, value);
     assert_eq!(a.len(), 1);
     assert_eq!(a[key], value);
 
-    match a.entry(key.clone()) {
+    match a.entry(key) {
         Vacant(_) => panic!(),
         Occupied(e) => assert_eq!(key, *e.key()),
     }
@@ -793,11 +793,11 @@ fn test_vacant_entry_key() {
     let value = "value goes here";
 
     assert!(a.is_empty());
-    match a.entry(key.clone()) {
+    match a.entry(key) {
         Occupied(_) => panic!(),
         Vacant(e) => {
             assert_eq!(key, *e.key());
-            e.insert(value.clone());
+            e.insert(value);
         }
     }
     assert_eq!(a.len(), 1);