Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion clippy_lints/src/allow_attributes.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -57,6 +57,7 @@ impl LateLintPass<'_> for AllowAttribute {
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,
Expand Down
9 changes: 6 additions & 3 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down
64 changes: 62 additions & 2 deletions clippy_utils/src/check_proc_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -25,12 +29,16 @@ use rustc_span::{Span, Symbol};
use rustc_target::spec::abi::Abi;

/// The search pattern to look for. Used by `span_matches_pat`
#[derive(Clone, Copy)]
#[derive(Clone)]
pub enum Pat {
/// A single string.
Str(&'static str),
/// A single string.
OwnedStr(String),
/// Any of the given strings.
MultiStr(&'static [&'static str]),
/// Any of the given strings.
OwnedMultiStr(Vec<String>),
/// The string representation of the symbol.
Sym(Symbol),
/// Any decimal or hexadecimal digit depending on the location.
Expand All @@ -51,12 +59,16 @@ fn span_matches_pat(sess: &Session, span: Span, start_pat: Pat, end_pat: Pat) ->
let end_str = s.trim_end_matches(|c: char| c.is_whitespace() || c == ')' || c == ',');
(match start_pat {
Pat::Str(text) => start_str.starts_with(text),
Pat::OwnedStr(text) => start_str.starts_with(&text),
Pat::MultiStr(texts) => texts.iter().any(|s| start_str.starts_with(s)),
Pat::OwnedMultiStr(texts) => texts.iter().any(|s| start_str.starts_with(s)),
Pat::Sym(sym) => start_str.starts_with(sym.as_str()),
Pat::Num => start_str.as_bytes().first().map_or(false, u8::is_ascii_digit),
} && match end_pat {
Pat::Str(text) => end_str.ends_with(text),
Pat::OwnedStr(text) => end_str.starts_with(&text),
Pat::MultiStr(texts) => texts.iter().any(|s| start_str.ends_with(s)),
Pat::OwnedMultiStr(texts) => texts.iter().any(|s| start_str.starts_with(s)),
Pat::Sym(sym) => end_str.ends_with(sym.as_str()),
Pat::Num => end_str.as_bytes().last().map_or(false, u8::is_ascii_hexdigit),
})
Expand Down Expand Up @@ -271,6 +283,42 @@ 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(..) => {
let mut pat = if matches!(attr.style, AttrStyle::Outer) {
(Pat::Str("#["), Pat::Str("]"))
} else {
(Pat::Str("#!["), Pat::Str("]"))
};

if let Some(ident) = attr.ident() && let Pat::Str(old_pat) = pat.0 {
// TODO: I feel like it's likely we can use `Cow` instead but this will require quite a bit of
// refactoring
// NOTE: This will likely have false positives, like `allow = 1`
pat.0 = Pat::OwnedMultiStr(vec![ident.to_string(), old_pat.to_owned()]);
pat.1 = Pat::Str("");
}

pat
},
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);
Expand Down Expand Up @@ -310,6 +358,18 @@ impl<'cx> WithSearchPat for (&FnKind<'cx>, &Body<'cx>, HirId, 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
Expand Down
20 changes: 19 additions & 1 deletion tests/ui/allow_attributes.fixed
Original file line number Diff line number Diff line change
@@ -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.

Expand All @@ -20,6 +23,21 @@ struct T4;
#[cfg_attr(panic = "unwind", expect(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
}
20 changes: 19 additions & 1 deletion tests/ui/allow_attributes.rs
Original file line number Diff line number Diff line change
@@ -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.

Expand All @@ -20,6 +23,21 @@ struct T4;
#[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
}
4 changes: 2 additions & 2 deletions tests/ui/allow_attributes.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
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
--> $DIR/allow_attributes.rs:23:30
|
LL | #[cfg_attr(panic = "unwind", allow(dead_code))]
| ^^^^^ help: replace it with: `expect`
Expand Down
5 changes: 0 additions & 5 deletions tests/ui/allow_attributes_false_positive.rs

This file was deleted.

18 changes: 17 additions & 1 deletion tests/ui/allow_attributes_without_reason.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,30 @@
//@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")]
#[warn(deref_nullptr)]
#[deny(deref_nullptr)]
#[forbid(deref_nullptr)]

fn main() {}
fn main() {
external! {
#[allow(dead_code)]
fn a() {}
}
with_span! {
span
#[allow(dead_code)]
fn b() {}
}
}
28 changes: 22 additions & 6 deletions tests/ui/allow_attributes_without_reason.stderr
Original file line number Diff line number Diff line change
@@ -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