Auto merge of #12342 - lucarlig:empty-docs, r=llogiq

Empty docs Fixes rust-lang/rust-clippy#9931 changelog: [`empty_doc`]: Detects documentation that is empty. changelog: Doc comment lints now trigger for struct field and enum variant documentation
rust-lang · Feb 26, 2024 · 1c50948 · 1c50948
2 parents d718995 + fca77c0
commit 1c50948
Show file tree

Hide file tree

Showing 12 changed files with 223 additions and 20 deletions.
diff --git a/CHANGELOG.md b/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

diff --git a/clippy_lints/src/declared_lints.rs b/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,

diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs
@@ -19,7 +19,8 @@ use rustc_middle::hir::nested_filter;
 use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty;
 use rustc_resolve::rustdoc::{
-    add_doc_fragment, attrs_to_doc_fragments, main_body_opts, source_span_for_markdown_range, DocFragment,
+    add_doc_fragment, attrs_to_doc_fragments, main_body_opts, source_span_for_markdown_range, span_of_fragments,
+    DocFragment,
 };
 use rustc_session::impl_lint_pass;
 use rustc_span::edition::Edition;
@@ -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?
+    /// Empty docs clutter code without adding value, reducing readability and maintainability.
+    /// ### Example
+    /// ```no_run
+    /// ///
+    /// fn returns_true() -> bool {
+    ///     true
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// 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 {
@@ -373,11 +399,22 @@ impl<'tcx> LateLintPass<'tcx> for Documentation {
         check_attrs(cx, &self.valid_idents, attrs);
     }
 
+    fn check_variant(&mut self, cx: &LateContext<'tcx>, variant: &'tcx hir::Variant<'tcx>) {
+        let attrs = cx.tcx.hir().attrs(variant.hir_id);
+        check_attrs(cx, &self.valid_idents, attrs);
+    }
+
+    fn check_field_def(&mut self, cx: &LateContext<'tcx>, variant: &'tcx hir::FieldDef<'tcx>) {
+        let attrs = cx.tcx.hir().attrs(variant.hir_id);
+        check_attrs(cx, &self.valid_idents, attrs);
+    }
+
     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 {
             return;
         };
