Auto merge of #6993 - Jarcho:expl_impl_clone, r=llogiq
Improve `expl_impl_clone_on_copy` fixes: #1254 changelog: Check to see if the generic constraints are the same as if using derive for `expl_impl_clone_on_copy`
This commit is contained in:
commit
c07103b4a2
3 changed files with 89 additions and 49 deletions
|
@ -1,6 +1,6 @@
|
||||||
use clippy_utils::diagnostics::{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::paths;
|
||||||
use clippy_utils::ty::is_copy;
|
use clippy_utils::ty::{implements_trait, is_copy};
|
||||||
use clippy_utils::{get_trait_def_id, is_allowed, is_automatically_derived, match_def_path};
|
use clippy_utils::{get_trait_def_id, is_allowed, is_automatically_derived, match_def_path};
|
||||||
use if_chain::if_chain;
|
use if_chain::if_chain;
|
||||||
use rustc_hir::def_id::DefId;
|
use rustc_hir::def_id::DefId;
|
||||||
|
@ -12,7 +12,7 @@ use rustc_lint::{LateContext, LateLintPass};
|
||||||
use rustc_middle::hir::map::Map;
|
use rustc_middle::hir::map::Map;
|
||||||
use rustc_middle::ty::{self, Ty};
|
use rustc_middle::ty::{self, Ty};
|
||||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||||
use rustc_span::source_map::Span;
|
use rustc_span::{def_id::LOCAL_CRATE, source_map::Span};
|
||||||
|
|
||||||
declare_clippy_lint! {
|
declare_clippy_lint! {
|
||||||
/// **What it does:** Checks for deriving `Hash` but implementing `PartialEq`
|
/// **What it does:** Checks for deriving `Hash` but implementing `PartialEq`
|
||||||
|
@ -293,48 +293,53 @@ fn check_ord_partial_ord<'tcx>(
|
||||||
|
|
||||||
/// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint.
|
/// 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>) {
|
fn check_copy_clone<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &TraitRef<'_>, ty: Ty<'tcx>) {
|
||||||
if cx
|
let clone_id = match cx.tcx.lang_items().clone_trait() {
|
||||||
.tcx
|
Some(id) if trait_ref.trait_def_id() == Some(id) => id,
|
||||||
.lang_items()
|
_ => return,
|
||||||
.clone_trait()
|
};
|
||||||
.map_or(false, |id| Some(id) == trait_ref.trait_def_id())
|
let copy_id = match cx.tcx.lang_items().copy_trait() {
|
||||||
{
|
Some(id) => id,
|
||||||
if !is_copy(cx, ty) {
|
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(LOCAL_CRATE)
|
||||||
|
.get(©_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;
|
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.
|
/// Implementation of the `UNSAFE_DERIVE_DESERIALIZE` lint.
|
||||||
|
|
|
@ -35,7 +35,6 @@ impl<'a> Clone for Lt<'a> {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Ok, `Clone` cannot be derived because of the big array
|
|
||||||
#[derive(Copy)]
|
#[derive(Copy)]
|
||||||
struct BigArray {
|
struct BigArray {
|
||||||
a: [u8; 65],
|
a: [u8; 65],
|
||||||
|
@ -47,7 +46,6 @@ impl Clone for BigArray {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Ok, function pointers are not always Clone
|
|
||||||
#[derive(Copy)]
|
#[derive(Copy)]
|
||||||
struct FnPtr {
|
struct FnPtr {
|
||||||
a: fn() -> !,
|
a: fn() -> !,
|
||||||
|
@ -59,7 +57,7 @@ impl Clone for FnPtr {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Ok, generics
|
// Ok, Clone trait impl doesn't have constrained generics.
|
||||||
#[derive(Copy)]
|
#[derive(Copy)]
|
||||||
struct Generic<T> {
|
struct Generic<T> {
|
||||||
a: T,
|
a: T,
|
||||||
|
@ -71,4 +69,21 @@ impl<T> Clone for Generic<T> {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Copy)]
|
||||||
|
struct Generic2<T>(T);
|
||||||
|
impl<T: Clone> Clone for Generic2<T> {
|
||||||
|
fn clone(&self) -> Self {
|
||||||
|
Self(self.0.clone())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Ok, Clone trait impl doesn't have constrained generics.
|
||||||
|
#[derive(Copy)]
|
||||||
|
struct GenericRef<'a, T, U>(T, &'a U);
|
||||||
|
impl<T: Clone, U> Clone for GenericRef<'_, T, U> {
|
||||||
|
fn clone(&self) -> Self {
|
||||||
|
Self(self.0.clone(), self.1)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fn main() {}
|
fn main() {}
|
||||||
|
|
|
@ -40,7 +40,7 @@ LL | | }
|
||||||
| |_^
|
| |_^
|
||||||
|
|
||||||
error: you are implementing `Clone` explicitly on a `Copy` type
|
error: you are implementing `Clone` explicitly on a `Copy` type
|
||||||
--> $DIR/derive.rs:44:1
|
--> $DIR/derive.rs:43:1
|
||||||
|
|
|
|
||||||
LL | / impl Clone for BigArray {
|
LL | / impl Clone for BigArray {
|
||||||
LL | | fn clone(&self) -> Self {
|
LL | | fn clone(&self) -> Self {
|
||||||
|
@ -50,7 +50,7 @@ LL | | }
|
||||||
| |_^
|
| |_^
|
||||||
|
|
|
|
||||||
note: consider deriving `Clone` or removing `Copy`
|
note: consider deriving `Clone` or removing `Copy`
|
||||||
--> $DIR/derive.rs:44:1
|
--> $DIR/derive.rs:43:1
|
||||||
|
|
|
|
||||||
LL | / impl Clone for BigArray {
|
LL | / impl Clone for BigArray {
|
||||||
LL | | fn clone(&self) -> Self {
|
LL | | fn clone(&self) -> Self {
|
||||||
|
@ -60,7 +60,7 @@ LL | | }
|
||||||
| |_^
|
| |_^
|
||||||
|
|
||||||
error: you are implementing `Clone` explicitly on a `Copy` type
|
error: you are implementing `Clone` explicitly on a `Copy` type
|
||||||
--> $DIR/derive.rs:56:1
|
--> $DIR/derive.rs:54:1
|
||||||
|
|
|
|
||||||
LL | / impl Clone for FnPtr {
|
LL | / impl Clone for FnPtr {
|
||||||
LL | | fn clone(&self) -> Self {
|
LL | | fn clone(&self) -> Self {
|
||||||
|
@ -70,7 +70,7 @@ LL | | }
|
||||||
| |_^
|
| |_^
|
||||||
|
|
|
|
||||||
note: consider deriving `Clone` or removing `Copy`
|
note: consider deriving `Clone` or removing `Copy`
|
||||||
--> $DIR/derive.rs:56:1
|
--> $DIR/derive.rs:54:1
|
||||||
|
|
|
|
||||||
LL | / impl Clone for FnPtr {
|
LL | / impl Clone for FnPtr {
|
||||||
LL | | fn clone(&self) -> Self {
|
LL | | fn clone(&self) -> Self {
|
||||||
|
@ -79,5 +79,25 @@ LL | | }
|
||||||
LL | | }
|
LL | | }
|
||||||
| |_^
|
| |_^
|
||||||
|
|
||||||
error: aborting due to 4 previous errors
|
error: you are implementing `Clone` explicitly on a `Copy` type
|
||||||
|
--> $DIR/derive.rs:74:1
|
||||||
|
|
|
||||||
|
LL | / impl<T: Clone> Clone for Generic2<T> {
|
||||||
|
LL | | fn clone(&self) -> Self {
|
||||||
|
LL | | Self(self.0.clone())
|
||||||
|
LL | | }
|
||||||
|
LL | | }
|
||||||
|
| |_^
|
||||||
|
|
|
||||||
|
note: consider deriving `Clone` or removing `Copy`
|
||||||
|
--> $DIR/derive.rs:74:1
|
||||||
|
|
|
||||||
|
LL | / impl<T: Clone> Clone for Generic2<T> {
|
||||||
|
LL | | fn clone(&self) -> Self {
|
||||||
|
LL | | Self(self.0.clone())
|
||||||
|
LL | | }
|
||||||
|
LL | | }
|
||||||
|
| |_^
|
||||||
|
|
||||||
|
error: aborting due to 5 previous errors
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue