about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-05-30 15:58:48 +0000
committerbors <bors@rust-lang.org>2024-05-30 15:58:48 +0000
commite7efe4381af5ae065105f6fa8c30ae86e01d3d9a (patch)
tree524eff0fcbff2beaff8ffb2f893233b73f7c2b35
parent03654badfd7221bd637daa7a214a0d73db176299 (diff)
parent1038927b479fb6c7fe8d91be5d605fb85d15479a (diff)
downloadrust-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.rs10
-rw-r--r--tests/ui/auxiliary/proc_macro_derive.rs13
-rw-r--r--tests/ui/non_canonical_clone_impl.fixed20
-rw-r--r--tests/ui/non_canonical_clone_impl.rs20
-rw-r--r--tests/ui/non_canonical_clone_impl.stderr8
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();