WIP: empty doc span is still broken

Author: lucarlig

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/mod.rs

@@ -23,7 +23,7 @@ use rustc_resolve::rustdoc::{
 };
 use rustc_session::impl_lint_pass;
 use rustc_span::edition::Edition;
-use rustc_span::{sym, Span};
+use rustc_span::{sym, Span, DUMMY_SP};
 use std::ops::Range;
 use url::Url;
 
@@ -338,6 +338,29 @@ 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
+    /// ```rust
+    /// ///
+    /// fn returns_true() -> bool {
+    ///     true
+    /// }
+    /// Use instead:
+    /// ```rust
+    /// 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 +387,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 {
@@ -378,6 +402,20 @@ impl<'tcx> LateLintPass<'tcx> for Documentation {
         let Some(headers) = check_attrs(cx, &self.valid_idents, attrs) else {
             return;
         };
+
+        if let Some(span) = get_empty_doc_combined_span(attrs, item.span)
+            && headers.empty
+        {
+            span_lint_and_help(
+                cx,
+                EMPTY_DOCS,
+                span,
+                "empty doc comment",
+                None,
+                "consider removing or filling it",
+            );
+        }
+
         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)) {
@@ -477,6 +515,7 @@ struct DocHeaders {
     safety: bool,
     errors: bool,
     panics: bool,
+    empty: bool,
 }
 
 /// Does some pre-processing on raw, desugared `#[doc]` attributes such as parsing them and
@@ -509,7 +548,10 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
     doc.pop();
 
     if doc.is_empty() {
-        return Some(DocHeaders::default());
+        return Some(DocHeaders {
+            empty: true,
+            ..DocHeaders::default()
+        });
     }
 
     let mut cb = fake_broken_link_callback;
@@ -717,3 +759,20 @@ impl<'a, 'tcx> Visitor<'tcx> for FindPanicUnwrap<'a, 'tcx> {
         self.cx.tcx.hir()
     }
 }
+
+fn get_empty_doc_combined_span(attrs: &[Attribute], item_span: Span) -> Option<Span> {
+    let mut attrs_span = DUMMY_SP;
+    if attrs.len() > 0 {
+        attrs_span = attrs
+            .iter()
+            .map(|attr| attr.span)
+            .fold(attrs[0].span, |acc, next| acc.to(next));
+    }
+
+    match (!item_span.is_dummy(), !attrs_span.is_dummy()) {
+        (true, true) => Some(item_span.shrink_to_lo().to(attrs_span)),
+        (true, false) => Some(item_span),
+        (false, true) => Some(attrs_span),
+        (false, false) => None,
+    }
+}

tests/ui/empty_docs.rs

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

tests/ui/empty_docs.stderr

@@ -0,0 +1,35 @@
+error: empty doc comment
+  --> tests/ui/empty_docs.rs:10:1
+   |
+LL | / ///
+LL | | enum Warn {
+   | |_
+   |
+   = 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:18:1
+   |
+LL | / enum WarnForB {
+LL | |     /// it's ok
+LL | |     A,
+LL | |     ///
+LL | |     B,
+LL | | }
+   | |_^
+   |
+   = help: consider removing or filling it
+
+error: empty doc comment
+  --> tests/ui/empty_docs.rs:32:1
+   |
+LL | / fn warn_about_this_as_well() {
+LL | |     //!
+   | |_______^
+   |
+   = help: consider removing or filling it
+
+error: aborting due to 3 previous errors
+