Auto merge of #12267 - Jarcho:issue_12254, r=Alexendoo

Don't allow derive macros to silence `disallowed_macros`

fixes #12254

The implementation is a bit of a hack, but "works". A derive expanding to another derive won't work properly, but we shouldn't be linting those anyways.

changelog: `disallowed_macros`: Don't allow derive macros to silence their own expansion
This commit is contained in:
bors 2024-02-12 13:19:52 +00:00
commit b0e7640fd7
5 changed files with 112 additions and 46 deletions

View File

@ -1,13 +1,16 @@
use clippy_config::types::DisallowedPath;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::macros::macro_backtrace;
use rustc_ast::Attribute;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Diagnostic;
use rustc_hir::def_id::DefIdMap;
use rustc_hir::{Expr, ExprKind, ForeignItem, HirId, ImplItem, Item, Pat, Path, Stmt, TraitItem, Ty};
use rustc_hir::{
Expr, ExprKind, ForeignItem, HirId, ImplItem, Item, ItemKind, OwnerId, Pat, Path, Stmt, TraitItem, Ty,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::impl_lint_pass;
use rustc_span::{ExpnId, Span};
use rustc_span::{ExpnId, MacroKind, Span};
declare_clippy_lint! {
/// ### What it does
@ -57,6 +60,10 @@ pub struct DisallowedMacros {
conf_disallowed: Vec<DisallowedPath>,
disallowed: DefIdMap<usize>,
seen: FxHashSet<ExpnId>,
// Track the most recently seen node that can have a `derive` attribute.
// Needed to use the correct lint level.
derive_src: Option<OwnerId>,
}
impl DisallowedMacros {
@ -65,10 +72,11 @@ impl DisallowedMacros {
conf_disallowed,
disallowed: DefIdMap::default(),
seen: FxHashSet::default(),
derive_src: None,
}
}
fn check(&mut self, cx: &LateContext<'_>, span: Span) {
fn check(&mut self, cx: &LateContext<'_>, span: Span, derive_src: Option<OwnerId>) {
if self.conf_disallowed.is_empty() {
return;
}
@ -80,18 +88,26 @@ impl DisallowedMacros {
if let Some(&index) = self.disallowed.get(&mac.def_id) {
let conf = &self.conf_disallowed[index];
span_lint_and_then(
cx,
DISALLOWED_MACROS,
mac.span,
&format!("use of a disallowed macro `{}`", conf.path()),
|diag| {
if let Some(reason) = conf.reason() {
diag.note(reason);
}
},
);
let msg = format!("use of a disallowed macro `{}`", conf.path());
let add_note = |diag: &mut Diagnostic| {
if let Some(reason) = conf.reason() {
diag.note(reason);
}
};
if matches!(mac.kind, MacroKind::Derive)
&& let Some(derive_src) = derive_src
{
span_lint_hir_and_then(
cx,
DISALLOWED_MACROS,
cx.tcx.local_def_id_to_hir_id(derive_src.def_id),
mac.span,
&msg,
add_note,
);
} else {
span_lint_and_then(cx, DISALLOWED_MACROS, mac.span, &msg, add_note);
}
}
}
}
@ -110,49 +126,57 @@ impl LateLintPass<'_> for DisallowedMacros {
}
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
self.check(cx, expr.span);
self.check(cx, expr.span, None);
// `$t + $t` can have the context of $t, check also the span of the binary operator
if let ExprKind::Binary(op, ..) = expr.kind {
self.check(cx, op.span);
self.check(cx, op.span, None);
}
}
fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) {
self.check(cx, stmt.span);
self.check(cx, stmt.span, None);
}
fn check_ty(&mut self, cx: &LateContext<'_>, ty: &Ty<'_>) {
self.check(cx, ty.span);
self.check(cx, ty.span, None);
}
fn check_pat(&mut self, cx: &LateContext<'_>, pat: &Pat<'_>) {
self.check(cx, pat.span);
self.check(cx, pat.span, None);
}
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
self.check(cx, item.span);
self.check(cx, item.vis_span);
self.check(cx, item.span, self.derive_src);
self.check(cx, item.vis_span, None);
if matches!(
item.kind,
ItemKind::Struct(..) | ItemKind::Enum(..) | ItemKind::Union(..)
) && macro_backtrace(item.span).all(|m| !matches!(m.kind, MacroKind::Derive))
{
self.derive_src = Some(item.owner_id);
}
}
fn check_foreign_item(&mut self, cx: &LateContext<'_>, item: &ForeignItem<'_>) {
self.check(cx, item.span);
self.check(cx, item.vis_span);
self.check(cx, item.span, None);
self.check(cx, item.vis_span, None);
}
fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &ImplItem<'_>) {
self.check(cx, item.span);
self.check(cx, item.vis_span);
self.check(cx, item.span, None);
self.check(cx, item.vis_span, None);
}
fn check_trait_item(&mut self, cx: &LateContext<'_>, item: &TraitItem<'_>) {
self.check(cx, item.span);
self.check(cx, item.span, None);
}
fn check_path(&mut self, cx: &LateContext<'_>, path: &Path<'_>, _: HirId) {
self.check(cx, path.span);
self.check(cx, path.span, None);
}
fn check_attribute(&mut self, cx: &LateContext<'_>, attr: &Attribute) {
self.check(cx, attr.span);
self.check(cx, attr.span, self.derive_src);
}
}

View File

