Skip to content

Commit

Permalink
stop [mixed_attributes_style] from linting on different attributes
Browse files Browse the repository at this point in the history
  • Loading branch information
J-ZhengLi committed Mar 22, 2024
1 parent 8521427 commit f69739b
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 13 deletions.
16 changes: 13 additions & 3 deletions clippy_lints/src/attrs/mixed_attributes_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Symbol>),
}

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::<Vec<_>>();
Self::Normal(path_symbols)
},
AttrKind::DocComment(..) => Self::Doc,
}
}
Expand Down
14 changes: 12 additions & 2 deletions clippy_lints/src/attrs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
54 changes: 50 additions & 4 deletions tests/ui/mixed_attributes_style.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -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
Expand All @@ -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]
}
33 changes: 29 additions & 4 deletions tests/ui/mixed_attributes_style.stderr
Original file line number Diff line number Diff line change
@@ -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() {
Expand All @@ -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 | |
Expand All @@ -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

0 comments on commit f69739b

Please sign in to comment.