Skip to content

Commit

Permalink
move [mixed_attributes_style] to LateLintPass to enable global `a…
Browse files Browse the repository at this point in the history
…llow`
  • Loading branch information
J-ZhengLi committed Mar 22, 2024
1 parent c6f794a commit 8521427
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 30 deletions.
92 changes: 65 additions & 27 deletions clippy_lints/src/attrs/mixed_attributes_style.rs
Original file line number Diff line number Diff line change
@@ -1,35 +1,73 @@
use super::MIXED_ATTRIBUTES_STYLE;
use clippy_utils::diagnostics::span_lint;
use rustc_ast::{AttrKind, AttrStyle};
use rustc_lint::EarlyContext;
use rustc_ast::{AttrKind, AttrStyle, Attribute};
use rustc_data_structures::fx::FxHashSet;
use rustc_lint::{LateContext, LintContext};
use rustc_span::Span;
use std::sync::Arc;

pub(super) fn check(cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
let mut has_outer_normal = false;
let mut has_inner_normal = false;
let mut has_outer_doc = false;
let mut has_inner_doc = false;
#[derive(Hash, PartialEq, Eq)]
enum SimpleAttrKind {
Doc,
Normal,
}

for attr in &item.attrs {
if attr.span.from_expansion() {
continue;
}
match (&attr.style, &attr.kind) {
(AttrStyle::Inner, AttrKind::Normal(_)) => has_inner_normal = true,
(AttrStyle::Inner, AttrKind::DocComment(..)) => has_inner_doc = true,
(AttrStyle::Outer, AttrKind::Normal(_)) => has_outer_normal = true,
(AttrStyle::Outer, AttrKind::DocComment(..)) => has_outer_doc = true,
impl From<&AttrKind> for SimpleAttrKind {
fn from(value: &AttrKind) -> Self {
match value {
AttrKind::Normal(_) => Self::Normal,
AttrKind::DocComment(..) => Self::Doc,
}
}
// Separating doc and normal attributes because mixing inner/outer docs
// with other outer/inner attributes doesn't really affecting readability.
if (has_inner_doc && has_outer_doc) || (has_outer_normal && has_inner_normal) {
let mut attrs_iter = item.attrs.iter().filter(|attr| !attr.span.from_expansion());
let span = attrs_iter.next().unwrap().span;
span_lint(
cx,
MIXED_ATTRIBUTES_STYLE,
span.with_hi(attrs_iter.last().unwrap().span.hi()),
"item has both inner and outer attributes",
);
}

pub(super) fn check(cx: &LateContext<'_>, item_span: Span, attrs: &[Attribute]) {
let mut inner_attr_kind: FxHashSet<SimpleAttrKind> = FxHashSet::default();
let mut outer_attr_kind: FxHashSet<SimpleAttrKind> = FxHashSet::default();

for attr in attrs {
if attr.span.from_expansion() || !attr_in_same_src_as_item(cx, attr.span, item_span) {
continue;
}

let kind: SimpleAttrKind = (&attr.kind).into();
match attr.style {
AttrStyle::Inner => {
if outer_attr_kind.contains(&kind) {
lint_mixed_attrs(cx, attrs);
return;
}
inner_attr_kind.insert(kind);
},
AttrStyle::Outer => {
if inner_attr_kind.contains(&kind) {
lint_mixed_attrs(cx, attrs);
return;
}
outer_attr_kind.insert(kind);
},
};
}
}

fn lint_mixed_attrs(cx: &LateContext<'_>, attrs: &[Attribute]) {
let mut attrs_iter = attrs.iter().filter(|attr| !attr.span.from_expansion());
let span = if let (Some(first), Some(last)) = (attrs_iter.next(), attrs_iter.last()) {
first.span.with_hi(last.span.hi())
} else {
return;
};
span_lint(
cx,
MIXED_ATTRIBUTES_STYLE,
span,
"item has both inner and outer attributes",
);
}

fn attr_in_same_src_as_item(cx: &LateContext<'_>, attr_span: Span, item_span: Span) -> bool {
let source_map = cx.sess().source_map();
let item_src = source_map.lookup_source_file(item_span.lo());
let attr_src = source_map.lookup_source_file(attr_span.lo());
Arc::ptr_eq(&attr_src, &item_src)
}
4 changes: 2 additions & 2 deletions clippy_lints/src/attrs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,7 @@ declare_lint_pass!(Attributes => [
USELESS_ATTRIBUTE,
BLANKET_CLIPPY_RESTRICTION_LINTS,
SHOULD_PANIC_WITHOUT_EXPECT,
MIXED_ATTRIBUTES_STYLE,
]);

impl<'tcx> LateLintPass<'tcx> for Attributes {
Expand Down Expand Up @@ -566,6 +567,7 @@ impl<'tcx> LateLintPass<'tcx> for Attributes {
ItemKind::ExternCrate(..) | ItemKind::Use(..) => useless_attribute::check(cx, item, attrs),
_ => {},
}
mixed_attributes_style::check(cx, item.span, attrs);
}

fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) {
Expand Down Expand Up @@ -594,7 +596,6 @@ impl_lint_pass!(EarlyAttributes => [
MAYBE_MISUSED_CFG,
DEPRECATED_CLIPPY_CFG_ATTR,
UNNECESSARY_CLIPPY_CFG,
MIXED_ATTRIBUTES_STYLE,
DUPLICATED_ATTRIBUTES,
]);

Expand All @@ -605,7 +606,6 @@ impl EarlyLintPass for EarlyAttributes {

fn check_item(&mut self, cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
empty_line_after::check(cx, item);
mixed_attributes_style::check(cx, item);
duplicated_attributes::check(cx, &item.attrs);
}

Expand Down
5 changes: 5 additions & 0 deletions tests/ui/mixed_attributes_style/auxiliary/submodule.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
//! Module level doc
#![allow(dead_code)]

#[allow(unused)]
//~^ ERROR: item has both inner and outer attributes
mod foo {
#![allow(dead_code)]
}
3 changes: 3 additions & 0 deletions tests/ui/mixed_attributes_style/mod_declaration.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#[path = "auxiliary/submodule.rs"] // don't lint.
/// This doc comment should not lint, it could be used to add context to the original module doc
mod submodule;
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
error: item has both inner and outer attributes
--> tests/ui/mixed_attributes_style/auxiliary/submodule.rs:1:1
--> tests/ui/mixed_attributes_style/auxiliary/submodule.rs:5:1
|
LL | / #[allow(unused)]
LL | |
LL | | mod foo {
LL | | #![allow(dead_code)]
| |________________________^
Expand Down

0 comments on commit 8521427

Please sign in to comment.