Auto merge of #10691 - jdswensen:jds/fix-doc-empty-line, r=Jarcho

fix: warn on empty line outer AttrKind::DocComment

changelog: [`empty_line_after_doc_comments`]: add lint for checking empty lines after rustdoc comments.

Fixes: #10395

Author: bors

CHANGELOG.md

@@ -4620,6 +4620,7 @@ Released 2018-09-13
 [`else_if_without_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#else_if_without_else
 [`empty_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_drop
 [`empty_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum
+[`empty_line_after_doc_comments`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_doc_comments
 [`empty_line_after_outer_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_outer_attr
 [`empty_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_loop
 [`empty_structs_with_brackets`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_structs_with_brackets

clippy_lints/src/attrs.rs

@@ -176,6 +176,52 @@ declare_clippy_lint! {
     "empty line after outer attribute"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for empty lines after documenation comments.
+    ///
+    /// ### Why is this bad?
+    /// The documentation comment was most likely meant to be an inner attribute or regular comment.
+    /// If it was intended to be a documentation comment, then the empty line should be removed to
+    /// be more idiomatic.
+    ///
+    /// ### Known problems
+    /// Only detects empty lines immediately following the documentation. If the doc comment is followed
+    /// by an attribute and then an empty line, this lint will not trigger. Use `empty_line_after_outer_attr`
+    /// in combination with this lint to detect both cases.
+    ///
+    /// Does not detect empty lines after doc attributes (e.g. `#[doc = ""]`).
+    ///
+    /// ### Example
+    /// ```rust
+    /// /// Some doc comment with a blank line after it.
+    ///
+    /// fn not_quite_good_code() { }
+    /// ```
+    ///
+    /// Use instead:
+    /// ```rust
+    /// /// Good (no blank line)
+    /// fn this_is_fine() { }
+    /// ```
+    ///
+    /// ```rust
+    /// // Good (convert to a regular comment)
+    ///
+    /// fn this_is_fine_too() { }
+    /// ```
+    ///
+    /// ```rust
+    /// //! Good (convert to a comment on an inner attribute)
+    ///
+    /// fn this_is_fine_as_well() { }
+    /// ```
+    #[clippy::version = "1.70.0"]
+    pub EMPTY_LINE_AFTER_DOC_COMMENTS,
+    nursery,
+    "empty line after documentation comments"
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for `warn`/`deny`/`forbid` attributes targeting the whole clippy::restriction category.
@@ -604,6 +650,7 @@ impl_lint_pass!(EarlyAttributes => [
     DEPRECATED_CFG_ATTR,
     MISMATCHED_TARGET_OS,
     EMPTY_LINE_AFTER_OUTER_ATTR,
+    EMPTY_LINE_AFTER_DOC_COMMENTS,
 ]);
 
 impl EarlyLintPass for EarlyAttributes {
@@ -619,10 +666,16 @@ impl EarlyLintPass for EarlyAttributes {
     extract_msrv_attr!(EarlyContext);
 }
 
+/// Check for empty lines after outer attributes.
+///
+/// Attributes and documenation comments are both considered outer attributes
+/// by the AST. However, the average user likely considers them to be different.
+/// Checking for empty lines after each of these attributes is split into two different
+/// lints but can share the same logic.
 fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
     let mut iter = item.attrs.iter().peekable();
     while let Some(attr) = iter.next() {
-        if matches!(attr.kind, AttrKind::Normal(..))
+        if (matches!(attr.kind, AttrKind::Normal(..)) || matches!(attr.kind, AttrKind::DocComment(..)))
             && attr.style == AttrStyle::Outer
             && is_present_in_source(cx, attr.span)
         {
@@ -639,13 +692,20 @@ fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::It
                 let lines = without_block_comments(lines);
 
                 if lines.iter().filter(|l| l.trim().is_empty()).count() > 2 {
-                    span_lint(
-                        cx,
-                        EMPTY_LINE_AFTER_OUTER_ATTR,
-                        begin_of_attr_to_item,
-                        "found an empty line after an outer attribute. \
-                        Perhaps you forgot to add a `!` to make it an inner attribute?",
-                    );
+                    let (lint_msg, lint_type) = match attr.kind {
+                        AttrKind::DocComment(..) => (
+                            "found an empty line after a doc comment. \
+                            Perhaps you need to use `//!` to make a comment on a module, remove the empty line, or make a regular comment with `//`?",
+                            EMPTY_LINE_AFTER_DOC_COMMENTS,
+                        ),
+                        AttrKind::Normal(..) => (
+                            "found an empty line after an outer attribute. \
+                            Perhaps you forgot to add a `!` to make it an inner attribute?",
+                            EMPTY_LINE_AFTER_OUTER_ATTR,
+                        ),
+                    };
+
+                    span_lint(cx, lint_type, begin_of_attr_to_item, lint_msg);
                 }
             }
         }

clippy_lints/src/declared_lints.rs

@@ -48,6 +48,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::attrs::BLANKET_CLIPPY_RESTRICTION_LINTS_INFO,
     crate::attrs::DEPRECATED_CFG_ATTR_INFO,
     crate::attrs::DEPRECATED_SEMVER_INFO,
+    crate::attrs::EMPTY_LINE_AFTER_DOC_COMMENTS_INFO,
     crate::attrs::EMPTY_LINE_AFTER_OUTER_ATTR_INFO,
     crate::attrs::INLINE_ALWAYS_INFO,
     crate::attrs::MISMATCHED_TARGET_OS_INFO,

tests/ui/empty_line_after_doc_comments.rs

@@ -0,0 +1,132 @@
+//@aux-build:proc_macro_attr.rs
+#![warn(clippy::empty_line_after_doc_comments)]
+#![allow(clippy::assertions_on_constants)]
+#![feature(custom_inner_attributes)]
+#![rustfmt::skip]
+
+#[macro_use]
+extern crate proc_macro_attr;
+
+mod some_mod {
+    //! This doc comment should *NOT* produce a warning
+
+    mod some_inner_mod {
+        fn some_noop() {}
+    }
+}
+
+/// This should produce a warning
+
+fn with_doc_and_newline() { assert!(true)}
+
+// This should *NOT* produce a warning
+#[crate_type = "lib"]
+
+/// some comment
+fn with_one_newline_and_comment() { assert!(true) }
+
+// This should *NOT* produce a warning
+#[crate_type = "lib"]
+/// some comment
+fn with_no_newline_and_comment() { assert!(true) }
+
+
+// This should *NOT* produce a warning
+#[crate_type = "lib"]
+
+fn with_one_newline() { assert!(true) }
+
+// This should *NOT* produce a warning
+#[crate_type = "lib"]
+
+
+fn with_two_newlines() { assert!(true) }
+
+
+// This should *NOT* produce a warning
+#[crate_type = "lib"]
+
+enum Baz {
+    One,
+    Two
+}
+
+// This should *NOT* produce a warning
+#[crate_type = "lib"]
+
+struct Foo {
+    one: isize,
+    two: isize
+}
+
+// This should *NOT* produce a warning
+#[crate_type = "lib"]
+
+mod foo {
+}
+
+/// This doc comment should produce a warning
+
+/** This is also a doc comment and should produce a warning
+ */
+
+// This should *NOT* produce a warning
+#[allow(non_camel_case_types)]
+#[allow(missing_docs)]
+#[allow(missing_docs)]
+fn three_attributes() { assert!(true) }
+
+// This should *NOT* produce a warning
+#[doc = "
+Returns the escaped value of the textual representation of
+
+"]
+pub fn function() -> bool {
+    true
+}
+
+// This should *NOT* produce a warning
+#[derive(Clone, Copy)]
+pub enum FooFighter {
+    Bar1,
+
+    Bar2,
+
+    Bar3,
+
+    Bar4
+}
+
+// This should *NOT* produce a warning because the empty line is inside a block comment
+#[crate_type = "lib"]
+/*
+
+*/
+pub struct S;
+
+// This should *NOT* produce a warning
+#[crate_type = "lib"]
+/* test */
+pub struct T;
+
+// This should *NOT* produce a warning
+// See https://github.com/rust-lang/rust-clippy/issues/5567
+#[fake_async_trait]
+pub trait Bazz {
+    fn foo() -> Vec<u8> {
+        let _i = "";
+
+
+
+        vec![]
+    }
+}
+
+#[derive(Clone, Copy)]
+#[dummy(string = "first line
+
+second line
+")]
+pub struct Args;
+
+fn main() {}

tests/ui/empty_line_after_doc_comments.stderr

@@ -0,0 +1,36 @@
+error: found an empty line after a doc comment. Perhaps you need to use `//!` to make a comment on a module, remove the empty line, or make a regular comment with `//`?
+  --> $DIR/empty_line_after_doc_comments.rs:18:1
+   |
+LL | / /// This should produce a warning
+LL | |
+LL | | fn with_doc_and_newline() { assert!(true)}
+   | |_
+   |
+   = note: `-D clippy::empty-line-after-doc-comments` implied by `-D warnings`
+
+error: found an empty line after a doc comment. Perhaps you need to use `//!` to make a comment on a module, remove the empty line, or make a regular comment with `//`?
+  --> $DIR/empty_line_after_doc_comments.rs:68:1
+   |
+LL | / /// This doc comment should produce a warning
+LL | |
+LL | | /** This is also a doc comment and should produce a warning
+LL | |  */
+...  |
+LL | | #[allow(missing_docs)]
+LL | | fn three_attributes() { assert!(true) }
+   | |_
+
+error: found an empty line after a doc comment. Perhaps you need to use `//!` to make a comment on a module, remove the empty line, or make a regular comment with `//`?
+  --> $DIR/empty_line_after_doc_comments.rs:70:1
+   |
+LL | / /** This is also a doc comment and should produce a warning
+LL | |  */
+LL | |
+LL | | // This should *NOT* produce a warning
+...  |
+LL | | #[allow(missing_docs)]
+LL | | fn three_attributes() { assert!(true) }
+   | |_
+
+error: aborting due to 3 previous errors
+