Skip to content

Commit

Permalink
fix: warn on empty line outer AttrKind::DocComment
Browse files Browse the repository at this point in the history
changelog: [`empty_line_after_doc_comments`]: add lint for checking
empty lines after rustdoc comments.

Fixes: rust-lang#10395
  • Loading branch information
jdswensen committed Apr 21, 2023
1 parent 9283497 commit fcde813
Show file tree
Hide file tree
Showing 5 changed files with 250 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4619,6 +4619,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
Expand Down
88 changes: 80 additions & 8 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -604,11 +650,13 @@ impl_lint_pass!(EarlyAttributes => [
DEPRECATED_CFG_ATTR,
MISMATCHED_TARGET_OS,
EMPTY_LINE_AFTER_OUTER_ATTR,
EMPTY_LINE_AFTER_DOC_COMMENTS,
]);

impl EarlyLintPass for EarlyAttributes {
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
check_empty_line_after_outer_attr(cx, item);
check_empty_line_after_doc_comments(cx, item);
}

fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
Expand All @@ -619,10 +667,16 @@ impl EarlyLintPass for EarlyAttributes {
extract_msrv_attr!(EarlyContext);
}

fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
/// Selectively checks 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_kind(cx: &EarlyContext<'_>, item: &rustc_ast::Item, lint_doc_comment: bool) {
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)
{
Expand All @@ -639,19 +693,37 @@ 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. \
let (lint_msg, lint_type) = if lint_doc_comment && matches!(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,
)
} else if !lint_doc_comment && matches!(attr.kind, 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,
)
} else {
continue;
};

span_lint(cx, lint_type, begin_of_attr_to_item, lint_msg);
}
}
}
}
}

fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
check_empty_line_after_outer_attr_kind(cx, item, false);
}

fn check_empty_line_after_doc_comments(cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
check_empty_line_after_outer_attr_kind(cx, item, true);
}

fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msrv) {
if_chain! {
if msrv.meets(msrvs::TOOL_ATTRIBUTES);
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
132 changes: 132 additions & 0 deletions tests/ui/empty_line_after_doc_comments.rs
Original file line number Diff line number Diff line change
@@ -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() {}
36 changes: 36 additions & 0 deletions tests/ui/empty_line_after_doc_comments.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit fcde813

Please sign in to comment.