fix empty after lint on impl/trait items

Co-authored-by: Guillaume Gomez <guillaume1.gomez@gmail.com>
matthiaskrgr · Feb 7, 2025 · 9221e33 · 9221e33
1 parent cca8995
commit 9221e33
Show file tree

Hide file tree

Showing 7 changed files with 126 additions and 38 deletions.
diff --git a/compiler/rustc_lint/src/early.rs b/compiler/rustc_lint/src/early.rs
@@ -246,6 +246,14 @@ impl<'ast, 'ecx, 'tcx, T: EarlyLintPass> ast_visit::Visitor<'ast>
                 }
             }
             ast_visit::walk_assoc_item(cx, item, ctxt);
+            match ctxt {
+                ast_visit::AssocCtxt::Trait => {
+                    lint_callback!(cx, check_trait_item_post, item);
+                }
+                ast_visit::AssocCtxt::Impl => {
+                    lint_callback!(cx, check_impl_item_post, item);
+                }
+            }
         });
     }
 

diff --git a/compiler/rustc_lint/src/passes.rs b/compiler/rustc_lint/src/passes.rs
@@ -162,7 +162,9 @@ macro_rules! early_lint_methods {
                 c: rustc_span::Span,
                 d_: rustc_ast::NodeId);
             fn check_trait_item(a: &rustc_ast::AssocItem);
+            fn check_trait_item_post(a: &rustc_ast::AssocItem);
             fn check_impl_item(a: &rustc_ast::AssocItem);
+            fn check_impl_item_post(a: &rustc_ast::AssocItem);
             fn check_variant(a: &rustc_ast::Variant);
             fn check_attribute(a: &rustc_ast::Attribute);
             fn check_attributes(a: &[rustc_ast::Attribute]);

diff --git a/src/tools/clippy/clippy_lints/src/empty_line_after.rs b/src/tools/clippy/clippy_lints/src/empty_line_after.rs
@@ -3,13 +3,13 @@ use clippy_utils::source::{SpanRangeExt, snippet_indent};
 use clippy_utils::tokenize_with_text;
 use itertools::Itertools;
 use rustc_ast::token::CommentKind;
-use rustc_ast::{AttrKind, AttrStyle, Attribute, Crate, Item, ItemKind, ModKind, NodeId};
+use rustc_ast::{AssocItemKind, AttrKind, AttrStyle, Attribute, Crate, Item, ItemKind, ModKind, NodeId};
 use rustc_errors::{Applicability, Diag, SuggestionStyle};
 use rustc_lexer::TokenKind;
 use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
 use rustc_session::impl_lint_pass;
 use rustc_span::symbol::kw;
