diff options
| author | bors <bors@rust-lang.org> | 2024-05-30 15:58:48 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2024-05-30 15:58:48 +0000 |
| commit | e7efe4381af5ae065105f6fa8c30ae86e01d3d9a (patch) | |
| tree | 524eff0fcbff2beaff8ffb2f893233b73f7c2b35 | |
| parent | 03654badfd7221bd637daa7a214a0d73db176299 (diff) | |
| parent | 1038927b479fb6c7fe8d91be5d605fb85d15479a (diff) | |
| download | rust-e7efe4381af5ae065105f6fa8c30ae86e01d3d9a.tar.gz rust-e7efe4381af5ae065105f6fa8c30ae86e01d3d9a.zip | |
Auto merge of #12857 - WeiTheShinobi:non_canonical_impls, r=y21
fix: let non_canonical_impls skip proc marco Fixed #12788 Although the issue only mentions `NON_CANONICAL_CLONE_IMPL`, this fix will also affect `NON_CANONICAL_PARTIAL_ORD_IMPL` because I saw > Because of these unforeseeable or unstable behaviors, macro expansion should often not be regarded as a part of the stable API. on Clippy Documentation and these two lints are similar, so I think it might be good, not sure if it's right or not. --- changelog: `NON_CANONICAL_CLONE_IMPL`, `NON_CANONICAL_PARTIAL_ORD_IMPL` will skip proc marco now
| -rw-r--r-- | clippy_lints/src/non_canonical_impls.rs | 10 | ||||
| -rw-r--r-- | tests/ui/auxiliary/proc_macro_derive.rs | 13 | ||||
| -rw-r--r-- | tests/ui/non_canonical_clone_impl.fixed | 20 | ||||
| -rw-r--r-- | tests/ui/non_canonical_clone_impl.rs | 20 | ||||
| -rw-r--r-- | tests/ui/non_canonical_clone_impl.stderr | 8 |
5 files changed, 64 insertions, 7 deletions
diff --git a/clippy_lints/src/non_canonical_impls.rs b/clippy_lints/src/non_canonical_impls.rs index 932d6fe54d6..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 @@ declare_lint_pass!(NonCanonicalImpls => [NON_CANONICAL_CLONE_IMPL, NON_CANONICAL 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,6 +129,9 @@ impl LateLintPass<'_> for NonCanonicalImpls { let ExprKind::Block(block, ..) = body.value.kind else { return; }; + if in_external_macro(cx.sess(), block.span) || is_from_proc_macro(cx, impl_item) { + 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..4c3df472269 100644 --- a/tests/ui/auxiliary/proc_macro_derive.rs +++ b/tests/ui/auxiliary/proc_macro_derive.rs @@ -169,3 +169,16 @@ pub fn derive_ignored_unit_pattern(_: TokenStream) -> TokenStream { } } } + +#[proc_macro_derive(NonCanonicalClone)] +pub fn non_canonical_clone_derive(_: TokenStream) -> TokenStream { + quote! { + struct NonCanonicalClone; + impl Clone for NonCanonicalClone { + fn clone(&self) -> Self { + todo!() + } + } + impl Copy for NonCanonicalClone {} + } +} diff --git a/tests/ui/non_canonical_clone_impl.fixed b/tests/ui/non_canonical_clone_impl.fixed index 7d1be412e54..11616f28825 100644 --- a/tests/ui/non_canonical_clone_impl.fixed +++ b/tests/ui/non_canonical_clone_impl.fixed @@ -1,7 +1,11 @@ +//@aux-build:proc_macro_derive.rs #![allow(clippy::clone_on_copy, unused)] #![allow(clippy::assigning_clones)] #![no_main] +extern crate proc_macros; +use proc_macros::with_span; + // lint struct A(u32); @@ -95,3 +99,19 @@ impl<A: Copy> Clone for Uwu<A> { } impl<A: std::fmt::Debug + Copy + Clone> Copy for Uwu<A> {} + +// 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 beae05efb2f..a36c7ed44c2 100644 --- a/tests/ui/non_canonical_clone_impl.rs +++ b/tests/ui/non_canonical_clone_impl.rs @@ -1,7 +1,11 @@ +//@aux-build:proc_macro_derive.rs #![allow(clippy::clone_on_copy, unused)] #![allow(clippy::assigning_clones)] #![no_main] +extern crate proc_macros; +use proc_macros::with_span; + // lint struct A(u32); @@ -105,3 +109,19 @@ impl<A: Copy> Clone for Uwu<A> { } impl<A: std::fmt::Debug + Copy + Clone> Copy for Uwu<A> {} + +// 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 6bfc99d988b..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:10: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:14: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:81: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:85:5 + --> tests/ui/non_canonical_clone_impl.rs:89:5 | LL | / fn clone_from(&mut self, source: &Self) { LL | | source.clone(); |
