Skip to content

Commit

Permalink
move [mixed_attributes_style] to LateLintPass;
Browse files Browse the repository at this point in the history
fix issue #12436;
  • Loading branch information
J-ZhengLi committed Mar 18, 2024
1 parent c6f794a commit b434eda
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 27 deletions.
73 changes: 49 additions & 24 deletions clippy_lints/src/attrs/mixed_attributes_style.rs
Original file line number Diff line number Diff line change
@@ -1,35 +1,60 @@
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::FileName;

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(Default)]
struct AttrGroup {
inner_doc: FxHashSet<FileName>,
outer_doc: FxHashSet<FileName>,
inner_normal: FxHashSet<FileName>,
outer_normal: FxHashSet<FileName>,
}

pub(super) fn check(cx: &LateContext<'_>, attrs: &[Attribute]) {
let mut attr_group = AttrGroup::default();

for attr in &item.attrs {
for attr in attrs {
if attr.span.from_expansion() {
continue;
}
// Keep tracking of the filename of the attribute to prevent linting across files,
// such as on outlined mod declarations.
let filename = cx.sess().source_map().span_to_filename(attr.span);
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,
}
(AttrStyle::Inner, AttrKind::DocComment(..)) => attr_group.inner_doc.insert(filename),
(AttrStyle::Outer, AttrKind::DocComment(..)) => attr_group.outer_doc.insert(filename),
(AttrStyle::Inner, AttrKind::Normal(..)) => attr_group.inner_normal.insert(filename),
(AttrStyle::Outer, AttrKind::Normal(..)) => attr_group.outer_normal.insert(filename),
};
}
// 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",
);

if !should_lint(&attr_group) {
return;
}
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",
);
}

/// Lint only when the attributes:
///
/// - Have the same kind.
/// - In the same file.
fn should_lint(attrs: &AttrGroup) -> bool {
let mixed_doc_attrs_in_same_file = attrs.inner_doc.intersection(&attrs.outer_doc).next().is_some();
let mixed_normal_attrs_in_same_file = attrs.inner_normal.intersection(&attrs.outer_normal).next().is_some();

mixed_doc_attrs_in_same_file || mixed_normal_attrs_in_same_file
}
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, 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 b434eda

Please sign in to comment.