Merge pull request #13657 from jdonszelmann/disallowed-macros-to-early

collect attribute spans early for disallowed macros

clippy_lints/src/disallowed_macros.rs

@@ -2,7 +2,6 @@
 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_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<OwnerId>,
+
+    // 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 @@ fn check(&mut self, cx: &LateContext<'_>, span: Span, derive_src: Option<OwnerId
 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 @@ fn check_trait_item(&mut self, cx: &LateContext<'_>, item: &TraitItem<'_>) {
     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);
-    }
 }

clippy_lints/src/lib.rs

@@ -401,6 +401,7 @@
 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
 ///
@@ -465,6 +466,7 @@ pub(crate) enum LintCategory {
     #[cfg(feature = "internal")]
     Internal,
 }
+
 #[allow(clippy::enum_glob_use)]
 use LintCategory::*;
 
@@ -586,6 +588,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")]
     {
@@ -797,7 +803,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));

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<OnceLock<Vec<Span>>>);
+
+pub struct AttrCollector {
+    storage: AttrStorage,
+    attrs: Vec<Span>,
+}
+
+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");
+    }
+}

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;

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