Skip to content

fix: warn on empty line outer AttrKind::DocComment #10691

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
76 changes: 68 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,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 {
Expand All @@ -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)
{
Expand All @@ -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);
}
}
}
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