From c53cea90ad4727fa190d5e548f09c2ce9e963470 Mon Sep 17 00:00:00 2001 From: WeiTheShinobi Date: Sun, 26 May 2024 17:54:44 +0800 Subject: [PATCH 1/2] fix: let non_canonical_impls skip proc marco --- clippy_lints/src/non_canonical_impls.rs | 5 +++++ tests/ui/auxiliary/proc_macro_derive.rs | 14 ++++++++++++++ tests/ui/non_canonical_clone_impl.fixed | 5 +++++ tests/ui/non_canonical_clone_impl.rs | 5 +++++ tests/ui/non_canonical_clone_impl.stderr | 8 ++++---- 5 files changed, 33 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/non_canonical_impls.rs b/clippy_lints/src/non_canonical_impls.rs index 932d6fe54d6..bbefc74e8d2 100644 --- a/clippy_lints/src/non_canonical_impls.rs +++ b/clippy_lints/src/non_canonical_impls.rs @@ -128,6 +128,11 @@ fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) { let ExprKind::Block(block, ..) = body.value.kind else { return; }; + if let Some(expr) = block.expr + && expr.span.from_expansion() + { + return; + } if cx.tcx.is_diagnostic_item(sym::Clone, trait_impl.def_id) && let Some(copy_def_id) = cx.tcx.get_diagnostic_item(sym::Copy) diff --git a/tests/ui/auxiliary/proc_macro_derive.rs b/tests/ui/auxiliary/proc_macro_derive.rs index 79a95d775b1..36ec31a7873 100644 --- a/tests/ui/auxiliary/proc_macro_derive.rs +++ b/tests/ui/auxiliary/proc_macro_derive.rs @@ -169,3 +169,17 @@ pub fn b(&self) { } } } + +#[proc_macro_derive(NonCanonicalClone)] +pub fn non_canonical_clone_derive(_: TokenStream) -> TokenStream { + quote! { + struct NonCanonicalClone; + impl Clone for NonCanonicalClone { + fn clone(&self) -> Self { + let a = *self; + a + } + } + impl Copy for NonCanonicalClone {} + } +} diff --git a/tests/ui/non_canonical_clone_impl.fixed b/tests/ui/non_canonical_clone_impl.fixed index 7d1be412e54..26274fe6574 100644 --- a/tests/ui/non_canonical_clone_impl.fixed +++ b/tests/ui/non_canonical_clone_impl.fixed @@ -1,3 +1,4 @@ +//@aux-build:proc_macro_derive.rs #![allow(clippy::clone_on_copy, unused)] #![allow(clippy::assigning_clones)] #![no_main] @@ -95,3 +96,7 @@ impl Clone for Uwu { } impl Copy for Uwu {} + +// should skip proc macros, see https://github.com/rust-lang/rust-clippy/issues/12788 +#[derive(proc_macro_derive::NonCanonicalClone)] +pub struct G; diff --git a/tests/ui/non_canonical_clone_impl.rs b/tests/ui/non_canonical_clone_impl.rs index beae05efb2f..e2b278ac493 100644 --- a/tests/ui/non_canonical_clone_impl.rs +++ b/tests/ui/non_canonical_clone_impl.rs @@ -1,3 +1,4 @@ +//@aux-build:proc_macro_derive.rs #![allow(clippy::clone_on_copy, unused)] #![allow(clippy::assigning_clones)] #![no_main] @@ -105,3 +106,7 @@ fn clone_from(&mut self, source: &Self) { } impl Copy for Uwu {} + +// should skip proc macros, see https://github.com/rust-lang/rust-clippy/issues/12788 +#[derive(proc_macro_derive::NonCanonicalClone)] +pub struct G; diff --git a/tests/ui/non_canonical_clone_impl.stderr b/tests/ui/non_canonical_clone_impl.stderr index 6bfc99d988b..80258ee8c77 100644 --- a/tests/ui/non_canonical_clone_impl.stderr +++ b/tests/ui/non_canonical_clone_impl.stderr @@ -1,5 +1,5 @@ error: non-canonical implementation of `clone` on a `Copy` type - --> tests/ui/non_canonical_clone_impl.rs:10:29 + --> tests/ui/non_canonical_clone_impl.rs:11:29 | LL | fn clone(&self) -> Self { | _____________________________^ @@ -11,7 +11,7 @@ LL | | } = help: to override `-D warnings` add `#[allow(clippy::non_canonical_clone_impl)]` error: unnecessary implementation of `clone_from` on a `Copy` type - --> tests/ui/non_canonical_clone_impl.rs:14:5 + --> tests/ui/non_canonical_clone_impl.rs:15:5 | LL | / fn clone_from(&mut self, source: &Self) { LL | | source.clone(); @@ -20,7 +20,7 @@ LL | | } | |_____^ help: remove it error: non-canonical implementation of `clone` on a `Copy` type - --> tests/ui/non_canonical_clone_impl.rs:81:29 + --> tests/ui/non_canonical_clone_impl.rs:82:29 | LL | fn clone(&self) -> Self { | _____________________________^ @@ -29,7 +29,7 @@ LL | | } | |_____^ help: change this to: `{ *self }` error: unnecessary implementation of `clone_from` on a `Copy` type - --> tests/ui/non_canonical_clone_impl.rs:85:5 + --> tests/ui/non_canonical_clone_impl.rs:86:5 | LL | / fn clone_from(&mut self, source: &Self) { LL | | source.clone(); From 1038927b479fb6c7fe8d91be5d605fb85d15479a Mon Sep 17 00:00:00 2001 From: WeiTheShinobi Date: Thu, 30 May 2024 21:43:40 +0800 Subject: [PATCH 2/2] fix: add test case, use a better conditional expression. --- clippy_lints/src/non_canonical_impls.rs | 11 +++++------ tests/ui/auxiliary/proc_macro_derive.rs | 3 +-- tests/ui/non_canonical_clone_impl.fixed | 15 +++++++++++++++ tests/ui/non_canonical_clone_impl.rs | 15 +++++++++++++++ tests/ui/non_canonical_clone_impl.stderr | 8 ++++---- 5 files changed, 40 insertions(+), 12 deletions(-) diff --git a/clippy_lints/src/non_canonical_impls.rs b/clippy_lints/src/non_canonical_impls.rs index bbefc74e8d2..de6a1a36f3e 100644 --- a/clippy_lints/src/non_canonical_impls.rs +++ b/clippy_lints/src/non_canonical_impls.rs @@ -1,10 +1,11 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use clippy_utils::ty::implements_trait; -use clippy_utils::{is_res_lang_ctor, last_path_segment, path_res, std_or_core}; +use clippy_utils::{is_from_proc_macro, is_res_lang_ctor, last_path_segment, path_res, std_or_core}; use rustc_errors::Applicability; use rustc_hir::def_id::LocalDefId; use rustc_hir::{Expr, ExprKind, ImplItem, ImplItemKind, LangItem, Node, UnOp}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; use rustc_middle::ty::EarlyBinder; use rustc_session::declare_lint_pass; use rustc_span::sym; @@ -111,7 +112,7 @@ impl LateLintPass<'_> for NonCanonicalImpls { #[expect(clippy::too_many_lines)] - fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) { + fn check_impl_item<'tcx>(&mut self, cx: &LateContext<'tcx>, impl_item: &ImplItem<'tcx>) { let Node::Item(item) = cx.tcx.parent_hir_node(impl_item.hir_id()) else { return; }; @@ -128,9 +129,7 @@ fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) { let ExprKind::Block(block, ..) = body.value.kind else { return; }; - if let Some(expr) = block.expr - && expr.span.from_expansion() - { + if in_external_macro(cx.sess(), block.span) || is_from_proc_macro(cx, impl_item) { return; } diff --git a/tests/ui/auxiliary/proc_macro_derive.rs b/tests/ui/auxiliary/proc_macro_derive.rs index 36ec31a7873..4c3df472269 100644 --- a/tests/ui/auxiliary/proc_macro_derive.rs +++ b/tests/ui/auxiliary/proc_macro_derive.rs @@ -176,8 +176,7 @@ pub fn non_canonical_clone_derive(_: TokenStream) -> TokenStream { struct NonCanonicalClone; impl Clone for NonCanonicalClone { fn clone(&self) -> Self { - let a = *self; - a + todo!() } } impl Copy for NonCanonicalClone {} diff --git a/tests/ui/non_canonical_clone_impl.fixed b/tests/ui/non_canonical_clone_impl.fixed index 26274fe6574..11616f28825 100644 --- a/tests/ui/non_canonical_clone_impl.fixed +++ b/tests/ui/non_canonical_clone_impl.fixed @@ -3,6 +3,9 @@ #![allow(clippy::assigning_clones)] #![no_main] +extern crate proc_macros; +use proc_macros::with_span; + // lint struct A(u32); @@ -100,3 +103,15 @@ impl Copy for Uwu {} // should skip proc macros, see https://github.com/rust-lang/rust-clippy/issues/12788 #[derive(proc_macro_derive::NonCanonicalClone)] pub struct G; + +with_span!( + span + + #[derive(Copy)] + struct H; + impl Clone for H { + fn clone(&self) -> Self { + todo!() + } + } +); diff --git a/tests/ui/non_canonical_clone_impl.rs b/tests/ui/non_canonical_clone_impl.rs index e2b278ac493..a36c7ed44c2 100644 --- a/tests/ui/non_canonical_clone_impl.rs +++ b/tests/ui/non_canonical_clone_impl.rs @@ -3,6 +3,9 @@ #![allow(clippy::assigning_clones)] #![no_main] +extern crate proc_macros; +use proc_macros::with_span; + // lint struct A(u32); @@ -110,3 +113,15 @@ impl Copy for Uwu {} // should skip proc macros, see https://github.com/rust-lang/rust-clippy/issues/12788 #[derive(proc_macro_derive::NonCanonicalClone)] pub struct G; + +with_span!( + span + + #[derive(Copy)] + struct H; + impl Clone for H { + fn clone(&self) -> Self { + todo!() + } + } +); diff --git a/tests/ui/non_canonical_clone_impl.stderr b/tests/ui/non_canonical_clone_impl.stderr index 80258ee8c77..f7cad58150f 100644 --- a/tests/ui/non_canonical_clone_impl.stderr +++ b/tests/ui/non_canonical_clone_impl.stderr @@ -1,5 +1,5 @@ error: non-canonical implementation of `clone` on a `Copy` type - --> tests/ui/non_canonical_clone_impl.rs:11:29 + --> tests/ui/non_canonical_clone_impl.rs:14:29 | LL | fn clone(&self) -> Self { | _____________________________^ @@ -11,7 +11,7 @@ LL | | } = help: to override `-D warnings` add `#[allow(clippy::non_canonical_clone_impl)]` error: unnecessary implementation of `clone_from` on a `Copy` type - --> tests/ui/non_canonical_clone_impl.rs:15:5 + --> tests/ui/non_canonical_clone_impl.rs:18:5 | LL | / fn clone_from(&mut self, source: &Self) { LL | | source.clone(); @@ -20,7 +20,7 @@ LL | | } | |_____^ help: remove it error: non-canonical implementation of `clone` on a `Copy` type - --> tests/ui/non_canonical_clone_impl.rs:82:29 + --> tests/ui/non_canonical_clone_impl.rs:85:29 | LL | fn clone(&self) -> Self { | _____________________________^ @@ -29,7 +29,7 @@ LL | | } | |_____^ help: change this to: `{ *self }` error: unnecessary implementation of `clone_from` on a `Copy` type - --> tests/ui/non_canonical_clone_impl.rs:86:5 + --> tests/ui/non_canonical_clone_impl.rs:89:5 | LL | / fn clone_from(&mut self, source: &Self) { LL | | source.clone();