Skip to content

Commit

Permalink
Auto merge of #12857 - WeiTheShinobi:non_canonical_impls, r=y21
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed May 30, 2024
2 parents 03654ba + 1038927 commit e7efe43
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 7 deletions.
10 changes: 7 additions & 3 deletions clippy_lints/src/non_canonical_impls.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;
};
Expand All @@ -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)
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/auxiliary/proc_macro_derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
}
}
20 changes: 20 additions & 0 deletions tests/ui/non_canonical_clone_impl.fixed
Original file line number Diff line number Diff line change
@@ -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);
Expand Down Expand Up @@ -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!()
}
}
);
20 changes: 20 additions & 0 deletions tests/ui/non_canonical_clone_impl.rs
Original file line number Diff line number Diff line change
@@ -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);
Expand Down Expand Up @@ -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!()
}
}
);
8 changes: 4 additions & 4 deletions tests/ui/non_canonical_clone_impl.stderr
Original file line number Diff line number Diff line change
@@ -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 {
| _____________________________^
Expand All @@ -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();
Expand All @@ -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 {
| _____________________________^
Expand All @@ -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();
Expand Down

0 comments on commit e7efe43

Please sign in to comment.