Address review comments

This commit is contained in:
Samuel E. Moelius III 2022-03-28 04:32:31 -04:00
parent 86872059ed
commit cb307bbfcd
6 changed files with 102 additions and 41 deletions

View file

@ -1,24 +1,24 @@
use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::diagnostics::span_lint_and_sugg;
use rustc_ast::ast::MacroDef; use rustc_ast::ast::{AttrKind, Attribute, Item, ItemKind};
use rustc_ast::node_id::NodeId;
use rustc_ast::token::{Token, TokenKind}; use rustc_ast::token::{Token, TokenKind};
use rustc_ast::tokenstream::{TokenStream, TokenTree}; use rustc_ast::tokenstream::{TokenStream, TokenTree};
use rustc_errors::Applicability; use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::Span; use rustc_span::{symbol::sym, Span};
declare_clippy_lint! { declare_clippy_lint! {
/// ### What it does /// ### What it does
/// Checks for use of `crate` as opposed to `$crate` in a macro definition. /// Checks for use of `crate` as opposed to `$crate` in a macro definition.
/// ///
/// ### Why is this bad? /// ### Why is this bad?
/// `crate` refers to the macro call's crate, whereas `$crate` refers to the macro /// `crate` refers to the macro call's crate, whereas `$crate` refers to the macro definition's
/// definition's crate. Rarely is the former intended. See: /// crate. Rarely is the former intended. See:
/// https://doc.rust-lang.org/reference/macros-by-example.html#hygiene /// https://doc.rust-lang.org/reference/macros-by-example.html#hygiene
/// ///
/// ### Example /// ### Example
/// ```rust /// ```rust
/// #[macro_export]
/// macro_rules! print_message { /// macro_rules! print_message {
/// () => { /// () => {
/// println!("{}", crate::MESSAGE); /// println!("{}", crate::MESSAGE);
@ -28,6 +28,7 @@ declare_clippy_lint! {
/// ``` /// ```
/// Use instead: /// Use instead:
/// ```rust /// ```rust
/// #[macro_export]
/// macro_rules! print_message { /// macro_rules! print_message {
/// () => { /// () => {
/// println!("{}", $crate::MESSAGE); /// println!("{}", $crate::MESSAGE);
@ -35,6 +36,13 @@ declare_clippy_lint! {
/// } /// }
/// pub const MESSAGE: &str = "Hello!"; /// 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"] #[clippy::version = "1.61.0"]
pub CRATE_IN_MACRO_DEF, pub CRATE_IN_MACRO_DEF,
correctness, correctness,
@ -43,9 +51,13 @@ declare_clippy_lint! {
declare_lint_pass!(CrateInMacroDef => [CRATE_IN_MACRO_DEF]); declare_lint_pass!(CrateInMacroDef => [CRATE_IN_MACRO_DEF]);
impl EarlyLintPass for CrateInMacroDef { impl EarlyLintPass for CrateInMacroDef {
fn check_mac_def(&mut self, cx: &EarlyContext<'_>, macro_def: &MacroDef, _: NodeId) { 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(); let tts = macro_def.body.inner_tokens();
if let Some(span) = contains_unhygienic_crate_reference(&tts) { if let Some(span) = contains_unhygienic_crate_reference(&tts);
then {
span_lint_and_sugg( span_lint_and_sugg(
cx, cx,
CRATE_IN_MACRO_DEF, CRATE_IN_MACRO_DEF,
@ -58,6 +70,20 @@ impl EarlyLintPass for CrateInMacroDef {
} }
} }
} }
}
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
}
}
}
fn contains_unhygienic_crate_reference(tts: &TokenStream) -> Option<Span> { fn contains_unhygienic_crate_reference(tts: &TokenStream) -> Option<Span> {
let mut prev_is_dollar = false; let mut prev_is_dollar = false;

View file

@ -123,7 +123,7 @@ macro_rules! define_Conf {
#[cfg(feature = "internal")] #[cfg(feature = "internal")]
pub mod metadata { pub mod metadata {
use $crate::utils::internal_lints::metadata_collector::ClippyConfiguration; use crate::utils::internal_lints::metadata_collector::ClippyConfiguration;
macro_rules! wrap_option { macro_rules! wrap_option {
() => (None); () => (None);

View file

@ -1,8 +1,8 @@
// run-rustfix // run-rustfix
#![warn(clippy::crate_in_macro_def)] #![warn(clippy::crate_in_macro_def)]
#[macro_use]
mod hygienic { mod hygienic {
#[macro_export]
macro_rules! print_message_hygienic { macro_rules! print_message_hygienic {
() => { () => {
println!("{}", $crate::hygienic::MESSAGE); println!("{}", $crate::hygienic::MESSAGE);
@ -12,8 +12,8 @@ mod hygienic {
pub const MESSAGE: &str = "Hello!"; pub const MESSAGE: &str = "Hello!";
} }
#[macro_use]
mod unhygienic { mod unhygienic {
#[macro_export]
macro_rules! print_message_unhygienic { macro_rules! print_message_unhygienic {
() => { () => {
println!("{}", $crate::unhygienic::MESSAGE); println!("{}", $crate::unhygienic::MESSAGE);
@ -23,7 +23,34 @@ mod unhygienic {
pub const MESSAGE: &str = "Hello!"; 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() { fn main() {
print_message_hygienic!(); print_message_hygienic!();
print_message_unhygienic!(); print_message_unhygienic!();
print_message_unhygienic_intentionally!();
print_message_not_exported!();
} }
pub const CALLER_PROVIDED_MESSAGE: &str = "Hello!";

View file

@ -1,8 +1,8 @@
// run-rustfix // run-rustfix
#![warn(clippy::crate_in_macro_def)] #![warn(clippy::crate_in_macro_def)]
#[macro_use]
mod hygienic { mod hygienic {
#[macro_export]
macro_rules! print_message_hygienic { macro_rules! print_message_hygienic {
() => { () => {
println!("{}", $crate::hygienic::MESSAGE); println!("{}", $crate::hygienic::MESSAGE);
@ -12,8 +12,8 @@ mod hygienic {
pub const MESSAGE: &str = "Hello!"; pub const MESSAGE: &str = "Hello!";
} }
#[macro_use]
mod unhygienic { mod unhygienic {
#[macro_export]
macro_rules! print_message_unhygienic { macro_rules! print_message_unhygienic {
() => { () => {
println!("{}", crate::unhygienic::MESSAGE); println!("{}", crate::unhygienic::MESSAGE);
@ -23,7 +23,34 @@ mod unhygienic {
pub const MESSAGE: &str = "Hello!"; 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() { fn main() {
print_message_hygienic!(); print_message_hygienic!();
print_message_unhygienic!(); print_message_unhygienic!();
print_message_unhygienic_intentionally!();
print_message_not_exported!();
} }
pub const CALLER_PROVIDED_MESSAGE: &str = "Hello!";

View file

@ -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!";