diff --git a/clippy_lints/src/crate_in_macro_def.rs b/clippy_lints/src/crate_in_macro_def.rs index de9c7ee5f..21c65f9d6 100644 --- a/clippy_lints/src/crate_in_macro_def.rs +++ b/clippy_lints/src/crate_in_macro_def.rs @@ -1,24 +1,24 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use rustc_ast::ast::MacroDef; -use rustc_ast::node_id::NodeId; +use rustc_ast::ast::{AttrKind, Attribute, Item, ItemKind}; use rustc_ast::token::{Token, TokenKind}; use rustc_ast::tokenstream::{TokenStream, TokenTree}; use rustc_errors::Applicability; use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::Span; +use rustc_span::{symbol::sym, Span}; declare_clippy_lint! { /// ### What it does /// Checks for use of `crate` as opposed to `$crate` in a macro definition. /// /// ### Why is this bad? - /// `crate` refers to the macro call's crate, whereas `$crate` refers to the macro - /// definition's crate. Rarely is the former intended. See: + /// `crate` refers to the macro call's crate, whereas `$crate` refers to the macro definition's + /// crate. Rarely is the former intended. See: /// https://doc.rust-lang.org/reference/macros-by-example.html#hygiene /// /// ### Example /// ```rust + /// #[macro_export] /// macro_rules! print_message { /// () => { /// println!("{}", crate::MESSAGE); @@ -28,6 +28,7 @@ declare_clippy_lint! { /// ``` /// Use instead: /// ```rust + /// #[macro_export] /// macro_rules! print_message { /// () => { /// println!("{}", $crate::MESSAGE); @@ -35,6 +36,13 @@ declare_clippy_lint! { /// } /// pub const MESSAGE: &str = "Hello!"; /// ``` + /// + /// Note that if the use of `crate` is intentional, an `allow` attribute can be applied to the + /// macro definition, e.g.: + /// ```rust,ignore + /// #[allow(clippy::crate_in_macro_def)] + /// macro_rules! ok { ... crate::foo ... } + /// ``` #[clippy::version = "1.61.0"] pub CRATE_IN_MACRO_DEF, correctness, @@ -43,18 +51,36 @@ declare_clippy_lint! { declare_lint_pass!(CrateInMacroDef => [CRATE_IN_MACRO_DEF]); impl EarlyLintPass for CrateInMacroDef { - fn check_mac_def(&mut self, cx: &EarlyContext<'_>, macro_def: &MacroDef, _: NodeId) { - let tts = macro_def.body.inner_tokens(); - if let Some(span) = contains_unhygienic_crate_reference(&tts) { - span_lint_and_sugg( - cx, - CRATE_IN_MACRO_DEF, - span, - "reference to the macro call's crate, which is rarely intended", - "if reference to the macro definition's crate is intended, use", - String::from("$crate"), - Applicability::MachineApplicable, - ); + fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { + if_chain! { + if item.attrs.iter().any(is_macro_export); + if let ItemKind::MacroDef(macro_def) = &item.kind; + let tts = macro_def.body.inner_tokens(); + if let Some(span) = contains_unhygienic_crate_reference(&tts); + then { + span_lint_and_sugg( + cx, + CRATE_IN_MACRO_DEF, + span, + "reference to the macro call's crate, which is rarely intended", + "if reference to the macro definition's crate is intended, use", + String::from("$crate"), + Applicability::MachineApplicable, + ); + } + } + } +} + +fn is_macro_export(attr: &Attribute) -> bool { + if_chain! { + if let AttrKind::Normal(attr_item, _) = &attr.kind; + if let [segment] = attr_item.path.segments.as_slice(); + if segment.ident.name == sym::macro_export; + then { + true + } else { + false } } } diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index b1e474f80..680b2eb1d 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -123,7 +123,7 @@ macro_rules! define_Conf { #[cfg(feature = "internal")] pub mod metadata { - use $crate::utils::internal_lints::metadata_collector::ClippyConfiguration; + use crate::utils::internal_lints::metadata_collector::ClippyConfiguration; macro_rules! wrap_option { () => (None); diff --git a/tests/ui/crate_in_macro_def.fixed b/tests/ui/crate_in_macro_def.fixed index 77b43f432..9fc594be3 100644 --- a/tests/ui/crate_in_macro_def.fixed +++ b/tests/ui/crate_in_macro_def.fixed @@ -1,8 +1,8 @@ // run-rustfix #![warn(clippy::crate_in_macro_def)] -#[macro_use] mod hygienic { + #[macro_export] macro_rules! print_message_hygienic { () => { println!("{}", $crate::hygienic::MESSAGE); @@ -12,8 +12,8 @@ mod hygienic { pub const MESSAGE: &str = "Hello!"; } -#[macro_use] mod unhygienic { + #[macro_export] macro_rules! print_message_unhygienic { () => { println!("{}", $crate::unhygienic::MESSAGE); @@ -23,7 +23,34 @@ mod unhygienic { pub const MESSAGE: &str = "Hello!"; } +mod unhygienic_intentionally { + // For cases where the use of `crate` is intentional, applying `allow` to the macro definition + // should suppress the lint. + #[allow(clippy::crate_in_macro_def)] + #[macro_export] + macro_rules! print_message_unhygienic_intentionally { + () => { + println!("{}", crate::CALLER_PROVIDED_MESSAGE); + }; + } +} + +#[macro_use] +mod not_exported { + macro_rules! print_message_not_exported { + () => { + println!("{}", crate::not_exported::MESSAGE); + }; + } + + pub const MESSAGE: &str = "Hello!"; +} + fn main() { print_message_hygienic!(); print_message_unhygienic!(); + print_message_unhygienic_intentionally!(); + print_message_not_exported!(); } + +pub const CALLER_PROVIDED_MESSAGE: &str = "Hello!"; diff --git a/tests/ui/crate_in_macro_def.rs b/tests/ui/crate_in_macro_def.rs index d72d4c400..ac456108e 100644 --- a/tests/ui/crate_in_macro_def.rs +++ b/tests/ui/crate_in_macro_def.rs @@ -1,8 +1,8 @@ // run-rustfix #![warn(clippy::crate_in_macro_def)] -#[macro_use] mod hygienic { + #[macro_export] macro_rules! print_message_hygienic { () => { println!("{}", $crate::hygienic::MESSAGE); @@ -12,8 +12,8 @@ mod hygienic { pub const MESSAGE: &str = "Hello!"; } -#[macro_use] mod unhygienic { + #[macro_export] macro_rules! print_message_unhygienic { () => { println!("{}", crate::unhygienic::MESSAGE); @@ -23,7 +23,34 @@ mod unhygienic { pub const MESSAGE: &str = "Hello!"; } +mod unhygienic_intentionally { + // For cases where the use of `crate` is intentional, applying `allow` to the macro definition + // should suppress the lint. + #[allow(clippy::crate_in_macro_def)] + #[macro_export] + macro_rules! print_message_unhygienic_intentionally { + () => { + println!("{}", crate::CALLER_PROVIDED_MESSAGE); + }; + } +} + +#[macro_use] +mod not_exported { + macro_rules! print_message_not_exported { + () => { + println!("{}", crate::not_exported::MESSAGE); + }; + } + + pub const MESSAGE: &str = "Hello!"; +} + fn main() { print_message_hygienic!(); print_message_unhygienic!(); + print_message_unhygienic_intentionally!(); + print_message_not_exported!(); } + +pub const CALLER_PROVIDED_MESSAGE: &str = "Hello!"; diff --git a/tests/ui/crate_in_macro_def_allow.rs b/tests/ui/crate_in_macro_def_allow.rs deleted file mode 100644 index d6b30fd96..000000000 --- a/tests/ui/crate_in_macro_def_allow.rs +++ /dev/null @@ -1,19 +0,0 @@ -#![warn(clippy::crate_in_macro_def)] - -#[macro_use] -mod intentional { - // For cases where use of `crate` is intentional, applying `allow` to the macro definition - // should suppress the lint. - #[allow(clippy::crate_in_macro_def)] - macro_rules! print_message { - () => { - println!("{}", crate::CALLER_PROVIDED_MESSAGE); - }; - } -} - -fn main() { - print_message!(); -} - -pub const CALLER_PROVIDED_MESSAGE: &str = "Hello!"; diff --git a/tests/ui/crate_in_macro_def_allow.stderr b/tests/ui/crate_in_macro_def_allow.stderr deleted file mode 100644 index e69de29bb..000000000