From c93109674f8408d621a4b44137f2a6fd00db62a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonathan=20D=C3=B6nszelmann?= Date: Tue, 5 Nov 2024 20:44:53 +0100 Subject: [PATCH] collect attribute spans early for disallowed macros --- clippy_lints/src/disallowed_macros.rs | 23 +++++-- clippy_lints/src/lib.rs | 9 ++- clippy_lints/src/utils/attr_collector.rs | 40 ++++++++++++ clippy_lints/src/utils/mod.rs | 2 + .../disallowed_macros.stderr | 62 +++++++++---------- 5 files changed, 98 insertions(+), 38 deletions(-) create mode 100644 clippy_lints/src/utils/attr_collector.rs diff --git a/clippy_lints/src/disallowed_macros.rs b/clippy_lints/src/disallowed_macros.rs index b51d343132b2..bdd49bf8aa7c 100644 --- a/clippy_lints/src/disallowed_macros.rs +++ b/clippy_lints/src/disallowed_macros.rs @@ -2,7 +2,6 @@ use clippy_config::Conf; use clippy_utils::create_disallowed_map; 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::Diag; use rustc_hir::def_id::DefIdMap; @@ -14,6 +13,8 @@ use rustc_middle::ty::TyCtxt; use rustc_session::impl_lint_pass; use rustc_span::{ExpnId, MacroKind, Span}; +use crate::utils::attr_collector::AttrStorage; + declare_clippy_lint! { /// ### What it does /// Denies the configured macros in clippy.toml @@ -64,14 +65,19 @@ pub struct DisallowedMacros { // Track the most recently seen node that can have a `derive` attribute. // Needed to use the correct lint level. derive_src: Option, + + // When a macro is disallowed in an early pass, it's stored + // and emitted during the late pass. This happens for attributes. + earlies: AttrStorage, } impl DisallowedMacros { - pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self { + pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf, earlies: AttrStorage) -> Self { Self { disallowed: create_disallowed_map(tcx, &conf.disallowed_macros), seen: FxHashSet::default(), derive_src: None, + earlies, } } @@ -114,6 +120,15 @@ impl DisallowedMacros { impl_lint_pass!(DisallowedMacros => [DISALLOWED_MACROS]); impl LateLintPass<'_> for DisallowedMacros { + fn check_crate(&mut self, cx: &LateContext<'_>) { + // once we check a crate in the late pass we can emit the early pass lints + if let Some(attr_spans) = self.earlies.clone().0.get() { + for span in attr_spans { + self.check(cx, *span, None); + } + } + } + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { self.check(cx, expr.span, None); // `$t + $t` can have the context of $t, check also the span of the binary operator @@ -164,8 +179,4 @@ impl LateLintPass<'_> for DisallowedMacros { fn check_path(&mut self, cx: &LateContext<'_>, path: &Path<'_>, _: HirId) { self.check(cx, path.span, None); } - - fn check_attribute(&mut self, cx: &LateContext<'_>, attr: &Attribute) { - self.check(cx, attr.span, self.derive_src); - } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index a0ff8316d5cd..0a5f58743a9e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -399,6 +399,7 @@ use clippy_config::{Conf, get_configuration_metadata, sanitize_explanation}; use clippy_utils::macros::FormatArgsStorage; use rustc_data_structures::fx::FxHashSet; use rustc_lint::{Lint, LintId}; +use utils::attr_collector::{AttrCollector, AttrStorage}; /// Register all pre expansion lints /// @@ -461,6 +462,7 @@ pub(crate) enum LintCategory { #[cfg(feature = "internal")] Internal, } + #[allow(clippy::enum_glob_use)] use LintCategory::*; @@ -582,6 +584,10 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { )) }); + let attr_storage = AttrStorage::default(); + let attrs = attr_storage.clone(); + store.register_early_pass(move || Box::new(AttrCollector::new(attrs.clone()))); + // all the internal lints #[cfg(feature = "internal")] { @@ -791,7 +797,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(unwrap_in_result::UnwrapInResult)); store.register_late_pass(|_| Box::new(semicolon_if_nothing_returned::SemicolonIfNothingReturned)); store.register_late_pass(|_| Box::new(async_yields_async::AsyncYieldsAsync)); - store.register_late_pass(move |tcx| Box::new(disallowed_macros::DisallowedMacros::new(tcx, conf))); + let attrs = attr_storage.clone(); + store.register_late_pass(move |tcx| Box::new(disallowed_macros::DisallowedMacros::new(tcx, conf, attrs.clone()))); store.register_late_pass(move |tcx| Box::new(disallowed_methods::DisallowedMethods::new(tcx, conf))); store.register_early_pass(|| Box::new(asm_syntax::InlineAsmX86AttSyntax)); store.register_early_pass(|| Box::new(asm_syntax::InlineAsmX86IntelSyntax)); diff --git a/clippy_lints/src/utils/attr_collector.rs b/clippy_lints/src/utils/attr_collector.rs new file mode 100644 index 000000000000..1522553bbf52 --- /dev/null +++ b/clippy_lints/src/utils/attr_collector.rs @@ -0,0 +1,40 @@ +use std::mem; +use std::sync::OnceLock; + +use rustc_ast::{Attribute, Crate}; +use rustc_data_structures::sync::Lrc; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_session::impl_lint_pass; +use rustc_span::Span; + +#[derive(Clone, Default)] +pub struct AttrStorage(pub Lrc>>); + +pub struct AttrCollector { + storage: AttrStorage, + attrs: Vec, +} + +impl AttrCollector { + pub fn new(storage: AttrStorage) -> Self { + Self { + storage, + attrs: Vec::new(), + } + } +} + +impl_lint_pass!(AttrCollector => []); + +impl EarlyLintPass for AttrCollector { + fn check_attribute(&mut self, _cx: &EarlyContext<'_>, attr: &Attribute) { + self.attrs.push(attr.span); + } + + fn check_crate_post(&mut self, _: &EarlyContext<'_>, _: &Crate) { + self.storage + .0 + .set(mem::take(&mut self.attrs)) + .expect("should only be called once"); + } +} diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 13e9ead9a57f..4476cd1005e7 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -1,5 +1,7 @@ +pub mod attr_collector; pub mod author; pub mod dump_hir; pub mod format_args_collector; + #[cfg(feature = "internal")] pub mod internal_lints; diff --git a/tests/ui-toml/disallowed_macros/disallowed_macros.stderr b/tests/ui-toml/disallowed_macros/disallowed_macros.stderr index ddeb2f8cc704..2e3386628b4d 100644 --- a/tests/ui-toml/disallowed_macros/disallowed_macros.stderr +++ b/tests/ui-toml/disallowed_macros/disallowed_macros.stderr @@ -1,11 +1,39 @@ +error: use of a disallowed macro `std::vec` + --> tests/ui-toml/disallowed_macros/disallowed_macros.rs:16:5 + | +LL | vec![1, 2, 3]; + | ^^^^^^^^^^^^^ + | + = note: `-D clippy::disallowed-macros` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::disallowed_macros)]` + +error: use of a disallowed macro `serde::Serialize` + --> tests/ui-toml/disallowed_macros/disallowed_macros.rs:18:14 + | +LL | #[derive(Serialize)] + | ^^^^^^^^^ + | + = note: no serializing + +error: use of a disallowed macro `macros::attr` + --> tests/ui-toml/disallowed_macros/disallowed_macros.rs:31:1 + | +LL | / macros::attr! { +LL | | struct S; +LL | | } + | |_^ + +error: use of a disallowed macro `proc_macros::Derive` + --> tests/ui-toml/disallowed_macros/disallowed_macros.rs:47:10 + | +LL | #[derive(Derive)] + | ^^^^^^ + error: use of a disallowed macro `std::println` --> tests/ui-toml/disallowed_macros/disallowed_macros.rs:13:5 | LL | println!("one"); | ^^^^^^^^^^^^^^^ - | - = note: `-D clippy::disallowed-macros` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::disallowed_macros)]` error: use of a disallowed macro `std::println` --> tests/ui-toml/disallowed_macros/disallowed_macros.rs:14:5 @@ -19,20 +47,6 @@ error: use of a disallowed macro `std::cfg` LL | cfg!(unix); | ^^^^^^^^^^ -error: use of a disallowed macro `std::vec` - --> tests/ui-toml/disallowed_macros/disallowed_macros.rs:16:5 - | -LL | vec![1, 2, 3]; - | ^^^^^^^^^^^^^ - -error: use of a disallowed macro `serde::Serialize` - --> tests/ui-toml/disallowed_macros/disallowed_macros.rs:18:14 - | -LL | #[derive(Serialize)] - | ^^^^^^^^^ - | - = note: no serializing - error: use of a disallowed macro `macros::expr` --> tests/ui-toml/disallowed_macros/disallowed_macros.rs:21:13 | @@ -69,14 +83,6 @@ error: use of a disallowed macro `macros::binop` LL | let _ = macros::binop!(1); | ^^^^^^^^^^^^^^^^^ -error: use of a disallowed macro `macros::attr` - --> tests/ui-toml/disallowed_macros/disallowed_macros.rs:31:1 - | -LL | / macros::attr! { -LL | | struct S; -LL | | } - | |_^ - error: use of a disallowed macro `macros::item` --> tests/ui-toml/disallowed_macros/disallowed_macros.rs:36:5 | @@ -95,11 +101,5 @@ error: use of a disallowed macro `macros::item` LL | macros::item!(); | ^^^^^^^^^^^^^^^ -error: use of a disallowed macro `proc_macros::Derive` - --> tests/ui-toml/disallowed_macros/disallowed_macros.rs:47:10 - | -LL | #[derive(Derive)] - | ^^^^^^ - error: aborting due to 16 previous errors