diff --git a/CHANGELOG.md b/CHANGELOG.md index 10dd28a8f6af..83aaeee8c186 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 751c262673b1..2c1335db5a77 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -176,6 +176,53 @@ 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() { } + /// + /// // or + /// + /// // Good (convert to a regular comment) + /// + /// fn this_is_fine_too() { } + /// ``` + /// + /// // or + /// + /// //! Good (convert to a comment on the inner attribute) + /// + /// fn this_is_fine_too() { } + /// ``` + #[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,11 +651,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) { @@ -619,10 +668,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) { @@ -639,19 +694,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); diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index f24dab627809..260be34a795d 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/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, diff --git a/tests/ui/empty_line_after_doc_comments.rs b/tests/ui/empty_line_after_doc_comments.rs new file mode 100644 index 000000000000..e843770f5785 --- /dev/null +++ b/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 { + let _i = ""; + + + + vec![] + } +} + +#[derive(Clone, Copy)] +#[dummy(string = "first line + +second line +")] +pub struct Args; + +fn main() {} diff --git a/tests/ui/empty_line_after_doc_comments.stderr b/tests/ui/empty_line_after_doc_comments.stderr new file mode 100644 index 000000000000..2ca1b51679ed --- /dev/null +++ b/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 +