about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJakub Beránek <berykubik@gmail.com>2024-01-01 18:51:45 +0100
committerJakub Beránek <berykubik@gmail.com>2024-03-01 16:35:28 +0100
commit0656d28f6b7d04463d0d5a1d8f87ba2489648dd0 (patch)
tree615fd24d7c9fb5d21c565ee58747ca17958f0d75
parent346b094a11b653954164be019d3c04ae15b7d2b3 (diff)
downloadrust-0656d28f6b7d04463d0d5a1d8f87ba2489648dd0.tar.gz
rust-0656d28f6b7d04463d0d5a1d8f87ba2489648dd0.zip
Add `assigning_clones` lint
-rw-r--r--clippy_lints/src/assigning_clones.rs312
-rw-r--r--clippy_lints/src/declared_lints.rs1
-rw-r--r--clippy_lints/src/lib.rs2
-rw-r--r--tests/ui/assigning_clones.fixed211
-rw-r--r--tests/ui/assigning_clones.rs211
-rw-r--r--tests/ui/assigning_clones.stderr107
6 files changed, 844 insertions, 0 deletions
diff --git a/clippy_lints/src/assigning_clones.rs b/clippy_lints/src/assigning_clones.rs
new file mode 100644
index 00000000000..c1dd068cc04
--- /dev/null
+++ b/clippy_lints/src/assigning_clones.rs
@@ -0,0 +1,312 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::sugg::Sugg;
+use clippy_utils::{is_trait_method, path_to_local};
+use rustc_errors::Applicability;
+use rustc_hir::{self as hir, Expr, ExprKind, Node};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
+use rustc_middle::ty::{Instance, Mutability};
+use rustc_session::declare_lint_pass;
+use rustc_span::def_id::DefId;
+use rustc_span::symbol::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for code like `foo = bar.clone();`
+    ///
+    /// ### Why is this bad?
+    /// Custom `Clone::clone_from()` or `ToOwned::clone_into` implementations allow the objects
+    /// to share resources and therefore avoid allocations.
+    ///
+    /// ### Example
+    /// ```rust
+    /// struct Thing;
+    ///
+    /// impl Clone for Thing {
+    ///     fn clone(&self) -> Self { todo!() }
+    ///     fn clone_from(&mut self, other: &Self) -> Self { todo!() }
+    /// }
+    ///
+    /// pub fn assign_to_ref(a: &mut Thing, b: Thing) {
+    ///     *a = b.clone();
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// struct Thing;
+    /// impl Clone for Thing {
+    ///     fn clone(&self) -> Self { todo!() }
+    ///     fn clone_from(&mut self, other: &Self) -> Self { todo!() }
+    /// }
+    ///
+    /// pub fn assign_to_ref(a: &mut Thing, b: Thing) {
+    ///     a.clone_from(&b);
+    /// }
+    /// ```
+    #[clippy::version = "1.77.0"]
+    pub ASSIGNING_CLONES,
+    perf,
+    "assigning the result of cloning may be inefficient"
+}
+declare_lint_pass!(AssigningClones => [ASSIGNING_CLONES]);
+
+impl<'tcx> LateLintPass<'tcx> for AssigningClones {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, assign_expr: &'tcx hir::Expr<'_>) {
+        let ExprKind::Assign(lhs, rhs, _span) = assign_expr.kind else {
+            return;
+        };
+
+        let Some(call) = extract_call(cx, rhs) else {
+            return;
+        };
+
+        if is_ok_to_suggest(cx, lhs, &call) {
+            suggest(cx, assign_expr, lhs, call);
+        }
+    }
+}
+
+// Try to resolve the call to `Clone::clone` or `ToOwned::to_owned`.
+fn extract_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<CallCandidate<'tcx>> {
+    let Some(fn_def_id) = clippy_utils::fn_def_id(cx, expr) else {
+        return None;
+    };
+
+    // Fast paths to only check method calls without arguments or function calls with a single argument
+    let (target, kind, resolved_method) = match expr.kind {
+        ExprKind::MethodCall(path, receiver, [], _span) => {
+            let args = cx.typeck_results().node_args(expr.hir_id);
+            let resolved_method = Instance::resolve(cx.tcx, cx.param_env, fn_def_id, args);
+            if is_trait_method(cx, expr, sym::Clone) && path.ident.name == sym::clone {
+                (TargetTrait::Clone, CallKind::MethodCall { receiver }, resolved_method)
+            } else if is_trait_method(cx, expr, sym::ToOwned) && path.ident.name == sym!(to_owned) {
+                (TargetTrait::ToOwned, CallKind::MethodCall { receiver }, resolved_method)
+            } else {
+                return None;
+            }
+        },
+        ExprKind::Call(function, args) if args.len() == 1 => {
+            let kind = cx.typeck_results().node_type(function.hir_id).kind();
+            let resolved_method = match kind {
+                ty::FnDef(_, args) => Instance::resolve(cx.tcx, cx.param_env, fn_def_id, args),
+                _ => return None,
+            };
+            if cx.tcx.is_diagnostic_item(sym::to_owned_method, fn_def_id) {
+                (
+                    TargetTrait::ToOwned,
+                    CallKind::FunctionCall { self_arg: &args[0] },
+                    resolved_method,
+                )
+            } else if let Some(trait_did) = cx.tcx.trait_of_item(fn_def_id)
+                && cx.tcx.is_diagnostic_item(sym::Clone, trait_did)
+            {
+                (
+                    TargetTrait::Clone,
+                    CallKind::FunctionCall { self_arg: &args[0] },
+                    resolved_method,
+                )
+            } else {
+                return None;
+            }
+        },
+        _ => return None,
+    };
+    let Ok(Some(resolved_method)) = resolved_method else {
+        // If we could not resolve the method, don't apply the lint
+        return None;
+    };
+
+    Some(CallCandidate {
+        target,
+        kind,
+        method_def_id: resolved_method.def_id(),
+    })
+}
+
+// Return true if we find that the called method has a custom implementation and isn't derived or
+// provided by default by the corresponding trait.
+fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallCandidate<'tcx>) -> bool {
+    // If the left-hand side is a local variable, it might be uninitialized at this point.
+    // In that case we do not want to suggest the lint.
+    if let Some(local) = path_to_local(lhs) {
+        // TODO: This check currently bails if the local variable has no initializer.
+        // That is overly conservative - the lint should fire even if there was no initializer,
+        // but the variable has been initialized before `lhs` was evaluated.
+        if let Some(Node::Local(local)) = cx.tcx.opt_hir_node(cx.tcx.hir().parent_id(local))
+            && local.init.is_none()
+        {
+            return false;
+        }
+    }
+
+    let Some(impl_block) = cx.tcx.impl_of_method(call.method_def_id) else {
+        return false;
+    };
+
+    // If the method implementation comes from #[derive(Clone)], then don't suggest the lint.
+    // Automatically generated Clone impls do not override `clone_from`.
+    // See e.g. https://github.com/rust-lang/rust/pull/98445#issuecomment-1190681305 for more details.
+    if cx.tcx.is_builtin_derived(impl_block) {
+        return false;
+    }
+
+    // Find the function for which we want to check that it is implemented.
+    let provided_fn = match call.target {
+        TargetTrait::Clone => cx.tcx.get_diagnostic_item(sym::Clone).and_then(|clone| {
+            cx.tcx
+                .provided_trait_methods(clone)
+                .find(|item| item.name == sym::clone_from)
+        }),
+        TargetTrait::ToOwned => cx.tcx.get_diagnostic_item(sym::ToOwned).and_then(|to_owned| {
+            cx.tcx
+                .provided_trait_methods(to_owned)
+                .find(|item| item.name == sym!(clone_into))
+        }),
+    };
+    let Some(provided_fn) = provided_fn else {
+        return false;
+    };
+
+    // Now take a look if the impl block defines an implementation for the method that we're interested
+    // in. If not, then we're using a default implementation, which is not interesting, so we will
+    // not suggest the lint.
+    let implemented_fns = cx.tcx.impl_item_implementor_ids(impl_block);
+    implemented_fns.contains_key(&provided_fn.def_id)
+}
+
+fn suggest<'tcx>(
+    cx: &LateContext<'tcx>,
+    assign_expr: &hir::Expr<'tcx>,
+    lhs: &hir::Expr<'tcx>,
+    call: CallCandidate<'tcx>,
+) {
+    span_lint_and_then(cx, ASSIGNING_CLONES, assign_expr.span, call.message(), |diag| {
+        let mut applicability = Applicability::MachineApplicable;
+
+        diag.span_suggestion(
+            assign_expr.span,
+            call.suggestion_msg(),
+            call.suggested_replacement(cx, lhs, &mut applicability),
+            applicability,
+        );
+    });
+}
+
+#[derive(Copy, Clone, Debug)]
+enum CallKind<'tcx> {
+    MethodCall { receiver: &'tcx Expr<'tcx> },
+    FunctionCall { self_arg: &'tcx Expr<'tcx> },
+}
+
+#[derive(Copy, Clone, Debug)]
+enum TargetTrait {
+    Clone,
+    ToOwned,
+}
+
+#[derive(Debug)]
+struct CallCandidate<'tcx> {
+    target: TargetTrait,
+    kind: CallKind<'tcx>,
+    // DefId of the called method from an impl block that implements the target trait
+    method_def_id: DefId,
+}
+
+impl<'tcx> CallCandidate<'tcx> {
+    fn message(&self) -> &'static str {
+        match self.target {
+            TargetTrait::Clone => "assigning the result of `Clone::clone()` may be inefficient",
+            TargetTrait::ToOwned => "assigning the result of `ToOwned::to_owned()` may be inefficient",
+        }
+    }
+
+    fn suggestion_msg(&self) -> &'static str {
+        match self.target {
+            TargetTrait::Clone => "use `clone_from()`",
+            TargetTrait::ToOwned => "use `clone_into()`",
+        }
+    }
+
+    fn suggested_replacement(
+        &self,
+        cx: &LateContext<'tcx>,
+        lhs: &hir::Expr<'tcx>,
+        applicability: &mut Applicability,
+    ) -> String {
+        match self.target {
+            TargetTrait::Clone => {
+                match self.kind {
+                    CallKind::MethodCall { receiver } => {
+                        let receiver_sugg = if let ExprKind::Unary(hir::UnOp::Deref, ref_expr) = lhs.kind {
+                            // `*lhs = self_expr.clone();` -> `lhs.clone_from(self_expr)`
+                            Sugg::hir_with_applicability(cx, ref_expr, "_", applicability)
+                        } else {
+                            // `lhs = self_expr.clone();` -> `lhs.clone_from(self_expr)`
+                            Sugg::hir_with_applicability(cx, lhs, "_", applicability)
+                        }
+                        .maybe_par();
+
+                        // Determine whether we need to reference the argument to clone_from().
+                        let clone_receiver_type = cx.typeck_results().expr_ty(receiver);
+                        let clone_receiver_adj_type = cx.typeck_results().expr_ty_adjusted(receiver);
+                        let mut arg_sugg = Sugg::hir_with_applicability(cx, receiver, "_", applicability);
+                        if clone_receiver_type != clone_receiver_adj_type {
+                            // The receiver may have been a value type, so we need to add an `&` to
+                            // be sure the argument to clone_from will be a reference.
+                            arg_sugg = arg_sugg.addr();
+                        };
+
+                        format!("{receiver_sugg}.clone_from({arg_sugg})")
+                    },
+                    CallKind::FunctionCall { self_arg, .. } => {
+                        let self_sugg = if let ExprKind::Unary(hir::UnOp::Deref, ref_expr) = lhs.kind {
+                            // `*lhs = Clone::clone(self_expr);` -> `Clone::clone_from(lhs, self_expr)`
+                            Sugg::hir_with_applicability(cx, ref_expr, "_", applicability)
+                        } else {
+                            // `lhs = Clone::clone(self_expr);` -> `Clone::clone_from(&mut lhs, self_expr)`
+                            Sugg::hir_with_applicability(cx, lhs, "_", applicability).mut_addr()
+                        };
+                        // The RHS had to be exactly correct before the call, there is no auto-deref for function calls.
+                        let rhs_sugg = Sugg::hir_with_applicability(cx, self_arg, "_", applicability);
+
+                        // TODO: how to get rid of the full path? Modify the function call function's (q)path? Or
+                        // auto-import it the trait?
+                        format!("::std::clone::Clone::clone_from({self_sugg}, {rhs_sugg})")
+                    },
+                }
+            },
+            TargetTrait::ToOwned => {
+                let rhs_sugg = if let ExprKind::Unary(hir::UnOp::Deref, ref_expr) = lhs.kind {
+                    // `*lhs = rhs.to_owned()` -> `rhs.clone_into(lhs)`
+                    // `*lhs = ToOwned::to_owned(rhs)` -> `ToOwned::clone_into(rhs, lhs)`
+                    let sugg = Sugg::hir_with_applicability(cx, ref_expr, "_", applicability).maybe_par();
+                    let inner_type = cx.typeck_results().expr_ty(ref_expr);
+                    // If after unwrapping the dereference, the type is not a mutable reference, we add &mut to make it
+                    // deref to a mutable reference.
+                    if matches!(inner_type.kind(), ty::Ref(_, _, Mutability::Mut)) {
+                        sugg
+                    } else {
+                        sugg.mut_addr()
+                    }
+                } else {
+                    // `lhs = rhs.to_owned()` -> `rhs.clone_into(&mut lhs)`
+                    // `lhs = ToOwned::to_owned(rhs)` -> `ToOwned::clone_into(rhs, &mut lhs)`
+                    Sugg::hir_with_applicability(cx, lhs, "_", applicability)
+                        .maybe_par()
+                        .mut_addr()
+                };
+
+                match self.kind {
+                    CallKind::MethodCall { receiver } => {
+                        let receiver_sugg = Sugg::hir_with_applicability(cx, receiver, "_", applicability);
+                        format!("{receiver_sugg}.clone_into({rhs_sugg})")
+                    },
+                    CallKind::FunctionCall { self_arg, .. } => {
+                        let self_sugg = Sugg::hir_with_applicability(cx, self_arg, "_", applicability);
+                        format!("::std::borrow::ToOwned::clone_into({self_sugg}, {rhs_sugg})")
+                    },
+                }
+            },
+        }
+    }
+}
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index a453386154d..2b324f5f96e 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -47,6 +47,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::asm_syntax::INLINE_ASM_X86_INTEL_SYNTAX_INFO,
     crate::assertions_on_constants::ASSERTIONS_ON_CONSTANTS_INFO,
     crate::assertions_on_result_states::ASSERTIONS_ON_RESULT_STATES_INFO,
