From b434eda32f9e088e2735d469c97198031813519b Mon Sep 17 00:00:00 2001 From: J-ZhengLi Date: Mon, 18 Mar 2024 15:59:19 +0800 Subject: [PATCH] move [`mixed_attributes_style`] to `LateLintPass`; fix issue #12436; --- .../src/attrs/mixed_attributes_style.rs | 73 +++++++++++++------ clippy_lints/src/attrs/mod.rs | 4 +- .../auxiliary/submodule.rs | 5 ++ .../mixed_attributes_style/mod_declaration.rs | 3 + ...al_allow.stderr => mod_declaration.stderr} | 3 +- 5 files changed, 61 insertions(+), 27 deletions(-) create mode 100644 tests/ui/mixed_attributes_style/mod_declaration.rs rename tests/ui/mixed_attributes_style/{global_allow.stderr => mod_declaration.stderr} (83%) diff --git a/clippy_lints/src/attrs/mixed_attributes_style.rs b/clippy_lints/src/attrs/mixed_attributes_style.rs index f826e9519c77..b0c9f039ee5f 100644 --- a/clippy_lints/src/attrs/mixed_attributes_style.rs +++ b/clippy_lints/src/attrs/mixed_attributes_style.rs @@ -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, + outer_doc: FxHashSet, + inner_normal: FxHashSet, + outer_normal: FxHashSet, +} + +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 } diff --git a/clippy_lints/src/attrs/mod.rs b/clippy_lints/src/attrs/mod.rs index 675c428948f6..1536ff6233b3 100644 --- a/clippy_lints/src/attrs/mod.rs +++ b/clippy_lints/src/attrs/mod.rs @@ -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 { @@ -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<'_>) { @@ -594,7 +596,6 @@ impl_lint_pass!(EarlyAttributes => [ MAYBE_MISUSED_CFG, DEPRECATED_CLIPPY_CFG_ATTR, UNNECESSARY_CLIPPY_CFG, - MIXED_ATTRIBUTES_STYLE, DUPLICATED_ATTRIBUTES, ]); @@ -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); } diff --git a/tests/ui/mixed_attributes_style/auxiliary/submodule.rs b/tests/ui/mixed_attributes_style/auxiliary/submodule.rs index acfc561835f8..df44b07a6941 100644 --- a/tests/ui/mixed_attributes_style/auxiliary/submodule.rs +++ b/tests/ui/mixed_attributes_style/auxiliary/submodule.rs @@ -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)] } diff --git a/tests/ui/mixed_attributes_style/mod_declaration.rs b/tests/ui/mixed_attributes_style/mod_declaration.rs new file mode 100644 index 000000000000..b0f1f0bda9e6 --- /dev/null +++ b/tests/ui/mixed_attributes_style/mod_declaration.rs @@ -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; diff --git a/tests/ui/mixed_attributes_style/global_allow.stderr b/tests/ui/mixed_attributes_style/mod_declaration.stderr similarity index 83% rename from tests/ui/mixed_attributes_style/global_allow.stderr rename to tests/ui/mixed_attributes_style/mod_declaration.stderr index 926d24397b5e..968c537c7e44 100644 --- a/tests/ui/mixed_attributes_style/global_allow.stderr +++ b/tests/ui/mixed_attributes_style/mod_declaration.stderr @@ -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)] | |________________________^