From 1c71163af98fa4bf2b9a68f8fbe7bb42cac53a80 Mon Sep 17 00:00:00 2001 From: J-ZhengLi Date: Fri, 22 Mar 2024 23:35:50 +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 | 46 +++++++++++++++++++ tests/ui/mixed_attributes_style.stderr | 33 +++++++++++-- 4 files changed, 100 insertions(+), 9 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..0b1d10dce16f 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)] @@ -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