+    crate::assigning_clones::ASSIGNING_CLONES_INFO,
     crate::async_yields_async::ASYNC_YIELDS_ASYNC_INFO,
     crate::attrs::ALLOW_ATTRIBUTES_WITHOUT_REASON_INFO,
     crate::attrs::BLANKET_CLIPPY_RESTRICTION_LINTS_INFO,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 76e75968314..b930175c4d8 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -80,6 +80,7 @@ mod as_conversions;
 mod asm_syntax;
 mod assertions_on_constants;
 mod assertions_on_result_states;
+mod assigning_clones;
 mod async_yields_async;
 mod attrs;
 mod await_holding_invalid;
@@ -1118,6 +1119,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| Box::new(incompatible_msrv::IncompatibleMsrv::new(msrv())));
     store.register_late_pass(|_| Box::new(to_string_trait_impl::ToStringTraitImpl));
     store.register_early_pass(|| Box::new(multiple_bound_locations::MultipleBoundLocations));
+    store.register_late_pass(|_| Box::new(assigning_clones::AssigningClones));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 
diff --git a/tests/ui/assigning_clones.fixed b/tests/ui/assigning_clones.fixed
new file mode 100644
index 00000000000..d065c604438
--- /dev/null
+++ b/tests/ui/assigning_clones.fixed
@@ -0,0 +1,211 @@
+// run-rustfix
+#![allow(unused)]
+#![allow(clippy::redundant_clone)]
+#![allow(clippy::ptr_arg)] // https://github.com/rust-lang/rust-clippy/issues/10612
+#![allow(clippy::needless_late_init)]
+#![allow(clippy::box_collection)]
+#![warn(clippy::assigning_clones)]
+
+use std::borrow::ToOwned;
+use std::ops::{Add, Deref, DerefMut};
+
+// Clone
+pub struct HasCloneFrom;
+
+impl Clone for HasCloneFrom {
+    fn clone(&self) -> Self {
+        Self
+    }
+    fn clone_from(&mut self, source: &Self) {
+        *self = HasCloneFrom;
+    }
+}
+
+fn clone_method_rhs_val(mut_thing: &mut HasCloneFrom, value_thing: HasCloneFrom) {
+    mut_thing.clone_from(&value_thing);
+}
+
+fn clone_method_rhs_ref(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
+    mut_thing.clone_from(ref_thing);
+}
+
+fn clone_method_lhs_val(mut mut_thing: HasCloneFrom, ref_thing: &HasCloneFrom) {
+    mut_thing.clone_from(ref_thing);
+}
+
+fn clone_function_lhs_mut_ref(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
+    ::std::clone::Clone::clone_from(mut_thing, ref_thing);
+}
+
+fn clone_function_lhs_val(mut mut_thing: HasCloneFrom, ref_thing: &HasCloneFrom) {
+    ::std::clone::Clone::clone_from(&mut mut_thing, ref_thing);
+}
+
+fn clone_function_through_trait(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
+    ::std::clone::Clone::clone_from(mut_thing, ref_thing);
+}
+
+fn clone_function_through_type(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
+    ::std::clone::Clone::clone_from(mut_thing, ref_thing);
+}
+
+fn clone_function_fully_qualified(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
+    ::std::clone::Clone::clone_from(mut_thing, ref_thing);
+}
+
+fn clone_method_lhs_complex(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
+    // These parens should be kept as necessary for a receiver
+    (mut_thing + &mut HasCloneFrom).clone_from(ref_thing);
+}
+
+fn clone_method_rhs_complex(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
+    // These parens should be removed since they are not needed in a function argument
+    mut_thing.clone_from(ref_thing + ref_thing);
+}
+
+fn assign_to_init_mut_var(b: HasCloneFrom) -> HasCloneFrom {
+    let mut a = HasCloneFrom;
+    for _ in 1..10 {
+        a.clone_from(&b);
+    }
+    a
+}
+
+fn assign_to_late_init_mut_var(b: HasCloneFrom) {
+    let mut a;
+    a = HasCloneFrom;
+    a = b.clone();
+}
+
+fn assign_to_uninit_var(b: HasCloneFrom) {
+    let a;
+    a = b.clone();
+}
+
+fn assign_to_uninit_mut_var(b: HasCloneFrom) {
+    let mut a;
+    a = b.clone();
+}
+
+#[derive(Clone)]
+pub struct HasDeriveClone;
+
+fn ignore_derive_clone(a: &mut HasDeriveClone, b: &HasDeriveClone) {
+    // Should not be linted, since the Clone impl is derived
+    *a = b.clone();
+}
+
+pub struct HasCloneImpl;
+
+impl Clone for HasCloneImpl {
+    fn clone(&self) -> Self {
+        Self
+    }
+}
+
+fn ignore_missing_clone_from(a: &mut HasCloneImpl, b: &HasCloneImpl) {
+    // Should not be linted, since the Clone impl doesn't override clone_from
+    *a = b.clone();
+}
+
+struct FakeClone;
+
+impl FakeClone {
+    /// This looks just like `Clone::clone`
+    fn clone(&self) -> Self {
+        FakeClone
+    }
+}
+
+fn ignore_fake_clone() {
+    let mut a = FakeClone;
+    let b = FakeClone;
+    // Should not be linted, since the Clone impl doesn't come from std
+    a = b.clone();
+}
+
+fn ignore_generic_clone<T: Clone>(a: &mut T, b: &T) {
+    // Should not be linted, since we don't know the actual clone impl
+    *a = b.clone();
+}
+
+// ToOwned
+fn owned_method_mut_ref(mut_string: &mut String, ref_str: &str) {
+    ref_str.clone_into(mut_string);
+}
+
+fn owned_method_val(mut mut_string: String, ref_str: &str) {
+    ref_str.clone_into(&mut mut_string);
+}
+
+struct HasDeref {
+    a: String
+}
+
+impl Deref for HasDeref {
+    type Target = String;
+    fn deref(&self) -> &Self::Target {
+        &self.a
+    }
+}
+
+impl DerefMut for HasDeref {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.a
+    }
+}
+
+fn owned_method_box(mut_box_string: &mut Box<String>, ref_str: &str) {
+    ref_str.clone_into(&mut (*mut_box_string));
+}
+
+fn owned_method_deref(mut_box_string: &mut HasDeref, ref_str: &str) {
+    ref_str.clone_into(&mut (*mut_box_string));
+}
+
+fn owned_function_mut_ref(mut_thing: &mut String, ref_str: &str) {
+    ::std::borrow::ToOwned::clone_into(ref_str, mut_thing);
+}
+
+fn owned_function_val(mut mut_thing: String, ref_str: &str) {
+    ::std::borrow::ToOwned::clone_into(ref_str, &mut mut_thing);
+}
+
+struct FakeToOwned;
+impl FakeToOwned {
+    /// This looks just like `ToOwned::to_owned`
+    fn to_owned(&self) -> Self {
+        FakeToOwned
+    }
+}
+
+fn fake_to_owned() {
+    let mut a = FakeToOwned;
+    let b = FakeToOwned;
+    // Should not be linted, since the ToOwned impl doesn't come from std
+    a = b.to_owned();
+}
+
+fn main() {}
+
+/// Trait implementation to allow producing a `Thing` with a low-precedence expression.
+impl Add for HasCloneFrom {
+    type Output = Self;
+    fn add(self, _: HasCloneFrom) -> Self {
+        self
+    }
+}
+/// Trait implementation to allow producing a `&Thing` with a low-precedence expression.
+impl<'a> Add for &'a HasCloneFrom {
+    type Output = Self;
+    fn add(self, _: &'a HasCloneFrom) -> Self {
+        self
+    }
+}
+/// Trait implementation to allow producing a `&mut Thing` with a low-precedence expression.
+impl<'a> Add for &'a mut HasCloneFrom {
+    type Output = Self;
+    fn add(self, _: &'a mut HasCloneFrom) -> Self {
+        self
+    }
+}
diff --git a/tests/ui/assigning_clones.rs b/tests/ui/assigning_clones.rs
new file mode 100644
index 00000000000..f1ee202c532
--- /dev/null
+++ b/tests/ui/assigning_clones.rs
@@ -0,0 +1,211 @@
+// run-rustfix
+#![allow(unused)]
+#![allow(clippy::redundant_clone)]
+#![allow(clippy::ptr_arg)] // https://github.com/rust-lang/rust-clippy/issues/10612
+#![allow(clippy::needless_late_init)]
+#![allow(clippy::box_collection)]
+#![warn(clippy::assigning_clones)]
+
+use std::borrow::ToOwned;
+use std::ops::{Add, Deref, DerefMut};
+
+// Clone
+pub struct HasCloneFrom;
+
+impl Clone for HasCloneFrom {
+    fn clone(&self) -> Self {
+        Self
+    }
+    fn clone_from(&mut self, source: &Self) {
+        *self = HasCloneFrom;
+    }
+}
+
+fn clone_method_rhs_val(mut_thing: &mut HasCloneFrom, value_thing: HasCloneFrom) {
+    *mut_thing = value_thing.clone();
+}
+
+fn clone_method_rhs_ref(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
+    *mut_thing = ref_thing.clone();
+}
+
+fn clone_method_lhs_val(mut mut_thing: HasCloneFrom, ref_thing: &HasCloneFrom) {
+    mut_thing = ref_thing.clone();
+}
+
+fn clone_function_lhs_mut_ref(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
+    *mut_thing = Clone::clone(ref_thing);
+}
+
+fn clone_function_lhs_val(mut mut_thing: HasCloneFrom, ref_thing: &HasCloneFrom) {
+    mut_thing = Clone::clone(ref_thing);
+}
+
+fn clone_function_through_trait(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
+    *mut_thing = Clone::clone(ref_thing);
+}
+
+fn clone_function_through_type(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
+    *mut_thing = HasCloneFrom::clone(ref_thing);
+}
+
+fn clone_function_fully_qualified(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
+    *mut_thing = <HasCloneFrom as Clone>::clone(ref_thing);
+}
+
+fn clone_method_lhs_complex(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
+    // These parens should be kept as necessary for a receiver
+    *(mut_thing + &mut HasCloneFrom) = ref_thing.clone();
+}
+
+fn clone_method_rhs_complex(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
+    // These parens should be removed since they are not needed in a function argument
+    *mut_thing = (ref_thing + ref_thing).clone();
+}
+
+fn assign_to_init_mut_var(b: HasCloneFrom) -> HasCloneFrom {
+    let mut a = HasCloneFrom;
+    for _ in 1..10 {
+        a = b.clone();
+    }
+    a
+}
+
+fn assign_to_late_init_mut_var(b: HasCloneFrom) {
+    let mut a;
+    a = HasCloneFrom;
+    a = b.clone();
+}
+
+fn assign_to_uninit_var(b: HasCloneFrom) {
+    let a;
+    a = b.clone();
+}
+
+fn assign_to_uninit_mut_var(b: HasCloneFrom) {
+    let mut a;
+    a = b.clone();
+}
+
+#[derive(Clone)]
+pub struct HasDeriveClone;
+
+fn ignore_derive_clone(a: &mut HasDeriveClone, b: &HasDeriveClone) {
+    // Should not be linted, since the Clone impl is derived
+    *a = b.clone();
+}
+
+pub struct HasCloneImpl;
+
+impl Clone for HasCloneImpl {
+    fn clone(&self) -> Self {
+        Self
+    }
+}
+
+fn ignore_missing_clone_from(a: &mut HasCloneImpl, b: &HasCloneImpl) {
+    // Should not be linted, since the Clone impl doesn't override clone_from
+    *a = b.clone();
+}
+
+struct FakeClone;
+
+impl FakeClone {
+    /// This looks just like `Clone::clone`
+    fn clone(&self) -> Self {
+        FakeClone
+    }
+}
+
+fn ignore_fake_clone() {
+    let mut a = FakeClone;
+    let b = FakeClone;
+    // Should not be linted, since the Clone impl doesn't come from std
+    a = b.clone();
+}
+
+fn ignore_generic_clone<T: Clone>(a: &mut T, b: &T) {
+    // Should not be linted, since we don't know the actual clone impl
+    *a = b.clone();
+}
+
+// ToOwned
+fn owned_method_mut_ref(mut_string: &mut String, ref_str: &str) {
+    *mut_string = ref_str.to_owned();
+}
+
+fn owned_method_val(mut mut_string: String, ref_str: &str) {
+    mut_string = ref_str.to_owned();
+}
+
+struct HasDeref {
+    a: String,
+}
+
+impl Deref for HasDeref {
+    type Target = String;
+    fn deref(&self) -> &Self::Target {
+        &self.a
+    }
+}
+
+impl DerefMut for HasDeref {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.a
+    }
+}
+
+fn owned_method_box(mut_box_string: &mut Box<String>, ref_str: &str) {
+    **mut_box_string = ref_str.to_owned();
+}
+
+fn owned_method_deref(mut_box_string: &mut HasDeref, ref_str: &str) {
+    **mut_box_string = ref_str.to_owned();
+}
+
+fn owned_function_mut_ref(mut_thing: &mut String, ref_str: &str) {
+    *mut_thing = ToOwned::to_owned(ref_str);
+}
+
+fn owned_function_val(mut mut_thing: String, ref_str: &str) {
+    mut_thing = ToOwned::to_owned(ref_str);
+}
+
+struct FakeToOwned;
+impl FakeToOwned {
+    /// This looks just like `ToOwned::to_owned`
+    fn to_owned(&self) -> Self {
+        FakeToOwned
+    }
+}
+
+fn fake_to_owned() {
+    let mut a = FakeToOwned;
+    let b = FakeToOwned;
+    // Should not be linted, since the ToOwned impl doesn't come from std
+    a = b.to_owned();
+}
+
+fn main() {}
+
+/// Trait implementation to allow producing a `Thing` with a low-precedence expression.
+impl Add for HasCloneFrom {
+    type Output = Self;
+    fn add(self, _: HasCloneFrom) -> Self {
+        self
+    }
+}
+/// Trait implementation to allow producing a `&Thing` with a low-precedence expression.
+impl<'a> Add for &'a HasCloneFrom {
+    type Output = Self;
+    fn add(self, _: &'a HasCloneFrom) -> Self {
+        self
+    }
+}
+/// Trait implementation to allow producing a `&mut Thing` with a low-precedence expression.
+impl<'a> Add for &'a mut HasCloneFrom {
+    type Output = Self;
+    fn add(self, _: &'a mut HasCloneFrom) -> Self {
+        self
+    }
+}
diff --git a/tests/ui/assigning_clones.stderr b/tests/ui/assigning_clones.stderr
new file mode 100644
index 00000000000..3b84406a71c
--- /dev/null
+++ b/tests/ui/assigning_clones.stderr
@@ -0,0 +1,107 @@
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> $DIR/assigning_clones.rs:25:5
+   |
+LL |     *mut_thing = value_thing.clone();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `mut_thing.clone_from(&value_thing)`
+   |
+   = note: `-D clippy::assigning-clones` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::assigning_clones)]`
+
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> $DIR/assigning_clones.rs:29:5
+   |
+LL |     *mut_thing = ref_thing.clone();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `mut_thing.clone_from(ref_thing)`
+
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> $DIR/assigning_clones.rs:33:5
+   |
+LL |     mut_thing = ref_thing.clone();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `mut_thing.clone_from(ref_thing)`
+
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> $DIR/assigning_clones.rs:37:5
+   |
+LL |     *mut_thing = Clone::clone(ref_thing);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `::std::clone::Clone::clone_from(mut_thing, ref_thing)`
+
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> $DIR/assigning_clones.rs:41:5
+   |
+LL |     mut_thing = Clone::clone(ref_thing);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `::std::clone::Clone::clone_from(&mut mut_thing, ref_thing)`
+
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> $DIR/assigning_clones.rs:45:5
+   |
+LL |     *mut_thing = Clone::clone(ref_thing);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `::std::clone::Clone::clone_from(mut_thing, ref_thing)`
+
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> $DIR/assigning_clones.rs:49:5
+   |
+LL |     *mut_thing = HasCloneFrom::clone(ref_thing);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `::std::clone::Clone::clone_from(mut_thing, ref_thing)`
+
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> $DIR/assigning_clones.rs:53:5
+   |
+LL |     *mut_thing = <HasCloneFrom as Clone>::clone(ref_thing);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `::std::clone::Clone::clone_from(mut_thing, ref_thing)`
+
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> $DIR/assigning_clones.rs:58:5
+   |
+LL |     *(mut_thing + &mut HasCloneFrom) = ref_thing.clone();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `(mut_thing + &mut HasCloneFrom).clone_from(ref_thing)`
+
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> $DIR/assigning_clones.rs:63:5
+   |
+LL |     *mut_thing = (ref_thing + ref_thing).clone();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `mut_thing.clone_from(ref_thing + ref_thing)`
+
+error: assigning the result of `Clone::clone()` may be inefficient
+  --> $DIR/assigning_clones.rs:69:9
+   |
+LL |         a = b.clone();
+   |         ^^^^^^^^^^^^^ help: use `clone_from()`: `a.clone_from(&b)`
+
+error: assigning the result of `ToOwned::to_owned()` may be inefficient
+  --> $DIR/assigning_clones.rs:134:5
+   |
+LL |     *mut_string = ref_str.to_owned();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(mut_string)`
+
+error: assigning the result of `ToOwned::to_owned()` may be inefficient
+  --> $DIR/assigning_clones.rs:138:5
+   |
+LL |     mut_string = ref_str.to_owned();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut mut_string)`
+
+error: assigning the result of `ToOwned::to_owned()` may be inefficient
+  --> $DIR/assigning_clones.rs:159:5
+   |
+LL |     **mut_box_string = ref_str.to_owned();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut (*mut_box_string))`
+
+error: assigning the result of `ToOwned::to_owned()` may be inefficient
+  --> $DIR/assigning_clones.rs:163:5
+   |
+LL |     **mut_box_string = ref_str.to_owned();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut (*mut_box_string))`
+
+error: assigning the result of `ToOwned::to_owned()` may be inefficient
+  --> $DIR/assigning_clones.rs:167:5
+   |
+LL |     *mut_thing = ToOwned::to_owned(ref_str);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `::std::borrow::ToOwned::clone_into(ref_str, mut_thing)`
+
+error: assigning the result of `ToOwned::to_owned()` may be inefficient
+  --> $DIR/assigning_clones.rs:171:5
+   |
+LL |     mut_thing = ToOwned::to_owned(ref_str);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `::std::borrow::ToOwned::clone_into(ref_str, &mut mut_thing)`
+
+error: aborting due to 17 previous errors
+