move [mixed_attributes_style] to LateLintPass;

fix issue #12436;

Author: J-ZhengLi

clippy_lints/src/attrs/mixed_attributes_style.rs

@@ -1,35 +1,62 @@
 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 has_mixed_doc_attrs = !attrs.inner_doc.is_empty() && !attrs.outer_doc.is_empty();
+    let mixed_doc_attrs_in_same_file = attrs.inner_doc.intersection(&attrs.outer_doc).next().is_some();
+    let has_mixed_normal_attrs = !attrs.inner_normal.is_empty() && !attrs.outer_normal.is_empty();
+    let mixed_normal_attrs_in_same_file = attrs.inner_normal.intersection(&attrs.outer_normal).next().is_some();
+
+    (has_mixed_doc_attrs && mixed_doc_attrs_in_same_file) || (has_mixed_normal_attrs && mixed_normal_attrs_in_same_file)
 }

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);
     }
 

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)]
 }

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;

tests/ui/mixed_attributes_style/global_allow.stderr 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)]
    | |________________________^