about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSamuel E. Moelius III <sam@moeli.us>2022-03-28 04:32:31 -0400
committerSamuel E. Moelius III <sam@moeli.us>2022-03-28 04:48:12 -0400
commitcb307bbfcd123d3bae8f210f2b51eebe68525c27 (patch)
treeae335b24b014e0e16fdba544522bc6abd798afe9
parent86872059ed143e93f04130578d8b2d1561d5f3a7 (diff)
downloadrust-cb307bbfcd123d3bae8f210f2b51eebe68525c27.tar.gz
rust-cb307bbfcd123d3bae8f210f2b51eebe68525c27.zip
Address review comments
-rw-r--r--clippy_lints/src/crate_in_macro_def.rs60
-rw-r--r--clippy_lints/src/utils/conf.rs2
-rw-r--r--tests/ui/crate_in_macro_def.fixed31
-rw-r--r--tests/ui/crate_in_macro_def.rs31
-rw-r--r--tests/ui/crate_in_macro_def_allow.rs19
-rw-r--r--tests/ui/crate_in_macro_def_allow.stderr0
6 files changed, 102 insertions, 41 deletions
diff --git a/clippy_lints/src/crate_in_macro_def.rs b/clippy_lints/src/crate_in_macro_def.rs
index de9c7ee5f20..21c65f9d6f0 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 b1e474f8056..680b2eb1da7 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 77b43f43260..9fc594be311 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 d72d4c4009c..ac456108e4a 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 d6b30fd9615..00000000000
--- 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 e69de29bb2d..00000000000
--- a/tests/ui/crate_in_macro_def_allow.stderr
+++ /dev/null