-use rustc_span::{BytePos, ExpnKind, InnerSpan, Span, SpanData, Symbol};
+use rustc_span::{BytePos, ExpnKind, Ident, InnerSpan, Span, SpanData, Symbol};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -92,7 +92,7 @@ struct ItemInfo {
     kind: &'static str,
     name: Symbol,
     span: Span,
-    mod_items: Vec<NodeId>,
+    mod_items: Option<NodeId>,
 }
 
 pub struct EmptyLineAfter {
@@ -347,7 +347,7 @@ impl EmptyLineAfter {
     fn suggest_inner(&self, diag: &mut Diag<'_, ()>, kind: StopKind, gaps: &[Gap<'_>], id: NodeId) {
         if let Some(parent) = self.items.iter().rev().nth(1)
             && (parent.kind == "module" || parent.kind == "crate")
-            && parent.mod_items.first() == Some(&id)
+            && parent.mod_items == Some(id)
         {
             let desc = if parent.kind == "module" {
                 "parent module"
@@ -367,48 +367,35 @@ impl EmptyLineAfter {
             );
         }
     }
-}
 
-impl EarlyLintPass for EmptyLineAfter {
-    fn check_crate(&mut self, _: &EarlyContext<'_>, krate: &Crate) {
+    fn check_item_kind(
+        &mut self,
+        cx: &EarlyContext<'_>,
+        kind: &ItemKind,
+        ident: &Ident,
+        span: Span,
+        attrs: &[Attribute],
+        id: NodeId,
+    ) {
         self.items.push(ItemInfo {
-            kind: "crate",
-            name: kw::Crate,
-            span: krate.spans.inner_span.with_hi(krate.spans.inner_span.lo()),
-            mod_items: krate
-                .items
-                .iter()
-                .filter(|i| !matches!(i.span.ctxt().outer_expn_data().kind, ExpnKind::AstPass(_)))
-                .map(|i| i.id)
-                .collect::<Vec<_>>(),
-        });
-    }
-
-    fn check_item_post(&mut self, _: &EarlyContext<'_>, _: &Item) {
-        self.items.pop();
-    }
-
-    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
-        self.items.push(ItemInfo {
-            kind: item.kind.descr(),
-            name: item.ident.name,
-            span: if item.span.contains(item.ident.span) {
-                item.span.with_hi(item.ident.span.hi())
+            kind: kind.descr(),
+            name: ident.name,
+            span: if span.contains(ident.span) {
+                span.with_hi(ident.span.hi())
             } else {
-                item.span.with_hi(item.span.lo())
+                span.with_hi(span.lo())
             },
-            mod_items: match item.kind {
-                ItemKind::Mod(_, ModKind::Loaded(ref items, _, _, _)) => items
+            mod_items: match kind {
+                ItemKind::Mod(_, ModKind::Loaded(items, _, _, _)) => items
                     .iter()
                     .filter(|i| !matches!(i.span.ctxt().outer_expn_data().kind, ExpnKind::AstPass(_)))
                     .map(|i| i.id)
-                    .collect::<Vec<_>>(),
-                _ => Vec::new(),
+                    .next(),
+                _ => None,
             },
         });
 
-        let mut outer = item
-            .attrs
+        let mut outer = attrs
             .iter()
             .filter(|attr| attr.style == AttrStyle::Outer && !attr.span.from_expansion())
             .map(|attr| Stop::from_attr(cx, attr))
@@ -448,6 +435,58 @@ impl EarlyLintPass for EmptyLineAfter {
             }
         }
 
-        self.check_gaps(cx, &gaps, item.id);
+        self.check_gaps(cx, &gaps, id);
+    }
+}
+
+impl EarlyLintPass for EmptyLineAfter {
+    fn check_crate(&mut self, _: &EarlyContext<'_>, krate: &Crate) {
+        self.items.push(ItemInfo {
+            kind: "crate",
+            name: kw::Crate,
+            span: krate.spans.inner_span.with_hi(krate.spans.inner_span.lo()),
+            mod_items: krate
+                .items
+                .iter()
+                .filter(|i| !matches!(i.span.ctxt().outer_expn_data().kind, ExpnKind::AstPass(_)))
+                .map(|i| i.id)
+                .next(),
+        });
+    }
+
+    fn check_item_post(&mut self, _: &EarlyContext<'_>, _: &Item) {
+        self.items.pop();
+    }
+    fn check_impl_item_post(&mut self, _: &EarlyContext<'_>, _: &Item<AssocItemKind>) {
+        self.items.pop();
+    }
+    fn check_trait_item_post(&mut self, _: &EarlyContext<'_>, _: &Item<AssocItemKind>) {
+        self.items.pop();
+    }
+
+    fn check_impl_item(&mut self, cx: &EarlyContext<'_>, item: &Item<AssocItemKind>) {
+        self.check_item_kind(
+            cx,
+            &item.kind.clone().into(),
+            &item.ident,
+            item.span,
+            &item.attrs,
+            item.id,
+        );
+    }
+
+    fn check_trait_item(&mut self, cx: &EarlyContext<'_>, item: &Item<AssocItemKind>) {
+        self.check_item_kind(
+            cx,
+            &item.kind.clone().into(),
+            &item.ident,
+            item.span,
+            &item.attrs,
+            item.id,
+        );
+    }
+
+    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
+        self.check_item_kind(cx, &item.kind, &item.ident, item.span, &item.attrs, item.id);
     }
 }
diff --git a/src/tools/clippy/tests/ui/empty_line_after/doc_comments.1.fixed b/src/tools/clippy/tests/ui/empty_line_after/doc_comments.1.fixed
@@ -132,4 +132,13 @@ pub struct BlockComment;
 ))]
 fn empty_line_in_cfg_attr() {}
 
+trait Foo {
+    fn bar();
+}
+
+impl Foo for LineComment {
+    /// comment on assoc item
+    fn bar() {}
+}
+
 fn main() {}
diff --git a/src/tools/clippy/tests/ui/empty_line_after/doc_comments.2.fixed b/src/tools/clippy/tests/ui/empty_line_after/doc_comments.2.fixed
@@ -141,4 +141,13 @@ pub struct BlockComment;
 ))]
 fn empty_line_in_cfg_attr() {}
 
+trait Foo {
+    fn bar();
+}
+
+impl Foo for LineComment {
+    /// comment on assoc item
+    fn bar() {}
+}
+
 fn main() {}
diff --git a/src/tools/clippy/tests/ui/empty_line_after/doc_comments.rs b/src/tools/clippy/tests/ui/empty_line_after/doc_comments.rs
@@ -144,4 +144,14 @@ pub struct BlockComment;
 ))]
 fn empty_line_in_cfg_attr() {}
 
+trait Foo {
+    fn bar();
+}
+
+impl Foo for LineComment {
+    /// comment on assoc item
+
+    fn bar() {}
+}
+
 fn main() {}
diff --git a/src/tools/clippy/tests/ui/empty_line_after/doc_comments.stderr b/src/tools/clippy/tests/ui/empty_line_after/doc_comments.stderr
@@ -171,5 +171,16 @@ help: if the doc comment should not document `new_code2` comment it out
 LL |     // /// Docs for `old_code2`
    |     ++
 
-error: aborting due to 10 previous errors
+error: empty line after doc comment
+  --> tests/ui/empty_line_after/doc_comments.rs:152:5
+   |
+LL | /     /// comment on assoc item
+LL | |
+   | |_^
+LL |       fn bar() {}
+   |       ------ the comment documents this function
+   |
+   = help: if the empty line is unintentional remove it
+
+error: aborting due to 11 previous errors