Skip to content

Commit

Permalink
Auto merge of #12267 - Jarcho:issue_12254, r=Alexendoo
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@bors
bors committed Feb 12, 2024
2 parents 75f57cf + ac7c6e5 commit b0e7640
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 46 deletions.
84 changes: 54 additions & 30 deletions clippy_lints/src/disallowed_macros.rs
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
}
Expand All @@ -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);
}
}
}
}
Expand All @@ -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);
}
}
29 changes: 29 additions & 0 deletions tests/ui-toml/disallowed_macros/auxiliary/proc_macros.rs
View file
Original file line number Diff line number Diff line change
@@ -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())),
])
}
1 change: 1 addition & 0 deletions tests/ui-toml/disallowed_macros/clippy.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ disallowed-macros = [
"macros::item",
"macros::binop",
"macros::attr",
"proc_macros::Derive",
]
6 changes: 6 additions & 0 deletions tests/ui-toml/disallowed_macros/disallowed_macros.rs
View file
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down Expand Up @@ -40,3 +43,6 @@ trait Y {
impl Y for S {
macros::item!();
}

#[derive(Derive)]
struct Foo;
38 changes: 22 additions & 16 deletions tests/ui-toml/disallowed_macros/disallowed_macros.stderr
View file
Original file line number Diff line number Diff line change
@@ -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");
| ^^^^^^^^^^^^^^^
Expand All @@ -8,92 +8,98 @@ 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)]
| ^^^^^^^^^
|
= 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;
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

0 comments on commit b0e7640

Please sign in to comment.