Skip to content

Commit 0a4cfbf

Browse files
committed
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
1 parent 9283497 commit 0a4cfbf

File tree

5 files changed

+238
-8
lines changed

5 files changed

+238
-8
lines changed

Diff for: CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -4619,6 +4619,7 @@ Released 2018-09-13
46194619
[`else_if_without_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#else_if_without_else
46204620
[`empty_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_drop
46214621
[`empty_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum
4622+
[`empty_line_after_doc_comments`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_doc_comments
46224623
[`empty_line_after_outer_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_outer_attr
46234624
[`empty_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_loop
46244625
[`empty_structs_with_brackets`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_structs_with_brackets

Diff for: clippy_lints/src/attrs.rs

+68-8
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,52 @@ declare_clippy_lint! {
176176
"empty line after outer attribute"
177177
}
178178

179+
declare_clippy_lint! {
180+
/// ### What it does
181+
/// Checks for empty lines after documenation comments.
182+
///
183+
/// ### Why is this bad?
184+
/// The documentation comment was most likely meant to be an inner attribute or regular comment.
185+
/// If it was intended to be a documentation comment, then the empty line should be removed to
186+
/// be more idiomatic.
187+
///
188+
/// ### Known problems
189+
/// Only detects empty lines immediately following the documentation. If the doc comment is followed
190+
/// by an attribute and then an empty line, this lint will not trigger. Use `empty_line_after_outer_attr`
191+
/// in combination with this lint to detect both cases.
192+
///
193+
/// Does not detect empty lines after doc attributes (e.g. `#[doc = ""]`).
194+
///
195+
/// ### Example
196+
/// ```rust
197+
/// /// Some doc comment with a blank line after it.
198+
///
199+
/// fn not_quite_good_code() { }
200+
/// ```
201+
///
202+
/// Use instead:
203+
/// ```rust
204+
/// /// Good (no blank line)
205+
/// fn this_is_fine() { }
206+
/// ```
207+
///
208+
/// ```rust
209+
/// // Good (convert to a regular comment)
210+
///
211+
/// fn this_is_fine_too() { }
212+
/// ```
213+
///
214+
/// ```rust
215+
/// //! Good (convert to a comment on an inner attribute)
216+
///
217+
/// fn this_is_fine_as_well() { }
218+
/// ```
219+
#[clippy::version = "1.70.0"]
220+
pub EMPTY_LINE_AFTER_DOC_COMMENTS,
221+
nursery,
222+
"empty line after documentation comments"
223+
}
224+
179225
declare_clippy_lint! {
180226
/// ### What it does
181227
/// Checks for `warn`/`deny`/`forbid` attributes targeting the whole clippy::restriction category.
@@ -604,6 +650,7 @@ impl_lint_pass!(EarlyAttributes => [
604650
DEPRECATED_CFG_ATTR,
605651
MISMATCHED_TARGET_OS,
606652
EMPTY_LINE_AFTER_OUTER_ATTR,
653+
EMPTY_LINE_AFTER_DOC_COMMENTS,
607654
]);
608655

609656
impl EarlyLintPass for EarlyAttributes {
@@ -619,10 +666,16 @@ impl EarlyLintPass for EarlyAttributes {
619666
extract_msrv_attr!(EarlyContext);
620667
}
621668

669+
/// Check for empty lines after outer attributes.
670+
///
671+
/// Attributes and documenation comments are both considered outer attributes
672+
/// by the AST. However, the average user likely considers them to be different.
673+
/// Checking for empty lines after each of these attributes is split into two different
674+
/// lints but can share the same logic.
622675
fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
623676
let mut iter = item.attrs.iter().peekable();
624677
while let Some(attr) = iter.next() {
625-
if matches!(attr.kind, AttrKind::Normal(..))
678+
if (matches!(attr.kind, AttrKind::Normal(..)) || matches!(attr.kind, AttrKind::DocComment(..)))
626679
&& attr.style == AttrStyle::Outer
627680
&& is_present_in_source(cx, attr.span)
628681
{
@@ -639,13 +692,20 @@ fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::It
639692
let lines = without_block_comments(lines);
640693

641694
if lines.iter().filter(|l| l.trim().is_empty()).count() > 2 {
642-
span_lint(
643-
cx,
644-
EMPTY_LINE_AFTER_OUTER_ATTR,
645-
begin_of_attr_to_item,
646-
"found an empty line after an outer attribute. \
647-
Perhaps you forgot to add a `!` to make it an inner attribute?",
648-
);
695+
let (lint_msg, lint_type) = match attr.kind {
696+
AttrKind::DocComment(..) => (
697+
"found an empty line after a doc comment. \
698+
Perhaps you need to use `//!` to make a comment on a module, remove the empty line, or make a regular comment with `//`?",
699+
EMPTY_LINE_AFTER_DOC_COMMENTS,
700+
),
701+
AttrKind::Normal(..) => (
702+
"found an empty line after an outer attribute. \
703+
Perhaps you forgot to add a `!` to make it an inner attribute?",
704+
EMPTY_LINE_AFTER_OUTER_ATTR,
705+
),
706+
};
707+
708+
span_lint(cx, lint_type, begin_of_attr_to_item, lint_msg);
649709
}
650710
}
651711
}

Diff for: clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
4848
crate::attrs::BLANKET_CLIPPY_RESTRICTION_LINTS_INFO,
4949
crate::attrs::DEPRECATED_CFG_ATTR_INFO,
5050
crate::attrs::DEPRECATED_SEMVER_INFO,
51+
crate::attrs::EMPTY_LINE_AFTER_DOC_COMMENTS_INFO,
5152
crate::attrs::EMPTY_LINE_AFTER_OUTER_ATTR_INFO,
5253
crate::attrs::INLINE_ALWAYS_INFO,
5354
crate::attrs::MISMATCHED_TARGET_OS_INFO,

Diff for: tests/ui/empty_line_after_doc_comments.rs

+132
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
//@aux-build:proc_macro_attr.rs
2+
#![warn(clippy::empty_line_after_doc_comments)]
3+
#![allow(clippy::assertions_on_constants)]
4+
#![feature(custom_inner_attributes)]
5+
#![rustfmt::skip]
6+
7+
#[macro_use]
8+
extern crate proc_macro_attr;
9+
10+
mod some_mod {
11+
//! This doc comment should *NOT* produce a warning
12+
13+
mod some_inner_mod {
14+
fn some_noop() {}
15+
}
16+
}
17+
18+
/// This should produce a warning
19+
20+
fn with_doc_and_newline() { assert!(true)}
21+
22+
// This should *NOT* produce a warning
23+
#[crate_type = "lib"]
24+
25+
/// some comment
26+
fn with_one_newline_and_comment() { assert!(true) }
27+
28+
// This should *NOT* produce a warning
29+
#[crate_type = "lib"]
30+
/// some comment
31+
fn with_no_newline_and_comment() { assert!(true) }
32+
33+
34+
// This should *NOT* produce a warning
35+
#[crate_type = "lib"]
36+
37+
fn with_one_newline() { assert!(true) }
38+
39+
// This should *NOT* produce a warning
40+
#[crate_type = "lib"]
41+
42+
43+
fn with_two_newlines() { assert!(true) }
44+
45+
46+
// This should *NOT* produce a warning
47+
#[crate_type = "lib"]
48+
49+
enum Baz {
50+
One,
51+
Two
52+
}
53+
54+
// This should *NOT* produce a warning
55+
#[crate_type = "lib"]
56+
57+
struct Foo {
58+
one: isize,
59+
two: isize
60+
}
61+
62+
// This should *NOT* produce a warning
63+
#[crate_type = "lib"]
64+
65+
mod foo {
66+
}
67+
68+
/// This doc comment should produce a warning
69+
70+
/** This is also a doc comment and should produce a warning
71+
*/
72+
73+
// This should *NOT* produce a warning
74+
#[allow(non_camel_case_types)]
75+
#[allow(missing_docs)]
76+
#[allow(missing_docs)]
77+
fn three_attributes() { assert!(true) }
78+
79+
// This should *NOT* produce a warning
80+
#[doc = "
81+
Returns the escaped value of the textual representation of
82+
83+
"]
84+
pub fn function() -> bool {
85+
true
86+
}
87+
88+
// This should *NOT* produce a warning
89+
#[derive(Clone, Copy)]
90+
pub enum FooFighter {
91+
Bar1,
92+
93+
Bar2,
94+
95+
Bar3,
96+
97+
Bar4
98+
}
99+
100+
// This should *NOT* produce a warning because the empty line is inside a block comment
101+
#[crate_type = "lib"]
102+
/*
103+
104+
*/
105+
pub struct S;
106+
107+
// This should *NOT* produce a warning
108+
#[crate_type = "lib"]
109+
/* test */
110+
pub struct T;
111+
112+
// This should *NOT* produce a warning
113+
// See https://github.com/rust-lang/rust-clippy/issues/5567
114+
#[fake_async_trait]
115+
pub trait Bazz {
116+
fn foo() -> Vec<u8> {
117+
let _i = "";
118+
119+
120+
121+
vec![]
122+
}
123+
}
124+
125+
#[derive(Clone, Copy)]
126+
#[dummy(string = "first line
127+
128+
second line
129+
")]
130+
pub struct Args;
131+
132+
fn main() {}

Diff for: tests/ui/empty_line_after_doc_comments.stderr

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
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 `//`?
2+
--> $DIR/empty_line_after_doc_comments.rs:18:1
3+
|
4+
LL | / /// This should produce a warning
5+
LL | |
6+
LL | | fn with_doc_and_newline() { assert!(true)}
7+
| |_
8+
|
9+
= note: `-D clippy::empty-line-after-doc-comments` implied by `-D warnings`
10+
11+
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 `//`?
12+
--> $DIR/empty_line_after_doc_comments.rs:68:1
13+
|
14+
LL | / /// This doc comment should produce a warning
15+
LL | |
16+
LL | | /** This is also a doc comment and should produce a warning
17+
LL | | */
18+
... |
19+
LL | | #[allow(missing_docs)]
20+
LL | | fn three_attributes() { assert!(true) }
21+
| |_
22+
23+
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 `//`?
24+
--> $DIR/empty_line_after_doc_comments.rs:70:1
25+
|
26+
LL | / /** This is also a doc comment and should produce a warning
27+
LL | | */
28+
LL | |
29+
LL | | // This should *NOT* produce a warning
30+
... |
31+
LL | | #[allow(missing_docs)]
32+
LL | | fn three_attributes() { assert!(true) }
33+
| |_
34+
35+
error: aborting due to 3 previous errors
36+

0 commit comments

Comments
 (0)