From 0086b6ab0a5a9a6a8e500f5742886097ed453a6d Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Thu, 1 Jun 2023 20:24:41 -0500 Subject: [PATCH] don't lint `allow_attributes` on attributes from proc macros --- clippy_lints/src/allow_attributes.rs | 3 +- clippy_lints/src/attrs.rs | 9 ++-- clippy_utils/src/check_proc_macro.rs | 44 ++++++++++++++++++- tests/ui/allow_attributes.fixed | 22 +++++++++- tests/ui/allow_attributes.rs | 20 ++++++++- tests/ui/allow_attributes.stderr | 10 +---- tests/ui/allow_attributes_false_positive.rs | 5 --- tests/ui/allow_attributes_without_reason.rs | 18 +++++++- .../ui/allow_attributes_without_reason.stderr | 28 +++++++++--- 9 files changed, 131 insertions(+), 28 deletions(-) delete mode 100644 tests/ui/allow_attributes_false_positive.rs diff --git a/clippy_lints/src/allow_attributes.rs b/clippy_lints/src/allow_attributes.rs index 554efdc58e1..69ccbeb7b06 100644 --- a/clippy_lints/src/allow_attributes.rs +++ b/clippy_lints/src/allow_attributes.rs @@ -1,5 +1,5 @@ use ast::AttrStyle; -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::{diagnostics::span_lint_and_sugg, is_from_proc_macro}; use rustc_ast as ast; use rustc_errors::Applicability; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -57,6 +57,7 @@ fn check_attribute(&mut self, cx: &LateContext<'_>, attr: &ast::Attribute) { if let AttrStyle::Outer = attr.style; if let Some(ident) = attr.ident(); if ident.name == rustc_span::symbol::sym::allow; + if !is_from_proc_macro(cx, &(attr, cx)); then { span_lint_and_sugg( cx, diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 897495ba108..e989cde0f3c 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -1,9 +1,12 @@ //! checks for attributes -use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::macros::{is_panic, macro_backtrace}; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::source::{first_line_of_span, is_present_in_source, snippet_opt, without_block_comments}; +use clippy_utils::{ + diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then}, + is_from_proc_macro, +}; use if_chain::if_chain; use rustc_ast::{AttrKind, AttrStyle, Attribute, LitKind, MetaItemKind, MetaItemLit, NestedMetaItem}; use rustc_errors::Applicability; @@ -540,7 +543,7 @@ fn check_clippy_lint_names(cx: &LateContext<'_>, name: Symbol, items: &[NestedMe } } -fn check_lint_reason(cx: &LateContext<'_>, name: Symbol, items: &[NestedMetaItem], attr: &'_ Attribute) { +fn check_lint_reason(cx: &LateContext<'_>, name: Symbol, items: &[NestedMetaItem], attr: &Attribute) { // Check for the feature if !cx.tcx.features().lint_reasons { return; @@ -555,7 +558,7 @@ fn check_lint_reason(cx: &LateContext<'_>, name: Symbol, items: &[NestedMetaItem } // Check if the attribute is in an external macro and therefore out of the developer's control - if in_external_macro(cx.sess(), attr.span) { + if in_external_macro(cx.sess(), attr.span) || is_from_proc_macro(cx, &(attr, cx)) { return; } diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index 9edaae85373..8a8ccc30205 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -12,7 +12,11 @@ //! code was written, and check if the span contains that text. Note this will only work correctly //! if the span is not from a `macro_rules` based macro. -use rustc_ast::ast::{IntTy, LitIntType, LitKind, StrStyle, UintTy}; +use rustc_ast::{ + ast::{AttrKind, Attribute, IntTy, LitIntType, LitKind, StrStyle, UintTy}, + token::CommentKind, + AttrStyle, +}; use rustc_hir::{ intravisit::FnKind, Block, BlockCheckMode, Body, Closure, Destination, Expr, ExprKind, FieldDef, FnHeader, HirId, Impl, ImplItem, ImplItemKind, IsAuto, Item, ItemKind, LoopSource, MatchSource, Node, QPath, TraitItem, @@ -271,6 +275,32 @@ fn fn_kind_pat(tcx: TyCtxt<'_>, kind: &FnKind<'_>, body: &Body<'_>, hir_id: HirI (start_pat, end_pat) } +fn attr_search_pat(attr: &Attribute) -> (Pat, Pat) { + match attr.kind { + AttrKind::Normal(..) => { + if matches!(attr.style, AttrStyle::Outer) { + (Pat::Str("#["), Pat::Str("]")) + } else { + (Pat::Str("#!["), Pat::Str("]")) + } + }, + AttrKind::DocComment(_kind @ CommentKind::Line, ..) => { + if matches!(attr.style, AttrStyle::Outer) { + (Pat::Str("///"), Pat::Str("")) + } else { + (Pat::Str("//!"), Pat::Str("")) + } + }, + AttrKind::DocComment(_kind @ CommentKind::Block, ..) => { + if matches!(attr.style, AttrStyle::Outer) { + (Pat::Str("/**"), Pat::Str("*/")) + } else { + (Pat::Str("/*!"), Pat::Str("*/")) + } + }, + } +} + pub trait WithSearchPat { type Context: LintContext; fn search_pat(&self, cx: &Self::Context) -> (Pat, Pat); @@ -310,6 +340,18 @@ fn span(&self) -> Span { } } +impl<'cx> WithSearchPat for (&Attribute, &LateContext<'cx>) { + type Context = LateContext<'cx>; + + fn search_pat(&self, _cx: &Self::Context) -> (Pat, Pat) { + attr_search_pat(&self.0) + } + + fn span(&self) -> Span { + self.0.span + } +} + /// Checks if the item likely came from a proc-macro. /// /// This should be called after `in_external_macro` and the initial pattern matching of the ast as diff --git a/tests/ui/allow_attributes.fixed b/tests/ui/allow_attributes.fixed index f0936b2608e..be93c5bffe7 100644 --- a/tests/ui/allow_attributes.fixed +++ b/tests/ui/allow_attributes.fixed @@ -1,9 +1,12 @@ //@run-rustfix +//@aux-build:proc_macros.rs #![allow(unused)] #![warn(clippy::allow_attributes)] #![feature(lint_reasons)] +#![no_main] -fn main() {} +extern crate proc_macros; +use proc_macros::{external, with_span}; // Using clippy::needless_borrow just as a placeholder, it isn't relevant. @@ -17,9 +20,24 @@ struct T3; #[warn(clippy::needless_borrow)] // Should not lint struct T4; // `panic = "unwind"` should always be true -#[cfg_attr(panic = "unwind", expect(dead_code))] +#[cfg_attr(panic = "unwind", allow(dead_code))] struct CfgT; +fn ignore_external() { + external! { + #[allow(clippy::needless_borrow)] + fn a() {} + } +} + +fn ignore_proc_macro() { + with_span! { + span + #[allow(clippy::needless_borrow)] // Should not lint + fn a() {} + } +} + fn ignore_inner_attr() { #![allow(unused)] // Should not lint } diff --git a/tests/ui/allow_attributes.rs b/tests/ui/allow_attributes.rs index 2fb9e86126e..10d2ed1a8a9 100644 --- a/tests/ui/allow_attributes.rs +++ b/tests/ui/allow_attributes.rs @@ -1,9 +1,12 @@ //@run-rustfix +//@aux-build:proc_macros.rs #![allow(unused)] #![warn(clippy::allow_attributes)] #![feature(lint_reasons)] +#![no_main] -fn main() {} +extern crate proc_macros; +use proc_macros::{external, with_span}; // Using clippy::needless_borrow just as a placeholder, it isn't relevant. @@ -20,6 +23,21 @@ fn main() {} #[cfg_attr(panic = "unwind", allow(dead_code))] struct CfgT; +fn ignore_external() { + external! { + #[allow(clippy::needless_borrow)] // Should not lint + fn a() {} + } +} + +fn ignore_proc_macro() { + with_span! { + span + #[allow(clippy::needless_borrow)] // Should not lint + fn a() {} + } +} + fn ignore_inner_attr() { #![allow(unused)] // Should not lint } diff --git a/tests/ui/allow_attributes.stderr b/tests/ui/allow_attributes.stderr index 681837e9ed7..37149104a47 100644 --- a/tests/ui/allow_attributes.stderr +++ b/tests/ui/allow_attributes.stderr @@ -1,16 +1,10 @@ error: #[allow] attribute found - --> $DIR/allow_attributes.rs:11:3 + --> $DIR/allow_attributes.rs:14:3 | LL | #[allow(dead_code)] | ^^^^^ help: replace it with: `expect` | = note: `-D clippy::allow-attributes` implied by `-D warnings` -error: #[allow] attribute found - --> $DIR/allow_attributes.rs:20:30 - | -LL | #[cfg_attr(panic = "unwind", allow(dead_code))] - | ^^^^^ help: replace it with: `expect` - -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/tests/ui/allow_attributes_false_positive.rs b/tests/ui/allow_attributes_false_positive.rs deleted file mode 100644 index 5c3407628be..00000000000 --- a/tests/ui/allow_attributes_false_positive.rs +++ /dev/null @@ -1,5 +0,0 @@ -#![warn(clippy::allow_attributes)] -#![feature(lint_reasons)] -#![crate_type = "proc-macro"] - -fn main() {} diff --git a/tests/ui/allow_attributes_without_reason.rs b/tests/ui/allow_attributes_without_reason.rs index 1a0d4e88657..95a9fa2aa47 100644 --- a/tests/ui/allow_attributes_without_reason.rs +++ b/tests/ui/allow_attributes_without_reason.rs @@ -1,9 +1,15 @@ +//@aux-build:proc_macros.rs #![feature(lint_reasons)] #![deny(clippy::allow_attributes_without_reason)] +#![allow(unfulfilled_lint_expectations)] + +extern crate proc_macros; +use proc_macros::{external, with_span}; // These should trigger the lint #[allow(dead_code)] #[allow(dead_code, deprecated)] +#[expect(dead_code)] // These should be fine #[allow(dead_code, reason = "This should be allowed")] #[warn(dyn_drop, reason = "Warnings can also have reasons")] @@ -11,4 +17,14 @@ #[deny(deref_nullptr)] #[forbid(deref_nullptr)] -fn main() {} +fn main() { + external! { + #[allow(dead_code)] + fn a() {} + } + with_span! { + span + #[allow(dead_code)] + fn b() {} + } +} diff --git a/tests/ui/allow_attributes_without_reason.stderr b/tests/ui/allow_attributes_without_reason.stderr index 23f17e9a7af..96f747d0026 100644 --- a/tests/ui/allow_attributes_without_reason.stderr +++ b/tests/ui/allow_attributes_without_reason.stderr @@ -1,23 +1,39 @@ error: `allow` attribute without specifying a reason - --> $DIR/allow_attributes_without_reason.rs:5:1 + --> $DIR/allow_attributes_without_reason.rs:4:1 | -LL | #[allow(dead_code)] - | ^^^^^^^^^^^^^^^^^^^ +LL | #![allow(unfulfilled_lint_expectations)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: try adding a reason at the end with `, reason = ".."` note: the lint level is defined here - --> $DIR/allow_attributes_without_reason.rs:2:9 + --> $DIR/allow_attributes_without_reason.rs:3:9 | LL | #![deny(clippy::allow_attributes_without_reason)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `allow` attribute without specifying a reason - --> $DIR/allow_attributes_without_reason.rs:6:1 + --> $DIR/allow_attributes_without_reason.rs:10:1 + | +LL | #[allow(dead_code)] + | ^^^^^^^^^^^^^^^^^^^ + | + = help: try adding a reason at the end with `, reason = ".."` + +error: `allow` attribute without specifying a reason + --> $DIR/allow_attributes_without_reason.rs:11:1 | LL | #[allow(dead_code, deprecated)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: try adding a reason at the end with `, reason = ".."` -error: aborting due to 2 previous errors +error: `expect` attribute without specifying a reason + --> $DIR/allow_attributes_without_reason.rs:12:1 + | +LL | #[expect(dead_code)] + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: try adding a reason at the end with `, reason = ".."` + +error: aborting due to 4 previous errors