Skip to content

Commit 21a5948

Browse files
committed
incorporate review feedback
1 parent 34d9a2a commit 21a5948

File tree

1 file changed

+17
-13
lines changed

1 file changed

+17
-13
lines changed

clippy_lints/src/non_canonical_impls.rs

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -113,16 +113,18 @@ declare_lint_pass!(NonCanonicalImpls => [NON_CANONICAL_CLONE_IMPL, NON_CANONICAL
113113

114114
impl LateLintPass<'_> for NonCanonicalImpls {
115115
fn check_impl_item<'tcx>(&mut self, cx: &LateContext<'tcx>, impl_item: &ImplItem<'tcx>) {
116-
if let Node::Item(item) = cx.tcx.parent_hir_node(impl_item.hir_id())
116+
if let ImplItemKind::Fn(_, impl_item_id) = impl_item.kind
117+
&& let Node::Item(item) = cx.tcx.parent_hir_node(impl_item.hir_id())
117118
&& let Some(trait_impl) = cx.tcx.impl_trait_ref(item.owner_id).map(EarlyBinder::skip_binder)
119+
&& let trait_name = cx.tcx.get_diagnostic_name(trait_impl.def_id)
120+
// NOTE: check this early to avoid expensive checks that come after this one
121+
&& matches!(trait_name, Some(sym::Clone | sym::PartialOrd))
118122
&& !cx.tcx.is_automatically_derived(item.owner_id.to_def_id())
119-
&& let ImplItemKind::Fn(_, impl_item_id) = cx.tcx.hir_impl_item(impl_item.impl_item_id()).kind
120123
&& let body = cx.tcx.hir_body(impl_item_id)
121124
&& let ExprKind::Block(block, ..) = body.value.kind
122125
&& !block.span.in_external_macro(cx.sess().source_map())
123126
&& !is_from_proc_macro(cx, impl_item)
124127
{
125-
let trait_name = cx.tcx.get_diagnostic_name(trait_impl.def_id);
126128
if trait_name == Some(sym::Clone)
127129
&& let Some(copy_def_id) = cx.tcx.get_diagnostic_item(sym::Copy)
128130
&& implements_trait(cx, trait_impl.self_ty(), copy_def_id, &[])
@@ -147,17 +149,19 @@ fn check_clone_on_copy(cx: &LateContext<'_>, impl_item: &ImplItem<'_>, block: &B
147149
&& let ExprKind::Path(qpath) = deref.kind
148150
&& last_path_segment(&qpath).ident.name == kw::SelfLower
149151
{
150-
} else {
151-
span_lint_and_sugg(
152-
cx,
153-
NON_CANONICAL_CLONE_IMPL,
154-
block.span,
155-
"non-canonical implementation of `clone` on a `Copy` type",
156-
"change this to",
157-
"{ *self }".to_owned(),
158-
Applicability::MaybeIncorrect,
159-
);
152+
// this is the canonical implementation, `fn clone(&self) -> Self { *self }`
153+
return;
160154
}
155+
156+
span_lint_and_sugg(
157+
cx,
158+
NON_CANONICAL_CLONE_IMPL,
159+
block.span,
160+
"non-canonical implementation of `clone` on a `Copy` type",
161+
"change this to",
162+
"{ *self }".to_owned(),
163+
Applicability::MaybeIncorrect,
164+
);
161165
}
162166

163167
if impl_item.ident.name == sym::clone_from {

0 commit comments

Comments
 (0)