@ -0,0 +1,29 @@
extern crate proc_macro;
use proc_macro::Delimiter::{Brace, Bracket, Parenthesis};
use proc_macro::Spacing::{Alone, Joint};
use proc_macro::{Group, Ident, Punct, Span, TokenStream, TokenTree as TT};
#[proc_macro_derive(Derive)]
pub fn derive(_: TokenStream) -> TokenStream {
TokenStream::from_iter([
TT::from(Punct::new('#', Alone)),
TT::from(Group::new(
Bracket,
TokenStream::from_iter([
TT::from(Ident::new("allow", Span::call_site())),
TT::from(Group::new(
Parenthesis,
TokenStream::from_iter([
TT::from(Ident::new("clippy", Span::call_site())),
TT::from(Punct::new(':', Joint)),
TT::from(Punct::new(':', Alone)),
TT::from(Ident::new("disallowed_macros", Span::call_site())),
]),
)),
]),
)),
TT::from(Ident::new("impl", Span::call_site())),
TT::from(Ident::new("Foo", Span::call_site())),
TT::from(Group::new(Brace, TokenStream::new())),
])
}

View File

@ -10,4 +10,5 @@ disallowed-macros = [
"macros::item",
"macros::binop",
"macros::attr",
"proc_macros::Derive",
]

View File

@ -1,9 +1,12 @@
//@aux-build:macros.rs
//@aux-build:proc_macros.rs
#![allow(unused)]
extern crate macros;
extern crate proc_macros;
use proc_macros::Derive;
use serde::Serialize;
fn main() {
@ -40,3 +43,6 @@ trait Y {
impl Y for S {
macros::item!();
}
#[derive(Derive)]
struct Foo;

View File

@ -1,5 +1,5 @@
error: use of a disallowed macro `std::println`
--> $DIR/disallowed_macros.rs:10:5
--> $DIR/disallowed_macros.rs:13:5
|
LL | println!("one");
| ^^^^^^^^^^^^^^^
@ -8,25 +8,25 @@ LL | println!("one");
= help: to override `-D warnings` add `#[allow(clippy::disallowed_macros)]`
error: use of a disallowed macro `std::println`
--> $DIR/disallowed_macros.rs:11:5
--> $DIR/disallowed_macros.rs:14:5
|
LL | println!("two");
| ^^^^^^^^^^^^^^^
error: use of a disallowed macro `std::cfg`
--> $DIR/disallowed_macros.rs:12:5
--> $DIR/disallowed_macros.rs:15:5
|
LL | cfg!(unix);
| ^^^^^^^^^^
error: use of a disallowed macro `std::vec`
--> $DIR/disallowed_macros.rs:13:5
--> $DIR/disallowed_macros.rs:16:5
|
LL | vec![1, 2, 3];
| ^^^^^^^^^^^^^
error: use of a disallowed macro `serde::Serialize`
--> $DIR/disallowed_macros.rs:15:14
--> $DIR/disallowed_macros.rs:18:14
|
LL | #[derive(Serialize)]
| ^^^^^^^^^
@ -34,43 +34,43 @@ LL | #[derive(Serialize)]
= note: no serializing (from clippy.toml)
error: use of a disallowed macro `macros::expr`
--> $DIR/disallowed_macros.rs:18:13
--> $DIR/disallowed_macros.rs:21:13
|
LL | let _ = macros::expr!();
| ^^^^^^^^^^^^^^^
error: use of a disallowed macro `macros::stmt`
--> $DIR/disallowed_macros.rs:19:5
--> $DIR/disallowed_macros.rs:22:5
|
LL | macros::stmt!();
| ^^^^^^^^^^^^^^^
error: use of a disallowed macro `macros::pat`
--> $DIR/disallowed_macros.rs:20:9
--> $DIR/disallowed_macros.rs:23:9
|
LL | let macros::pat!() = 1;
| ^^^^^^^^^^^^^^
error: use of a disallowed macro `macros::ty`
--> $DIR/disallowed_macros.rs:21:12
--> $DIR/disallowed_macros.rs:24:12
|
LL | let _: macros::ty!() = "";
| ^^^^^^^^^^^^^
error: use of a disallowed macro `macros::item`
--> $DIR/disallowed_macros.rs:22:5
--> $DIR/disallowed_macros.rs:25:5
|
LL | macros::item!();
| ^^^^^^^^^^^^^^^
error: use of a disallowed macro `macros::binop`
--> $DIR/disallowed_macros.rs:23:13
--> $DIR/disallowed_macros.rs:26:13
|
LL | let _ = macros::binop!(1);
| ^^^^^^^^^^^^^^^^^
error: use of a disallowed macro `macros::attr`
--> $DIR/disallowed_macros.rs:28:1
--> $DIR/disallowed_macros.rs:31:1
|
LL | / macros::attr! {
LL | | struct S;
@ -78,22 +78,28 @@ LL | | }
| |_^
error: use of a disallowed macro `macros::item`
--> $DIR/disallowed_macros.rs:33:5
--> $DIR/disallowed_macros.rs:36:5
|
LL | macros::item!();
| ^^^^^^^^^^^^^^^
error: use of a disallowed macro `macros::item`
--> $DIR/disallowed_macros.rs:37:5
--> $DIR/disallowed_macros.rs:40:5
|
LL | macros::item!();
| ^^^^^^^^^^^^^^^
error: use of a disallowed macro `macros::item`
--> $DIR/disallowed_macros.rs:41:5
--> $DIR/disallowed_macros.rs:44:5
|
LL | macros::item!();
| ^^^^^^^^^^^^^^^
error: aborting due to 15 previous errors
error: use of a disallowed macro `proc_macros::Derive`
--> $DIR/disallowed_macros.rs:47:10
|
LL | #[derive(Derive)]
| ^^^^^^
error: aborting due to 16 previous errors