-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(disallowed_macros): Fix emitting on attr macros #15452
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,18 +4,18 @@ use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then}; | |
use clippy_utils::macros::macro_backtrace; | ||
use clippy_utils::paths::PathNS; | ||
use rustc_data_structures::fx::FxHashSet; | ||
use rustc_hir::attrs::AttributeKind; | ||
use rustc_hir::def::DefKind; | ||
use rustc_hir::def_id::DefIdMap; | ||
use rustc_hir::{ | ||
AmbigArg, Expr, ExprKind, ForeignItem, HirId, ImplItem, Item, ItemKind, OwnerId, Pat, Path, Stmt, TraitItem, Ty, | ||
AmbigArg, Attribute, Expr, ExprKind, ForeignItem, HirId, ImplItem, Item, ItemKind, OwnerId, Pat, Path, Stmt, | ||
TraitItem, Ty, | ||
}; | ||
use rustc_lint::{LateContext, LateLintPass}; | ||
use rustc_middle::ty::TyCtxt; | ||
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 | ||
|
@@ -68,14 +68,10 @@ 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. | ||
early_macro_cache: AttrStorage, | ||
} | ||
|
||
impl DisallowedMacros { | ||
pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf, early_macro_cache: AttrStorage) -> Self { | ||
pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self { | ||
let (disallowed, _) = create_disallowed_map( | ||
tcx, | ||
&conf.disallowed_macros, | ||
|
@@ -88,7 +84,6 @@ impl DisallowedMacros { | |
disallowed, | ||
seen: FxHashSet::default(), | ||
derive_src: None, | ||
early_macro_cache, | ||
} | ||
} | ||
|
||
|
@@ -125,15 +120,83 @@ impl DisallowedMacros { | |
} | ||
|
||
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.early_macro_cache.clone().0.get() { | ||
for span in attr_spans { | ||
self.check(cx, *span, None); | ||
} | ||
} | ||
fn check_attribute(&mut self, cx: &LateContext<'_>, attr: &Attribute) { | ||
let span = match attr { | ||
Attribute::Unparsed(attr_item) => attr_item.span, | ||
Attribute::Parsed(kind) => match kind { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not particularly happy with this giant match block. |
||
AttributeKind::Align { span, .. } | ||
| AttributeKind::AllowConstFnUnstable(_, span) | ||
| AttributeKind::AllowIncoherentImpl(span) | ||
| AttributeKind::AllowInternalUnstable(_, span) | ||
| AttributeKind::AsPtr(span) | ||
| AttributeKind::AutomaticallyDerived(span) | ||
| AttributeKind::BodyStability { span, .. } | ||
| AttributeKind::Coinductive(span) | ||
| AttributeKind::Cold(span) | ||
| AttributeKind::Confusables { first_span: span, .. } | ||
| AttributeKind::ConstContinue(span) | ||
| AttributeKind::ConstStability { span, .. } | ||
| AttributeKind::ConstTrait(span) | ||
| AttributeKind::Coverage(span, _) | ||
| AttributeKind::DenyExplicitImpl(span) | ||
| AttributeKind::Deprecation { span, .. } | ||
| AttributeKind::DoNotImplementViaObject(span) | ||
| AttributeKind::DocComment { span, .. } | ||
| AttributeKind::ExportName { span, .. } | ||
| AttributeKind::FfiConst(span) | ||
| AttributeKind::FfiPure(span) | ||
| AttributeKind::Ignore { span, .. } | ||
| AttributeKind::Inline(_, span) | ||
| AttributeKind::LinkName { span, .. } | ||
| AttributeKind::LinkOrdinal { span, .. } | ||
| AttributeKind::LinkSection { span, .. } | ||
| AttributeKind::LoopMatch(span) | ||
| AttributeKind::MacroEscape(span) | ||
| AttributeKind::MacroUse { span, .. } | ||
| AttributeKind::Marker(span) | ||
| AttributeKind::MayDangle(span) | ||
| AttributeKind::MustUse { span, .. } | ||
| AttributeKind::Naked(span) | ||
| AttributeKind::NoImplicitPrelude(span) | ||
| AttributeKind::NoMangle(span) | ||
| AttributeKind::NonExhaustive(span) | ||
| AttributeKind::Optimize(_, span) | ||
| AttributeKind::ParenSugar(span) | ||
| AttributeKind::PassByValue(span) | ||
| AttributeKind::Path(_, span) | ||
| AttributeKind::Pointee(span) | ||
| AttributeKind::ProcMacro(span) | ||
| AttributeKind::ProcMacroAttribute(span) | ||
| AttributeKind::ProcMacroDerive { span, .. } | ||
| AttributeKind::PubTransparent(span) | ||
| AttributeKind::Repr { first_span: span, .. } | ||
| AttributeKind::RustcBuiltinMacro { span, .. } | ||
| AttributeKind::RustcLayoutScalarValidRangeEnd(_, span) | ||
| AttributeKind::RustcLayoutScalarValidRangeStart(_, span) | ||
| AttributeKind::SkipDuringMethodDispatch { span, .. } | ||
| AttributeKind::SpecializationTrait(span) | ||
| AttributeKind::Stability { span, .. } | ||
| AttributeKind::StdInternalSymbol(span) | ||
| AttributeKind::TargetFeature(_, span) | ||
| AttributeKind::TrackCaller(span) | ||
| AttributeKind::TypeConst(span) | ||
| AttributeKind::UnsafeSpecializationMarker(span) | ||
| AttributeKind::Used { span, .. } => *span, | ||
|
||
AttributeKind::CoherenceIsCore | ||
| AttributeKind::ConstStabilityIndirect | ||
| AttributeKind::Dummy | ||
| AttributeKind::ExportStable | ||
| AttributeKind::Fundamental | ||
| AttributeKind::MacroTransparency(_) | ||
| AttributeKind::RustcObjectLifetimeDefault | ||
| AttributeKind::UnstableFeatureBound(_) => { | ||
return; | ||
}, | ||
}, | ||
}; | ||
self.check(cx, span, self.derive_src); | ||
} | ||
|
||
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
disallowed-macros = [ | ||
"std::panic", | ||
"std::println", | ||
"std::vec", | ||
{ path = "std::cfg" }, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,33 +1,11 @@ | ||
error: use of a disallowed macro `serde::Serialize` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The order here changed. The new order is correct and matches the order that was here prior to a regression that introduced the bug that this PR fixes. |
||
--> tests/ui-toml/disallowed_macros/disallowed_macros.rs:22:14 | ||
| | ||
LL | #[derive(Serialize)] | ||
| ^^^^^^^^^ | ||
| | ||
= note: no serializing | ||
= 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 `macros::attr` | ||
--> tests/ui-toml/disallowed_macros/disallowed_macros.rs:42:1 | ||
| | ||
LL | / macros::attr! { | ||
LL | | | ||
LL | | struct S; | ||
LL | | } | ||
| |_^ | ||
|
||
error: use of a disallowed macro `proc_macros::Derive` | ||
--> tests/ui-toml/disallowed_macros/disallowed_macros.rs:62: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:15:5 | ||
|
@@ -47,6 +25,14 @@ error: use of a disallowed macro `std::vec` | |
LL | vec![1, 2, 3]; | ||
| ^^^^^^^^^^^^^ | ||
|
||
error: use of a disallowed macro `serde::Serialize` | ||
--> tests/ui-toml/disallowed_macros/disallowed_macros.rs:22:14 | ||
| | ||
LL | #[derive(Serialize)] | ||
| ^^^^^^^^^ | ||
| | ||
= note: no serializing | ||
|
||
error: use of a disallowed macro `macros::expr` | ||
--> tests/ui-toml/disallowed_macros/disallowed_macros.rs:26:13 | ||
| | ||
|
@@ -83,6 +69,15 @@ 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:42:1 | ||
| | ||
LL | / macros::attr! { | ||
LL | | | ||
LL | | struct S; | ||
LL | | } | ||
| |_^ | ||
|
||
error: use of a disallowed macro `macros::item` | ||
--> tests/ui-toml/disallowed_macros/disallowed_macros.rs:48:5 | ||
| | ||
|
@@ -101,5 +96,11 @@ 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:62:10 | ||
| | ||
LL | #[derive(Derive)] | ||
| ^^^^^^ | ||
|
||
error: aborting due to 16 previous errors | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
fn test_allow_on_function() { | ||
#![allow(clippy::disallowed_macros)] | ||
|
||
panic!(); | ||
panic!("message"); | ||
panic!("{}", 123); | ||
panic!("{} {}", 123, 456); | ||
println!("test"); | ||
println!("{}", 123); | ||
vec![1, 2, 3]; | ||
} | ||
|
||
fn test_expect_on_function() { | ||
#![expect(clippy::disallowed_macros)] | ||
|
||
panic!(); | ||
panic!("message"); | ||
panic!("{}", 123); | ||
panic!("{} {}", 123, 456); | ||
println!("test"); | ||
println!("{}", 123); | ||
vec![1, 2, 3]; | ||
} | ||
|
||
fn test_no_attributes() { | ||
panic!(); | ||
//~^ disallowed_macros | ||
panic!("message"); | ||
//~^ disallowed_macros | ||
panic!("{}", 123); | ||
//~^ disallowed_macros | ||
panic!("{} {}", 123, 456); | ||
//~^ disallowed_macros | ||
println!("test"); | ||
//~^ disallowed_macros | ||
println!("{}", 123); | ||
//~^ disallowed_macros | ||
vec![1, 2, 3]; | ||
//~^ disallowed_macros | ||
} | ||
|
||
#[allow(clippy::disallowed_macros)] | ||
mod allowed_module { | ||
pub fn test() { | ||
panic!(); | ||
panic!("message"); | ||
panic!("{}", 123); | ||
println!("test"); | ||
vec![1, 2, 3]; | ||
} | ||
} | ||
|
||
#[expect(clippy::disallowed_macros)] | ||
mod expected_module { | ||
pub fn test() { | ||
panic!(); | ||
panic!("message"); | ||
panic!("{}", 123); | ||
println!("test"); | ||
vec![1, 2, 3]; | ||
} | ||
} | ||
|
||
fn main() { | ||
test_allow_on_function(); | ||
test_expect_on_function(); | ||
test_no_attributes(); | ||
allowed_module::test(); | ||
expected_module::test(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
error: use of a disallowed macro `std::panic` | ||
--> tests/ui-toml/disallowed_macros/disallowed_macros_regression.rs:26:5 | ||
| | ||
LL | panic!(); | ||
| ^^^^^^^^ | ||
| | ||
= 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::panic` | ||
--> tests/ui-toml/disallowed_macros/disallowed_macros_regression.rs:28:5 | ||
| | ||
LL | panic!("message"); | ||
| ^^^^^^^^^^^^^^^^^ | ||
|
||
error: use of a disallowed macro `std::panic` | ||
--> tests/ui-toml/disallowed_macros/disallowed_macros_regression.rs:30:5 | ||
| | ||
LL | panic!("{}", 123); | ||
| ^^^^^^^^^^^^^^^^^ | ||
|
||
error: use of a disallowed macro `std::panic` | ||
--> tests/ui-toml/disallowed_macros/disallowed_macros_regression.rs:32:5 | ||
| | ||
LL | panic!("{} {}", 123, 456); | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
error: use of a disallowed macro `std::println` | ||
--> tests/ui-toml/disallowed_macros/disallowed_macros_regression.rs:34:5 | ||
| | ||
LL | println!("test"); | ||
| ^^^^^^^^^^^^^^^^ | ||
|
||
error: use of a disallowed macro `std::println` | ||
--> tests/ui-toml/disallowed_macros/disallowed_macros_regression.rs:36:5 | ||
| | ||
LL | println!("{}", 123); | ||
| ^^^^^^^^^^^^^^^^^^^ | ||
|
||
error: use of a disallowed macro `std::vec` | ||
--> tests/ui-toml/disallowed_macros/disallowed_macros_regression.rs:38:5 | ||
| | ||
LL | vec![1, 2, 3]; | ||
| ^^^^^^^^^^^^^ | ||
|
||
error: aborting due to 7 previous errors | ||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not clear to me what the plan is for
AttrStorage
and its usage inDisallowedMacros
and elsewhere. Since it was introduced early in the attribute refactor, perhaps it was a temporary solution.@jdonszelmann Could you please take a look?