From f69739b7631f8a98a84cd5e7c4af6d882a6626f9 Mon Sep 17 00:00:00 2001 From: J-ZhengLi Date: Fri, 22 Mar 2024 21:37:12 +0800 Subject: [PATCH] stop [`mixed_attributes_style`] from linting on different attributes --- .../src/attrs/mixed_attributes_style.rs | 16 ++++-- clippy_lints/src/attrs/mod.rs | 14 ++++- tests/ui/mixed_attributes_style.rs | 54 +++++++++++++++++-- tests/ui/mixed_attributes_style.stderr | 33 ++++++++++-- 4 files changed, 104 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/attrs/mixed_attributes_style.rs b/clippy_lints/src/attrs/mixed_attributes_style.rs index 3666b13ca058..22bc7b65cb8c 100644 --- a/clippy_lints/src/attrs/mixed_attributes_style.rs +++ b/clippy_lints/src/attrs/mixed_attributes_style.rs @@ -3,19 +3,29 @@ use clippy_utils::diagnostics::span_lint; use rustc_ast::{AttrKind, AttrStyle, Attribute}; use rustc_data_structures::fx::FxHashSet; use rustc_lint::{LateContext, LintContext}; -use rustc_span::Span; +use rustc_span::{Span, Symbol}; use std::sync::Arc; #[derive(Hash, PartialEq, Eq)] enum SimpleAttrKind { Doc, - Normal, + /// A normal attribute, with its name symbols. + Normal(Vec), } impl From<&AttrKind> for SimpleAttrKind { fn from(value: &AttrKind) -> Self { match value { - AttrKind::Normal(_) => Self::Normal, + AttrKind::Normal(attr) => { + let path_symbols = attr + .item + .path + .segments + .iter() + .map(|seg| seg.ident.name) + .collect::>(); + Self::Normal(path_symbols) + }, AttrKind::DocComment(..) => Self::Doc, } } diff --git a/clippy_lints/src/attrs/mod.rs b/clippy_lints/src/attrs/mod.rs index b8d8a7b6a711..6a31fcad05d6 100644 --- a/clippy_lints/src/attrs/mod.rs +++ b/clippy_lints/src/attrs/mod.rs @@ -465,10 +465,20 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks that an item has only one kind of attributes. + /// Checks for item that has same kind of attributes with mixed styles (inner/outer). /// /// ### Why is this bad? - /// Having both kinds of attributes makes it more complicated to read code. + /// Having both style of said attributes makes it more complicated to read code. + /// + /// ### Known problems + /// This lint currently have false-nagative when mixing same attributes + /// but they have different path symbols, for example: + /// ```ignore + /// #[custom_attribute] + /// pub fn foo() { + /// #![my_crate::custom_attribute] + /// } + /// ``` /// /// ### Example /// ```no_run diff --git a/tests/ui/mixed_attributes_style.rs b/tests/ui/mixed_attributes_style.rs index 9f39949110cf..cd532a253afc 100644 --- a/tests/ui/mixed_attributes_style.rs +++ b/tests/ui/mixed_attributes_style.rs @@ -1,6 +1,10 @@ +//@aux-build:proc_macro_attr.rs +#![feature(custom_inner_attributes)] #![warn(clippy::mixed_attributes_style)] #![allow(clippy::duplicated_attributes)] +use proc_macro_attr::dummy; + #[allow(unused)] //~ ERROR: item has both inner and outer attributes fn foo1() { #![allow(unused)] @@ -40,10 +44,10 @@ fn main() { } // issue #12435 -#[cfg(test)] -mod tests { - //! Module doc, don't lint -} +// #[cfg(test)] +// mod tests { +// //! Module doc, don't lint +// } #[allow(unused)] mod baz { //! Module doc, don't lint @@ -53,3 +57,45 @@ mod baz { mod quz { #![allow(unused)] } + +// issue #12530, don't lint different attributes entirely +#[cfg(test)] +mod tests { + #![allow(clippy::unreadable_literal)] +} +#[cfg(unix)] +mod another_mod { + #![allow(clippy::question_mark)] +} +/// Nested mod - Good +mod nested_mod { + #[allow(dead_code)] //~ ERROR: item has both inner and outer attributes + mod inner_mod { + #![allow(dead_code)] + } +} +/// Nested mod - Good //~ ERROR: item has both inner and outer attributes +#[allow(unused)] +mod nest_mod_2 { + #![allow(unused)] + + #[allow(dead_code)] //~ ERROR: item has both inner and outer attributes + mod inner_mod { + #![allow(dead_code)] + } +} +/// Nested mod - Possible FN +#[cfg(test)] +mod nested_mod_false_nagative { + #![allow(unused)] + + #[allow(dead_code)] // This should lint but it does not, removing the `#[cfg(test)]` solves the problem. + mod inner_mod { + #![allow(dead_code)] + } +} +// Different path symbols - Known FN +#[dummy] +fn use_dummy() { + #![proc_macro_attr::dummy] +} diff --git a/tests/ui/mixed_attributes_style.stderr b/tests/ui/mixed_attributes_style.stderr index ed798073cb7c..bc5517088abb 100644 --- a/tests/ui/mixed_attributes_style.stderr +++ b/tests/ui/mixed_attributes_style.stderr @@ -1,5 +1,5 @@ error: item has both inner and outer attributes - --> tests/ui/mixed_attributes_style.rs:4:1 + --> tests/ui/mixed_attributes_style.rs:8:1 | LL | / #[allow(unused)] LL | | fn foo1() { @@ -10,7 +10,7 @@ LL | | #![allow(unused)] = help: to override `-D warnings` add `#[allow(clippy::mixed_attributes_style)]` error: item has both inner and outer attributes - --> tests/ui/mixed_attributes_style.rs:18:1 + --> tests/ui/mixed_attributes_style.rs:22:1 | LL | / /// linux LL | | @@ -19,12 +19,37 @@ LL | | //! windows | |_______________^ error: item has both inner and outer attributes - --> tests/ui/mixed_attributes_style.rs:33:1 + --> tests/ui/mixed_attributes_style.rs:37:1 | LL | / #[allow(unused)] LL | | mod bar { LL | | #![allow(unused)] | |_____________________^ -error: aborting due to 3 previous errors +error: item has both inner and outer attributes + --> tests/ui/mixed_attributes_style.rs:72:5 + | +LL | / #[allow(dead_code)] +LL | | mod inner_mod { +LL | | #![allow(dead_code)] + | |____________________________^ + +error: item has both inner and outer attributes + --> tests/ui/mixed_attributes_style.rs:77:1 + | +LL | / /// Nested mod - Good +LL | | #[allow(unused)] +LL | | mod nest_mod_2 { +LL | | #![allow(unused)] + | |_____________________^ + +error: item has both inner and outer attributes + --> tests/ui/mixed_attributes_style.rs:82:5 + | +LL | / #[allow(dead_code)] +LL | | mod inner_mod { +LL | | #![allow(dead_code)] + | |____________________________^ + +error: aborting due to 6 previous errors