From 65a26692fd361b794f528ead645d880527ce3de0 Mon Sep 17 00:00:00 2001 From: "Samuel E. Moelius III" Date: Wed, 23 Mar 2022 21:08:52 -0400 Subject: [PATCH] Add `crate_in_macro_def` lint --- CHANGELOG.md | 1 + clippy_lints/src/crate_in_macro_def.rs | 100 +++++++++++++++++++ clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_correctness.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/utils/conf.rs | 2 +- tests/ui/crate_in_macro_def.fixed | 29 ++++++ tests/ui/crate_in_macro_def.rs | 29 ++++++ tests/ui/crate_in_macro_def.stderr | 14 +++ tests/ui/crate_in_macro_def_allow.rs | 19 ++++ tests/ui/crate_in_macro_def_allow.stderr | 0 12 files changed, 198 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/crate_in_macro_def.rs create mode 100644 tests/ui/crate_in_macro_def.fixed create mode 100644 tests/ui/crate_in_macro_def.rs create mode 100644 tests/ui/crate_in_macro_def.stderr create mode 100644 tests/ui/crate_in_macro_def_allow.rs create mode 100644 tests/ui/crate_in_macro_def_allow.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index dc83de665..9cede8065 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3097,6 +3097,7 @@ Released 2018-09-13 [`comparison_chain`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain [`comparison_to_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_to_empty [`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator +[`crate_in_macro_def`]: https://rust-lang.github.io/rust-clippy/master/index.html#crate_in_macro_def [`create_dir`]: https://rust-lang.github.io/rust-clippy/master/index.html#create_dir [`crosspointer_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#crosspointer_transmute [`dbg_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#dbg_macro diff --git a/clippy_lints/src/crate_in_macro_def.rs b/clippy_lints/src/crate_in_macro_def.rs new file mode 100644 index 000000000..9ac6d6d52 --- /dev/null +++ b/clippy_lints/src/crate_in_macro_def.rs @@ -0,0 +1,100 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use rustc_ast::ast::MacroDef; +use rustc_ast::node_id::NodeId; +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; + +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 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_rules! print_message { + /// () => { + /// println!("{}", crate::MESSAGE); + /// }; + /// } + /// pub const MESSAGE: &str = "Hello!"; + /// ``` + /// Use instead: + /// ```rust + /// macro_rules! print_message { + /// () => { + /// println!("{}", $crate::MESSAGE); + /// }; + /// } + /// pub const MESSAGE: &str = "Hello!"; + /// ``` + #[clippy::version = "1.61.0"] + pub CRATE_IN_MACRO_DEF, + correctness, + "using `crate` in a macro definition" +} +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 contains_unhygienic_crate_reference(tts: &TokenStream) -> Option { + let mut prev_is_dollar = false; + let mut cursor = tts.trees(); + while let Some(curr) = cursor.next() { + if_chain! { + if !prev_is_dollar; + if let Some(span) = is_crate_keyword(&curr); + if let Some(next) = cursor.look_ahead(0); + if is_token(next, &TokenKind::ModSep); + then { + return Some(span); + } + } + if let TokenTree::Delimited(_, _, tts) = &curr { + let span = contains_unhygienic_crate_reference(tts); + if span.is_some() { + return span; + } + } + prev_is_dollar = is_token(&curr, &TokenKind::Dollar); + } + None +} + +fn is_crate_keyword(tt: &TokenTree) -> Option { + if_chain! { + if let TokenTree::Token(Token { kind: TokenKind::Ident(symbol, _), span }) = tt; + if symbol.as_str() == "crate"; + then { Some(*span) } else { None } + } +} + +fn is_token(tt: &TokenTree, kind: &TokenKind) -> bool { + if let TokenTree::Token(Token { kind: other, .. }) = tt { + kind == other + } else { + false + } +} diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 132a46626..1fb3ca1fd 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -37,6 +37,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(comparison_chain::COMPARISON_CHAIN), LintId::of(copies::IFS_SAME_COND), LintId::of(copies::IF_SAME_THEN_ELSE), + LintId::of(crate_in_macro_def::CRATE_IN_MACRO_DEF), LintId::of(default::FIELD_REASSIGN_WITH_DEFAULT), LintId::of(dereference::NEEDLESS_BORROW), LintId::of(derivable_impls::DERIVABLE_IMPLS), diff --git a/clippy_lints/src/lib.register_correctness.rs b/clippy_lints/src/lib.register_correctness.rs index df63f8446..d5cade8fe 100644 --- a/clippy_lints/src/lib.register_correctness.rs +++ b/clippy_lints/src/lib.register_correctness.rs @@ -16,6 +16,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve LintId::of(casts::CAST_SLICE_DIFFERENT_SIZES), LintId::of(copies::IFS_SAME_COND), LintId::of(copies::IF_SAME_THEN_ELSE), + LintId::of(crate_in_macro_def::CRATE_IN_MACRO_DEF), LintId::of(derive::DERIVE_HASH_XOR_EQ), LintId::of(derive::DERIVE_ORD_XOR_PARTIAL_ORD), LintId::of(drop_forget_ref::DROP_COPY), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 65ad64f19..984939a74 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -97,6 +97,7 @@ store.register_lints(&[ copies::IF_SAME_THEN_ELSE, copies::SAME_FUNCTIONS_IN_IF_CONDITION, copy_iterator::COPY_ITERATOR, + crate_in_macro_def::CRATE_IN_MACRO_DEF, create_dir::CREATE_DIR, dbg_macro::DBG_MACRO, default::DEFAULT_TRAIT_ACCESS, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f2a079991..c8b57956b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -190,6 +190,7 @@ mod collapsible_match; mod comparison_chain; mod copies; mod copy_iterator; +mod crate_in_macro_def; mod create_dir; mod dbg_macro; mod default; @@ -867,6 +868,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: ignore_publish: cargo_ignore_publish, }) }); + store.register_early_pass(|| Box::new(crate_in_macro_def::CrateInMacroDef)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 680b2eb1d..b1e474f80 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 new file mode 100644 index 000000000..77b43f432 --- /dev/null +++ b/tests/ui/crate_in_macro_def.fixed @@ -0,0 +1,29 @@ +// run-rustfix +#![warn(clippy::crate_in_macro_def)] + +#[macro_use] +mod hygienic { + macro_rules! print_message_hygienic { + () => { + println!("{}", $crate::hygienic::MESSAGE); + }; + } + + pub const MESSAGE: &str = "Hello!"; +} + +#[macro_use] +mod unhygienic { + macro_rules! print_message_unhygienic { + () => { + println!("{}", $crate::unhygienic::MESSAGE); + }; + } + + pub const MESSAGE: &str = "Hello!"; +} + +fn main() { + print_message_hygienic!(); + print_message_unhygienic!(); +} diff --git a/tests/ui/crate_in_macro_def.rs b/tests/ui/crate_in_macro_def.rs new file mode 100644 index 000000000..d72d4c400 --- /dev/null +++ b/tests/ui/crate_in_macro_def.rs @@ -0,0 +1,29 @@ +// run-rustfix +#![warn(clippy::crate_in_macro_def)] + +#[macro_use] +mod hygienic { + macro_rules! print_message_hygienic { + () => { + println!("{}", $crate::hygienic::MESSAGE); + }; + } + + pub const MESSAGE: &str = "Hello!"; +} + +#[macro_use] +mod unhygienic { + macro_rules! print_message_unhygienic { + () => { + println!("{}", crate::unhygienic::MESSAGE); + }; + } + + pub const MESSAGE: &str = "Hello!"; +} + +fn main() { + print_message_hygienic!(); + print_message_unhygienic!(); +} diff --git a/tests/ui/crate_in_macro_def.stderr b/tests/ui/crate_in_macro_def.stderr new file mode 100644 index 000000000..3f5e4870a --- /dev/null +++ b/tests/ui/crate_in_macro_def.stderr @@ -0,0 +1,14 @@ +error: reference to the macro call's crate, which is rarely intended + --> $DIR/crate_in_macro_def.rs:19:28 + | +LL | println!("{}", crate::unhygienic::MESSAGE); + | ^^^^^ + | + = note: `-D clippy::crate-in-macro-def` implied by `-D warnings` +help: if reference to the macro definition's crate is intended, use + | +LL | println!("{}", $crate::unhygienic::MESSAGE); + | ~~~~~~ + +error: aborting due to previous error + diff --git a/tests/ui/crate_in_macro_def_allow.rs b/tests/ui/crate_in_macro_def_allow.rs new file mode 100644 index 000000000..d6b30fd96 --- /dev/null +++ b/tests/ui/crate_in_macro_def_allow.rs @@ -0,0 +1,19 @@ +#![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 new file mode 100644 index 000000000..e69de29bb