]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/derive.rs
Update docs for `expl_impl_clone_on_copy`. Bug was fixed in 879fa5c9721c89c27be6a9db5...
[rust.git] / clippy_lints / src / derive.rs
index b1e363663bb27fd8fd5a051960fbc6e953e5ba39..8416b8440dfbe0bf99988c21d09aea8b60799d18 100644 (file)
@@ -1,8 +1,7 @@
-use crate::utils::paths;
-use crate::utils::{
-    get_trait_def_id, is_allowed, is_automatically_derived, is_copy, match_def_path, match_path, span_lint_and_help,
-    span_lint_and_note, span_lint_and_then,
-};
+use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note, span_lint_and_then};
+use clippy_utils::paths;
+use clippy_utils::ty::{implements_trait, is_copy};
+use clippy_utils::{get_trait_def_id, is_automatically_derived, is_lint_allowed, match_def_path};
 use if_chain::if_chain;
 use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::{walk_expr, walk_fn, walk_item, FnKind, NestedVisitorMap, Visitor};
 use rustc_span::source_map::Span;
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for deriving `Hash` but implementing `PartialEq`
+    /// ### What it does
+    /// Checks for deriving `Hash` but implementing `PartialEq`
     /// explicitly or vice versa.
     ///
-    /// **Why is this bad?** The implementation of these traits must agree (for
+    /// ### Why is this bad?
+    /// The implementation of these traits must agree (for
     /// example for use with `HashMap`) so it’s probably a bad idea to use a
     /// default-generated `Hash` implementation with an explicitly defined
     /// `PartialEq`. In particular, the following must hold for any type:
@@ -28,9 +29,7 @@
     /// k1 == k2 ⇒ hash(k1) == hash(k2)
     /// ```
     ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
+    /// ### Example
     /// ```ignore
     /// #[derive(Hash)]
     /// struct Foo;
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for deriving `Ord` but implementing `PartialOrd`
+    /// ### What it does
+    /// Checks for deriving `Ord` but implementing `PartialOrd`
     /// explicitly or vice versa.
     ///
-    /// **Why is this bad?** The implementation of these traits must agree (for
+    /// ### Why is this bad?
+    /// The implementation of these traits must agree (for
     /// example for use with `sort`) so it’s probably a bad idea to use a
     /// default-generated `Ord` implementation with an explicitly defined
     /// `PartialOrd`. In particular, the following must hold for any type
     /// k1.cmp(&k2) == k1.partial_cmp(&k2).unwrap()
     /// ```
     ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
-    ///
+    /// ### Example
     /// ```rust,ignore
     /// #[derive(Ord, PartialEq, Eq)]
     /// struct Foo;
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for explicit `Clone` implementations for `Copy`
+    /// ### What it does
+    /// Checks for explicit `Clone` implementations for `Copy`
     /// types.
     ///
-    /// **Why is this bad?** To avoid surprising behaviour, these traits should
+    /// ### Why is this bad?
+    /// To avoid surprising behaviour, these traits should
     /// agree and the behaviour of `Copy` cannot be overridden. In almost all
     /// situations a `Copy` type should have a `Clone` implementation that does
     /// nothing more than copy the object, which is what `#[derive(Copy, Clone)]`
     /// gets you.
     ///
-    /// **Known problems:** Bounds of generic types are sometimes wrong: https://github.com/rust-lang/rust/issues/26925
-    ///
-    /// **Example:**
+    /// ### Example
     /// ```rust,ignore
     /// #[derive(Copy)]
     /// struct Foo;
 }
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for deriving `serde::Deserialize` on a type that
+    /// ### What it does
+    /// Checks for deriving `serde::Deserialize` on a type that
     /// has methods using `unsafe`.
     ///
-    /// **Why is this bad?** Deriving `serde::Deserialize` will create a constructor
+    /// ### Why is this bad?
+    /// Deriving `serde::Deserialize` will create a constructor
     /// that may violate invariants hold by another constructor.
     ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
-    ///
+    /// ### Example
     /// ```rust,ignore
     /// use serde::Deserialize;
     ///
