Empty docs

CHANGELOG.md

@@ -5158,6 +5158,7 @@ Released 2018-09-13
 [`duration_subsec`]: https://rust-lang.github.io/rust-clippy/master/index.html#duration_subsec
 [`eager_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#eager_transmute
 [`else_if_without_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#else_if_without_else
+[`empty_docs`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_docs
 [`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_enum_variants_with_brackets`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum_variants_with_brackets

clippy_lints/src/declared_lints.rs

@@ -139,6 +139,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::disallowed_types::DISALLOWED_TYPES_INFO,
     crate::doc::DOC_LINK_WITH_QUOTES_INFO,
     crate::doc::DOC_MARKDOWN_INFO,
+    crate::doc::EMPTY_DOCS_INFO,
     crate::doc::MISSING_ERRORS_DOC_INFO,
     crate::doc::MISSING_PANICS_DOC_INFO,
     crate::doc::MISSING_SAFETY_DOC_INFO,

clippy_lints/src/doc/empty_docs.rs

@@ -0,0 +1,25 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use rustc_ast::Attribute;
+use rustc_lint::LateContext;
+
+use super::EMPTY_DOCS;
+
+// TODO: Adjust the parameters as necessary
+pub(super) fn check(cx: &LateContext<'_>, attrs: &[Attribute]) {
+    let doc_attrs: Vec<_> = attrs.iter().filter(|attr| attr.doc_str().is_some()).collect();
+
+    let span;
+    if let Some(first) = doc_attrs.first()
+        && let Some(last) = doc_attrs.last()
+    {
+        span = first.span.with_hi(last.span.hi());
+        span_lint_and_help(
+            cx,
+            EMPTY_DOCS,
+            span,
+            "empty doc comment",
+            None,
+            "consider removing or filling it",
+        );
+    }
+}

clippy_lints/src/doc/mod.rs

@@ -27,6 +27,7 @@ use rustc_span::{sym, Span};
 use std::ops::Range;
 use url::Url;
 
+mod empty_docs;
 mod link_with_quotes;
 mod markdown;
 mod missing_headers;
@@ -338,6 +339,30 @@ declare_clippy_lint! {
     "suspicious usage of (outer) doc comments"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Detects documentation that is empty.
+    /// ### Why is this bad?
+    /// It is unlikely that there is any reason to have empty documentation for an item
+    /// ### Example
+    /// ```rs
+    /// ///
+    /// fn returns_true() -> bool {
+    ///     true
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rs
+    /// fn returns_true() -> bool {
+    ///     true
+    /// }
+    /// ```
+    #[clippy::version = "1.78.0"]
+    pub EMPTY_DOCS,
+    suspicious,
+    "docstrings exist but documentation is empty"
+}
+
 #[derive(Clone)]
 pub struct Documentation {
     valid_idents: FxHashSet<String>,
@@ -364,7 +389,8 @@ impl_lint_pass!(Documentation => [
     NEEDLESS_DOCTEST_MAIN,
     TEST_ATTR_IN_DOCTEST,
     UNNECESSARY_SAFETY_DOC,
-    SUSPICIOUS_DOC_COMMENTS
+    SUSPICIOUS_DOC_COMMENTS,
+    EMPTY_DOCS,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Documentation {
@@ -375,9 +401,18 @@ impl<'tcx> LateLintPass<'tcx> for Documentation {
 
     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
         let attrs = cx.tcx.hir().attrs(item.hir_id());
-        let Some(headers) = check_attrs(cx, &self.valid_idents, attrs) else {
+        let Some(DocInfo {
+            empty,
+            doc_headers: headers,
+        }) = check_attrs(cx, &self.valid_idents, attrs)
+        else {
             return;
         };
+
+        if empty && !item.span.is_dummy() {
+            empty_docs::check(cx, attrs);
+        }
+
         match item.kind {
             hir::ItemKind::Fn(ref sig, _, body_id) => {
                 if !(is_entrypoint_fn(cx, item.owner_id.to_def_id()) || in_external_macro(cx.tcx.sess, item.span)) {
@@ -425,7 +460,11 @@ impl<'tcx> LateLintPass<'tcx> for Documentation {
 
     fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) {
         let attrs = cx.tcx.hir().attrs(item.hir_id());
-        let Some(headers) = check_attrs(cx, &self.valid_idents, attrs) else {
+        let Some(DocInfo {
+            empty: _,
+            doc_headers: headers,
+        }) = check_attrs(cx, &self.valid_idents, attrs)
+        else {
             return;
         };
         if let hir::TraitItemKind::Fn(ref sig, ..) = item.kind {
@@ -437,7 +476,11 @@ impl<'tcx> LateLintPass<'tcx> for Documentation {
 
     fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'_>) {
         let attrs = cx.tcx.hir().attrs(item.hir_id());
-        let Some(headers) = check_attrs(cx, &self.valid_idents, attrs) else {
+        let Some(DocInfo {
+            empty: _,
+            doc_headers: headers,
+        }) = check_attrs(cx, &self.valid_idents, attrs)
+        else {
             return;
         };
         if self.in_trait_impl || in_external_macro(cx.tcx.sess, item.span) {
@@ -479,14 +522,20 @@ struct DocHeaders {
     panics: bool,
 }
 
+#[derive(Copy, Clone, Default)]
+struct DocInfo {
+    empty: bool,
+    doc_headers: DocHeaders,
+}
+
 /// Does some pre-processing on raw, desugared `#[doc]` attributes such as parsing them and
 /// then delegates to `check_doc`.
 /// Some lints are already checked here if they can work with attributes directly and don't need
 /// to work with markdown.
 /// Others are checked elsewhere, e.g. in `check_doc` if they need access to markdown, or
 /// back in the various late lint pass methods if they need the final doc headers, like "Safety" or
 /// "Panics" sections.
-fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[Attribute]) -> Option<DocHeaders> {
+fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[Attribute]) -> Option<DocInfo> {
     /// We don't want the parser to choke on intra doc links. Since we don't
     /// actually care about rendering them, just pretend that all broken links
     /// point to a fake address.
@@ -502,14 +551,18 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
     suspicious_doc_comments::check(cx, attrs);
 
     let (fragments, _) = attrs_to_doc_fragments(attrs.iter().map(|attr| (attr, None)), true);
-    let mut doc = String::new();
-    for fragment in &fragments {
-        add_doc_fragment(&mut doc, fragment);
-    }
+    let mut doc = fragments.iter().fold(String::new(), |mut acc, fragment| {
+        add_doc_fragment(&mut acc, fragment);
+        acc
+    });
     doc.pop();
+    let doc = doc.trim().to_string();
 
     if doc.is_empty() {
-        return Some(DocHeaders::default());
+        return Some(DocInfo {
+            empty: true,
+            doc_headers: DocHeaders::default(),
+        });
     }
 
     let mut cb = fake_broken_link_callback;
@@ -518,15 +571,18 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
     let opts = main_body_opts() - Options::ENABLE_SMART_PUNCTUATION;
     let parser = pulldown_cmark::Parser::new_with_broken_link_callback(&doc, opts, Some(&mut cb));
 
-    Some(check_doc(
-        cx,
-        valid_idents,
-        parser.into_offset_iter(),
-        Fragments {
-            fragments: &fragments,
-            doc: &doc,
-        },
-    ))
+    Some(DocInfo {
+        empty: false,
+        doc_headers: check_doc(
+            cx,
+            valid_idents,
+            parser.into_offset_iter(),
+            Fragments {
+                fragments: &fragments,
+                doc: &doc,
+            },
+        ),
+    })
 }
 
 const RUST_CODE: &[&str] = &["rust", "no_run", "should_panic", "compile_fail"];

tests/ui/empty_docs.rs

@@ -0,0 +1,61 @@
+#![allow(unused)]
+#![warn(clippy::empty_docs)]
+
+/// this is a struct
+struct Bananas {
+    /// count
+    count: usize,
+}
+
+///
+enum Warn {
+    ///
+    A,
+    ///
+    B,
+}
+
+enum WarnA {
+    ///
+    A,
+    B,
+}
+
+enum DontWarn {
+    /// it's ok
+    A,
+    ///
+    B,
+}
+
+#[doc = ""]
+fn warn_about_this() {}
+
+#[doc = ""]
+#[doc = ""]
+fn this_doesn_warn() {}
+
+#[doc = "a fine function"]
+fn this_is_fine() {}
+
+fn warn_about_this_as_well() {
+    //!
+}
+
+///
+fn warn_inner_outer() {
+    //!w
+}
+
+fn this_is_ok() {
+    //!
+    //! inside the function
+}
+
+fn warn() {
+    /*! */
+}
+
+fn dont_warn() {
+    /*! dont warn me */
+}

tests/ui/empty_docs.stderr

@@ -0,0 +1,45 @@
+error: empty doc comment
+  --> tests/ui/empty_docs.rs:10:1
+   |
+LL | ///
+   | ^^^
+   |
+   = help: consider removing or filling it
+   = note: `-D clippy::empty-docs` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::empty_docs)]`
+
+error: empty doc comment
+  --> tests/ui/empty_docs.rs:31:1
+   |
+LL | #[doc = ""]
+   | ^^^^^^^^^^^
+   |
+   = help: consider removing or filling it
+
+error: empty doc comment
+  --> tests/ui/empty_docs.rs:34:1
+   |
+LL | / #[doc = ""]
+LL | | #[doc = ""]
+   | |___________^
+   |
+   = help: consider removing or filling it
+
+error: empty doc comment
+  --> tests/ui/empty_docs.rs:42:5
+   |
+LL |     //!
+   |     ^^^
+   |
+   = help: consider removing or filling it
+
+error: empty doc comment
+  --> tests/ui/empty_docs.rs:56:5
+   |
+LL |     /*! */
+   |     ^^^^^^
+   |
+   = help: consider removing or filling it
+
+error: aborting due to 5 previous errors
+

tests/ui/semicolon_if_nothing_returned.fixed

@@ -1,7 +1,12 @@
 //@aux-build:proc_macro_attr.rs
 
 #![warn(clippy::semicolon_if_nothing_returned)]
-#![allow(clippy::redundant_closure, clippy::uninlined_format_args, clippy::needless_late_init)]
+#![allow(
+    clippy::redundant_closure,
+    clippy::uninlined_format_args,
+    clippy::needless_late_init,
+    clippy::empty_docs
+)]
 
 #[macro_use]
 extern crate proc_macro_attr;

tests/ui/semicolon_if_nothing_returned.rs

@@ -1,7 +1,12 @@
 //@aux-build:proc_macro_attr.rs
 
 #![warn(clippy::semicolon_if_nothing_returned)]
-#![allow(clippy::redundant_closure, clippy::uninlined_format_args, clippy::needless_late_init)]
+#![allow(
+    clippy::redundant_closure,
+    clippy::uninlined_format_args,
+    clippy::needless_late_init,
+    clippy::empty_docs
+)]
 
 #[macro_use]
 extern crate proc_macro_attr;

tests/ui/semicolon_if_nothing_returned.stderr

@@ -1,5 +1,5 @@
 error: consider adding a `;` to the last statement for consistent formatting
-  --> tests/ui/semicolon_if_nothing_returned.rs:13:5
+  --> tests/ui/semicolon_if_nothing_returned.rs:18:5
    |
 LL |     println!("Hello")
    |     ^^^^^^^^^^^^^^^^^ help: add a `;` here: `println!("Hello");`
@@ -8,25 +8,25 @@ LL |     println!("Hello")
    = help: to override `-D warnings` add `#[allow(clippy::semicolon_if_nothing_returned)]`
 
 error: consider adding a `;` to the last statement for consistent formatting
-  --> tests/ui/semicolon_if_nothing_returned.rs:17:5
+  --> tests/ui/semicolon_if_nothing_returned.rs:22:5
    |
 LL |     get_unit()
    |     ^^^^^^^^^^ help: add a `;` here: `get_unit();`
 
 error: consider adding a `;` to the last statement for consistent formatting
-  --> tests/ui/semicolon_if_nothing_returned.rs:22:5
+  --> tests/ui/semicolon_if_nothing_returned.rs:27:5
    |
 LL |     y = x + 1
    |     ^^^^^^^^^ help: add a `;` here: `y = x + 1;`
 
 error: consider adding a `;` to the last statement for consistent formatting
-  --> tests/ui/semicolon_if_nothing_returned.rs:28:9
+  --> tests/ui/semicolon_if_nothing_returned.rs:33:9
    |
 LL |         hello()
    |         ^^^^^^^ help: add a `;` here: `hello();`
 
 error: consider adding a `;` to the last statement for consistent formatting
-  --> tests/ui/semicolon_if_nothing_returned.rs:39:9
+  --> tests/ui/semicolon_if_nothing_returned.rs:44:9
    |
 LL |         ptr::drop_in_place(s.as_mut_ptr())
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add a `;` here: `ptr::drop_in_place(s.as_mut_ptr());`