+
         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)) {
@@ -502,13 +539,23 @@ 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();
 
-    if doc.is_empty() {
+    if doc.trim().is_empty() {
+        if let Some(span) = span_of_fragments(&fragments) {
+            span_lint_and_help(
+                cx,
+                EMPTY_DOCS,
+                span,
+                "empty doc comment",
+                None,
+                "consider removing or filling it",
+            );
+        }
         return Some(DocHeaders::default());
     }
 

diff --git a/clippy_lints/src/manual_is_ascii_check.rs b/clippy_lints/src/manual_is_ascii_check.rs
@@ -74,7 +74,7 @@ enum CharRange {
     LowerChar,
     /// 'A'..='Z' | b'A'..=b'Z'
     UpperChar,
-    /// AsciiLower | AsciiUpper
+    /// `AsciiLower` | `AsciiUpper`
     FullChar,
     /// '0..=9'
     Digit,

diff --git a/clippy_lints/src/methods/filter_map.rs b/clippy_lints/src/methods/filter_map.rs
@@ -75,7 +75,7 @@ enum OffendingFilterExpr<'tcx> {
     },
     /// `.filter(|enum| matches!(enum, Enum::A(_)))`
     Matches {
-        /// The DefId of the variant being matched
+        /// The `DefId` of the variant being matched
         variant_def_id: hir::def_id::DefId,
     },
 }

diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs
@@ -74,12 +74,12 @@ impl QuestionMark {
 enum IfBlockType<'hir> {
     /// An `if x.is_xxx() { a } else { b } ` expression.
     ///
-    /// Contains: caller (x), caller_type, call_sym (is_xxx), if_then (a), if_else (b)
+    /// Contains: `caller (x), caller_type, call_sym (is_xxx), if_then (a), if_else (b)`
     IfIs(&'hir Expr<'hir>, Ty<'hir>, Symbol, &'hir Expr<'hir>),
     /// An `if let Xxx(a) = b { c } else { d }` expression.
     ///
-    /// Contains: let_pat_qpath (Xxx), let_pat_type, let_pat_sym (a), let_expr (b), if_then (c),
-    /// if_else (d)
+    /// Contains: `let_pat_qpath (Xxx), let_pat_type, let_pat_sym (a), let_expr (b), if_then (c),
+    /// if_else (d)`
     IfLet(
         Res,
         Ty<'hir>,

diff --git a/clippy_lints/src/slow_vector_initialization.rs b/clippy_lints/src/slow_vector_initialization.rs
@@ -62,7 +62,7 @@ declare_lint_pass!(SlowVectorInit => [SLOW_VECTOR_INITIALIZATION]);
 /// assigned to a variable. For example, `let mut vec = Vec::with_capacity(0)` or
 /// `vec = Vec::with_capacity(0)`
 struct VecAllocation<'tcx> {
-    /// HirId of the variable
+    /// `HirId` of the variable
     local_id: HirId,
 
     /// Reference to the expression which allocates the vector

diff --git a/tests/ui/empty_docs.rs b/tests/ui/empty_docs.rs
@@ -0,0 +1,67 @@
+#![allow(unused)]
+#![warn(clippy::empty_docs)]
+mod outer {
+    //!
+
+    /// this is a struct
+    struct Bananas {
+        /// count
+        count: usize,
+    }
+
+    ///
+    enum Warn {
+        ///
+        A,
+        B,
+    }
+
+    enum DontWarn {
+        /// i
+        A,
+        B,
+    }
+
+    #[doc = ""]
+    fn warn_about_this() {}
+
+    #[doc = ""]
+    #[doc = ""]
+    fn this_doesn_warn() {}
+
+    #[doc = "a fine function"]
+    fn this_is_fine() {}
+
+    ///
+    mod inner {
+        ///
+        fn dont_warn_inner_outer() {
+            //!w
+        }
+
+        fn this_is_ok() {
+            //!
+            //! inside the function
+        }
+
+        fn warn() {
+            /*! */
+        }
+
+        fn dont_warn() {
+            /*! dont warn me */
+        }
+
+        trait NoDoc {
+            ///
+            fn some() {}
+        }
+    }
+
+    union Unite {
+        /// lint y
+        x: i32,
+        ///
+        y: i32,
+    }
+}
diff --git a/tests/ui/empty_docs.stderr b/tests/ui/empty_docs.stderr
@@ -0,0 +1,77 @@
+error: empty doc comment
+  --> tests/ui/empty_docs.rs:4:5
+   |
+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:12:5
+   |
+LL |     ///
+   |     ^^^
+   |
+   = help: consider removing or filling it
+
+error: empty doc comment
+  --> tests/ui/empty_docs.rs:14:9
+   |
+LL |         ///
+   |         ^^^
+   |
+   = help: consider removing or filling it
+
+error: empty doc comment
+  --> tests/ui/empty_docs.rs:25:5
+   |
+LL |     #[doc = ""]
+   |     ^^^^^^^^^^^
+   |
+   = help: consider removing or filling it
+
+error: empty doc comment
+  --> tests/ui/empty_docs.rs:28:5
+   |
+LL | /     #[doc = ""]
+LL | |     #[doc = ""]
+   | |_______________^
+   |
+   = help: consider removing or filling it
+
+error: empty doc comment
+  --> tests/ui/empty_docs.rs:35:5
+   |
+LL |     ///
+   |     ^^^
+   |
+   = help: consider removing or filling it
+
+error: empty doc comment
+  --> tests/ui/empty_docs.rs:48:13
+   |
+LL |             /*! */
+   |             ^^^^^^
+   |
+   = help: consider removing or filling it
+
+error: empty doc comment
+  --> tests/ui/empty_docs.rs:56:13
+   |
+LL |             ///
+   |             ^^^
+   |
+   = help: consider removing or filling it
+
+error: empty doc comment
+  --> tests/ui/empty_docs.rs:64:9
+   |
+LL |         ///
+   |         ^^^
+   |
+   = help: consider removing or filling it
+
+error: aborting due to 9 previous errors
+
diff --git a/tests/ui/semicolon_if_nothing_returned.fixed b/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;

diff --git a/tests/ui/semicolon_if_nothing_returned.rs b/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;

diff --git a/tests/ui/semicolon_if_nothing_returned.stderr b/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());`