@@ -169,8 +166,9 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
             ..
         }) = item.kind
         {
-            let ty = cx.tcx.type_of(cx.tcx.hir().local_def_id(item.hir_id));
-            let is_automatically_derived = is_automatically_derived(&*item.attrs);
+            let ty = cx.tcx.type_of(item.def_id);
+            let attrs = cx.tcx.hir().attrs(item.hir_id());
+            let is_automatically_derived = is_automatically_derived(attrs);
 
             check_hash_peq(cx, item.span, trait_ref, ty, is_automatically_derived);
             check_ord_partial_ord(cx, item.span, trait_ref, ty, is_automatically_derived);
@@ -199,7 +197,7 @@ fn check_hash_peq<'tcx>(
         then {
             // Look for the PartialEq implementations for `ty`
             cx.tcx.for_each_relevant_impl(peq_trait_def_id, ty, |impl_id| {
-                let peq_is_automatically_derived = is_automatically_derived(&cx.tcx.get_attrs(impl_id));
+                let peq_is_automatically_derived = is_automatically_derived(cx.tcx.get_attrs(impl_id));
 
                 if peq_is_automatically_derived == hash_is_automatically_derived {
                     return;
@@ -253,7 +251,7 @@ fn check_ord_partial_ord<'tcx>(
         then {
             // Look for the PartialOrd implementations for `ty`
             cx.tcx.for_each_relevant_impl(partial_ord_trait_def_id, ty, |impl_id| {
-                let partial_ord_is_automatically_derived = is_automatically_derived(&cx.tcx.get_attrs(impl_id));
+                let partial_ord_is_automatically_derived = is_automatically_derived(cx.tcx.get_attrs(impl_id));
 
                 if partial_ord_is_automatically_derived == ord_is_automatically_derived {
                     return;
@@ -293,43 +291,49 @@ fn check_ord_partial_ord<'tcx>(
 
 /// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint.
 fn check_copy_clone<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &TraitRef<'_>, ty: Ty<'tcx>) {
-    if match_path(&trait_ref.path, &paths::CLONE_TRAIT) {
-        if !is_copy(cx, ty) {
+    let clone_id = match cx.tcx.lang_items().clone_trait() {
+        Some(id) if trait_ref.trait_def_id() == Some(id) => id,
+        _ => return,
+    };
+    let copy_id = match cx.tcx.lang_items().copy_trait() {
+        Some(id) => id,
+        None => return,
+    };
+    let (ty_adt, ty_subs) = match *ty.kind() {
+        // Unions can't derive clone.
+        ty::Adt(adt, subs) if !adt.is_union() => (adt, subs),
+        _ => return,
+    };
+    // If the current self type doesn't implement Copy (due to generic constraints), search to see if
+    // there's a Copy impl for any instance of the adt.
+    if !is_copy(cx, ty) {
+        if ty_subs.non_erasable_generics().next().is_some() {
+            let has_copy_impl = cx.tcx.all_local_trait_impls(()).get(&copy_id).map_or(false, |impls| {
+                impls
+                    .iter()
+                    .any(|&id| matches!(cx.tcx.type_of(id).kind(), ty::Adt(adt, _) if ty_adt.did == adt.did))
+            });
+            if !has_copy_impl {
+                return;
+            }
+        } else {
             return;
         }
-
-        match *ty.kind() {
-            ty::Adt(def, _) if def.is_union() => return,
-
-            // Some types are not Clone by default but could be cloned “by hand” if necessary
-            ty::Adt(def, substs) => {
-                for variant in &def.variants {
-                    for field in &variant.fields {
-                        if let ty::FnDef(..) = field.ty(cx.tcx, substs).kind() {
-                            return;
-                        }
-                    }
-                    for subst in substs {
-                        if let ty::subst::GenericArgKind::Type(subst) = subst.unpack() {
-                            if let ty::Param(_) = subst.kind() {
-                                return;
-                            }
-                        }
-                    }
-                }
-            },
-            _ => (),
-        }
-
-        span_lint_and_note(
-            cx,
-            EXPL_IMPL_CLONE_ON_COPY,
-            item.span,
-            "you are implementing `Clone` explicitly on a `Copy` type",
-            Some(item.span),
-            "consider deriving `Clone` or removing `Copy`",
-        );
     }
+    // Derive constrains all generic types to requiring Clone. Check if any type is not constrained for
+    // this impl.
+    if ty_subs.types().any(|ty| !implements_trait(cx, ty, clone_id, &[])) {
+        return;
+    }
+
+    span_lint_and_note(
+        cx,
+        EXPL_IMPL_CLONE_ON_COPY,
+        item.span,
+        "you are implementing `Clone` explicitly on a `Copy` type",
+        Some(item.span),
+        "consider deriving `Clone` or removing `Copy`",
+    );
 }
 
 /// Implementation of the `UNSAFE_DERIVE_DESERIALIZE` lint.
@@ -356,7 +360,7 @@ fn has_unsafe<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>) -> bool {
         if let ty::Adt(def, _) = ty.kind();
         if let Some(local_def_id) = def.did.as_local();
         let adt_hir_id = cx.tcx.hir().local_def_id_to_hir_id(local_def_id);
-        if !is_allowed(cx, UNSAFE_DERIVE_DESERIALIZE, adt_hir_id);
+        if !is_lint_allowed(cx, UNSAFE_DERIVE_DESERIALIZE, adt_hir_id);
         if cx.tcx.inherent_impls(def.did)
             .iter()
             .map(|imp_did| item_from_def_id(cx, *imp_did))
@@ -404,13 +408,8 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
         }
 
         if let ExprKind::Block(block, _) = expr.kind {
-            match block.rules {
-                BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided)
-                | BlockCheckMode::PushUnsafeBlock(UnsafeSource::UserProvided)
-                | BlockCheckMode::PopUnsafeBlock(UnsafeSource::UserProvided) => {
-                    self.has_unsafe = true;
-                },
-                _ => {},
+            if let BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) = block.rules {
+                self.has_unsafe = true;
             }
         }