From ac7c6e54176a7e7d987b8c5d48716267a964e839 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sat, 10 Feb 2024 16:10:05 -0500 Subject: [PATCH] Don't allow derive macros to silence `disallowed_macros` for their own call. --- clippy_lints/src/disallowed_macros.rs | 84 ++++++++++++------- .../auxiliary/proc_macros.rs | 29 +++++++ tests/ui-toml/disallowed_macros/clippy.toml | 1 + .../disallowed_macros/disallowed_macros.rs | 6 ++ .../disallowed_macros.stderr | 38 +++++---- 5 files changed, 112 insertions(+), 46 deletions(-) create mode 100644 tests/ui-toml/disallowed_macros/auxiliary/proc_macros.rs diff --git a/clippy_lints/src/disallowed_macros.rs b/clippy_lints/src/disallowed_macros.rs index 656b3d9bfaf..4652cec722e 100644 --- a/clippy_lints/src/disallowed_macros.rs +++ b/clippy_lints/src/disallowed_macros.rs @@ -1,13 +1,16 @@ use clippy_config::types::DisallowedPath; -use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then}; use clippy_utils::macros::macro_backtrace; use rustc_ast::Attribute; use rustc_data_structures::fx::FxHashSet; +use rustc_errors::Diagnostic; use rustc_hir::def_id::DefIdMap; -use rustc_hir::{Expr, ExprKind, ForeignItem, HirId, ImplItem, Item, Pat, Path, Stmt, TraitItem, Ty}; +use rustc_hir::{ + Expr, ExprKind, ForeignItem, HirId, ImplItem, Item, ItemKind, OwnerId, Pat, Path, Stmt, TraitItem, Ty, +}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::impl_lint_pass; -use rustc_span::{ExpnId, Span}; +use rustc_span::{ExpnId, MacroKind, Span}; declare_clippy_lint! { /// ### What it does @@ -57,6 +60,10 @@ pub struct DisallowedMacros { conf_disallowed: Vec, disallowed: DefIdMap, seen: FxHashSet, + + // Track the most recently seen node that can have a `derive` attribute. + // Needed to use the correct lint level. + derive_src: Option, } impl DisallowedMacros { @@ -65,10 +72,11 @@ impl DisallowedMacros { conf_disallowed, disallowed: DefIdMap::default(), seen: FxHashSet::default(), + derive_src: None, } } - fn check(&mut self, cx: &LateContext<'_>, span: Span) { + fn check(&mut self, cx: &LateContext<'_>, span: Span, derive_src: Option) { if self.conf_disallowed.is_empty() { return; } @@ -80,18 +88,26 @@ impl DisallowedMacros { if let Some(&index) = self.disallowed.get(&mac.def_id) { let conf = &self.conf_disallowed[index]; - - span_lint_and_then( - cx, - DISALLOWED_MACROS, - mac.span, - &format!("use of a disallowed macro `{}`", conf.path()), - |diag| { - if let Some(reason) = conf.reason() { - diag.note(reason); - } - }, - ); + let msg = format!("use of a disallowed macro `{}`", conf.path()); + let add_note = |diag: &mut Diagnostic| { + if let Some(reason) = conf.reason() { + diag.note(reason); + } + }; + if matches!(mac.kind, MacroKind::Derive) + && let Some(derive_src) = derive_src + { + span_lint_hir_and_then( + cx, + DISALLOWED_MACROS, + cx.tcx.local_def_id_to_hir_id(derive_src.def_id), + mac.span, + &msg, + add_note, + ); + } else { + span_lint_and_then(cx, DISALLOWED_MACROS, mac.span, &msg, add_note); + } } } } @@ -110,49 +126,57 @@ impl LateLintPass<'_> for DisallowedMacros { } fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { - self.check(cx, expr.span); + self.check(cx, expr.span, None); // `$t + $t` can have the context of $t, check also the span of the binary operator if let ExprKind::Binary(op, ..) = expr.kind { - self.check(cx, op.span); + self.check(cx, op.span, None); } } fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) { - self.check(cx, stmt.span); + self.check(cx, stmt.span, None); } fn check_ty(&mut self, cx: &LateContext<'_>, ty: &Ty<'_>) { - self.check(cx, ty.span); + self.check(cx, ty.span, None); } fn check_pat(&mut self, cx: &LateContext<'_>, pat: &Pat<'_>) { - self.check(cx, pat.span); + self.check(cx, pat.span, None); } fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { - self.check(cx, item.span); - self.check(cx, item.vis_span); + self.check(cx, item.span, self.derive_src); + self.check(cx, item.vis_span, None); + + if matches!( + item.kind, + ItemKind::Struct(..) | ItemKind::Enum(..) | ItemKind::Union(..) + ) && macro_backtrace(item.span).all(|m| !matches!(m.kind, MacroKind::Derive)) + { + self.derive_src = Some(item.owner_id); + } } fn check_foreign_item(&mut self, cx: &LateContext<'_>, item: &ForeignItem<'_>) { - self.check(cx, item.span); - self.check(cx, item.vis_span); + self.check(cx, item.span, None); + self.check(cx, item.vis_span, None); } fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &ImplItem<'_>) { - self.check(cx, item.span); - self.check(cx, item.vis_span); + self.check(cx, item.span, None); + self.check(cx, item.vis_span, None); } fn check_trait_item(&mut self, cx: &LateContext<'_>, item: &TraitItem<'_>) { - self.check(cx, item.span); + self.check(cx, item.span, None); } fn check_path(&mut self, cx: &LateContext<'_>, path: &Path<'_>, _: HirId) { - self.check(cx, path.span); + self.check(cx, path.span, None); } fn check_attribute(&mut self, cx: &LateContext<'_>, attr: &Attribute) { - self.check(cx, attr.span); + self.check(cx, attr.span, self.derive_src); } } diff --git a/tests/ui-toml/disallowed_macros/auxiliary/proc_macros.rs b/tests/ui-toml/disallowed_macros/auxiliary/proc_macros.rs new file mode 100644 index 00000000000..dbfce16858a --- /dev/null +++ b/tests/ui-toml/disallowed_macros/auxiliary/proc_macros.rs @@ -0,0 +1,29 @@ +extern crate proc_macro; +use proc_macro::Delimiter::{Brace, Bracket, Parenthesis}; +use proc_macro::Spacing::{Alone, Joint}; +use proc_macro::{Group, Ident, Punct, Span, TokenStream, TokenTree as TT}; + +#[proc_macro_derive(Derive)] +pub fn derive(_: TokenStream) -> TokenStream { + TokenStream::from_iter([ + TT::from(Punct::new('#', Alone)), + TT::from(Group::new( + Bracket, + TokenStream::from_iter([ + TT::from(Ident::new("allow", Span::call_site())), + TT::from(Group::new( + Parenthesis, + TokenStream::from_iter([ + TT::from(Ident::new("clippy", Span::call_site())), + TT::from(Punct::new(':', Joint)), + TT::from(Punct::new(':', Alone)), + TT::from(Ident::new("disallowed_macros", Span::call_site())), + ]), + )), + ]), + )), + TT::from(Ident::new("impl", Span::call_site())), + TT::from(Ident::new("Foo", Span::call_site())), + TT::from(Group::new(Brace, TokenStream::new())), + ]) +} diff --git a/tests/ui-toml/disallowed_macros/clippy.toml b/tests/ui-toml/disallowed_macros/clippy.toml index 85f1b71eb66..8b4f3b713bb 100644 --- a/tests/ui-toml/disallowed_macros/clippy.toml +++ b/tests/ui-toml/disallowed_macros/clippy.toml @@ -10,4 +10,5 @@ disallowed-macros = [ "macros::item", "macros::binop", "macros::attr", + "proc_macros::Derive", ] diff --git a/tests/ui-toml/disallowed_macros/disallowed_macros.rs b/tests/ui-toml/disallowed_macros/disallowed_macros.rs index 4a3d55e13c9..e63a99e74cb 100644 --- a/tests/ui-toml/disallowed_macros/disallowed_macros.rs +++ b/tests/ui-toml/disallowed_macros/disallowed_macros.rs @@ -1,9 +1,12 @@ //@aux-build:macros.rs +//@aux-build:proc_macros.rs #![allow(unused)] extern crate macros; +extern crate proc_macros; +use proc_macros::Derive; use serde::Serialize; fn main() { @@ -40,3 +43,6 @@ trait Y { impl Y for S { macros::item!(); } + +#[derive(Derive)] +struct Foo; diff --git a/tests/ui-toml/disallowed_macros/disallowed_macros.stderr b/tests/ui-toml/disallowed_macros/disallowed_macros.stderr index 3c6f59b16e7..986a7b171c4 100644 --- a/tests/ui-toml/disallowed_macros/disallowed_macros.stderr +++ b/tests/ui-toml/disallowed_macros/disallowed_macros.stderr @@ -1,5 +1,5 @@ error: use of a disallowed macro `std::println` - --> $DIR/disallowed_macros.rs:10:5 + --> $DIR/disallowed_macros.rs:13:5 | LL | println!("one"); | ^^^^^^^^^^^^^^^ @@ -8,25 +8,25 @@ LL | println!("one"); = help: to override `-D warnings` add `#[allow(clippy::disallowed_macros)]` error: use of a disallowed macro `std::println` - --> $DIR/disallowed_macros.rs:11:5 + --> $DIR/disallowed_macros.rs:14:5 | LL | println!("two"); | ^^^^^^^^^^^^^^^ error: use of a disallowed macro `std::cfg` - --> $DIR/disallowed_macros.rs:12:5 + --> $DIR/disallowed_macros.rs:15:5 | LL | cfg!(unix); | ^^^^^^^^^^ error: use of a disallowed macro `std::vec` - --> $DIR/disallowed_macros.rs:13:5 + --> $DIR/disallowed_macros.rs:16:5 | LL | vec![1, 2, 3]; | ^^^^^^^^^^^^^ error: use of a disallowed macro `serde::Serialize` - --> $DIR/disallowed_macros.rs:15:14 + --> $DIR/disallowed_macros.rs:18:14 | LL | #[derive(Serialize)] | ^^^^^^^^^ @@ -34,43 +34,43 @@ LL | #[derive(Serialize)] = note: no serializing (from clippy.toml) error: use of a disallowed macro `macros::expr` - --> $DIR/disallowed_macros.rs:18:13 + --> $DIR/disallowed_macros.rs:21:13 | LL | let _ = macros::expr!(); | ^^^^^^^^^^^^^^^ error: use of a disallowed macro `macros::stmt` - --> $DIR/disallowed_macros.rs:19:5 + --> $DIR/disallowed_macros.rs:22:5 | LL | macros::stmt!(); | ^^^^^^^^^^^^^^^ error: use of a disallowed macro `macros::pat` - --> $DIR/disallowed_macros.rs:20:9 + --> $DIR/disallowed_macros.rs:23:9 | LL | let macros::pat!() = 1; | ^^^^^^^^^^^^^^ error: use of a disallowed macro `macros::ty` - --> $DIR/disallowed_macros.rs:21:12 + --> $DIR/disallowed_macros.rs:24:12 | LL | let _: macros::ty!() = ""; | ^^^^^^^^^^^^^ error: use of a disallowed macro `macros::item` - --> $DIR/disallowed_macros.rs:22:5 + --> $DIR/disallowed_macros.rs:25:5 | LL | macros::item!(); | ^^^^^^^^^^^^^^^ error: use of a disallowed macro `macros::binop` - --> $DIR/disallowed_macros.rs:23:13 + --> $DIR/disallowed_macros.rs:26:13 | LL | let _ = macros::binop!(1); | ^^^^^^^^^^^^^^^^^ error: use of a disallowed macro `macros::attr` - --> $DIR/disallowed_macros.rs:28:1 + --> $DIR/disallowed_macros.rs:31:1 | LL | / macros::attr! { LL | | struct S; @@ -78,22 +78,28 @@ LL | | } | |_^ error: use of a disallowed macro `macros::item` - --> $DIR/disallowed_macros.rs:33:5 + --> $DIR/disallowed_macros.rs:36:5 | LL | macros::item!(); | ^^^^^^^^^^^^^^^ error: use of a disallowed macro `macros::item` - --> $DIR/disallowed_macros.rs:37:5 + --> $DIR/disallowed_macros.rs:40:5 | LL | macros::item!(); | ^^^^^^^^^^^^^^^ error: use of a disallowed macro `macros::item` - --> $DIR/disallowed_macros.rs:41:5 + --> $DIR/disallowed_macros.rs:44:5 | LL | macros::item!(); | ^^^^^^^^^^^^^^^ -error: aborting due to 15 previous errors +error: use of a disallowed macro `proc_macros::Derive` + --> $DIR/disallowed_macros.rs:47:10 + | +LL | #[derive(Derive)] + | ^^^^^^ + +error: aborting due to 16 previous errors