Add new too_long_first_doc_paragraph first paragraph lint

GuillaumeGomez · GuillaumeGomez · commit 855a9d1377a2 · 2024-07-26T11:21:32.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5913,6 +5913,7 @@ Released 2018-09-13
 [`to_string_in_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_format_args
 [`to_string_trait_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_trait_impl
 [`todo`]: https://rust-lang.github.io/rust-clippy/master/index.html#todo
+[`too_long_first_doc_paragraph`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_long_first_doc_paragraph
 [`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
 [`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines
 [`toplevel_ref_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#toplevel_ref_arg
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
@@ -148,6 +148,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::doc::NEEDLESS_DOCTEST_MAIN_INFO,
     crate::doc::SUSPICIOUS_DOC_COMMENTS_INFO,
     crate::doc::TEST_ATTR_IN_DOCTEST_INFO,
+    crate::doc::TOO_LONG_FIRST_DOC_PARAGRAPH_INFO,
     crate::doc::UNNECESSARY_SAFETY_DOC_INFO,
     crate::double_parens::DOUBLE_PARENS_INFO,
     crate::drop_forget_ref::DROP_NON_DROP_INFO,
diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs
@@ -1,4 +1,6 @@
 mod lazy_continuation;
+mod too_long_first_doc_paragraph;
+
 use clippy_config::Conf;
 use clippy_utils::attrs::is_doc_hidden;
 use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
@@ -422,6 +424,38 @@ declare_clippy_lint! {
     "require every line of a paragraph to be indented and marked"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks if the first line in the documentation of items listed in module page is too long.
+    ///
+    /// ### Why is this bad?
+    /// Documentation will show the first paragraph of the doscstring in the summary page of a
+    /// module, so having a nice, short summary in the first paragraph is part of writing good docs.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// /// A very short summary.
+    /// /// A much longer explanation that goes into a lot more detail about
+    /// /// how the thing works, possibly with doclinks and so one,
+    /// /// and probably spanning a many rows.
+    /// struct Foo {}
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// /// A very short summary.
+    /// ///
+    /// /// A much longer explanation that goes into a lot more detail about
+    /// /// how the thing works, possibly with doclinks and so one,
+    /// /// and probably spanning a many rows.
+    /// struct Foo {}
+    /// ```
+    #[clippy::version = "1.81.0"]
+    pub TOO_LONG_FIRST_DOC_PARAGRAPH,
+    style,
+    "ensure that the first line of a documentation paragraph isn't too long"
+}
+
+#[derive(Clone)]
 pub struct Documentation {
     valid_idents: &'static FxHashSet<String>,
     check_private_items: bool,
@@ -448,6 +482,7 @@ impl_lint_pass!(Documentation => [
     SUSPICIOUS_DOC_COMMENTS,
     EMPTY_DOCS,
     DOC_LAZY_CONTINUATION,
+    TOO_LONG_FIRST_DOC_PARAGRAPH,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Documentation {
@@ -457,39 +492,44 @@ impl<'tcx> LateLintPass<'tcx> for Documentation {
         };
 
         match cx.tcx.hir_node(cx.last_node_with_lint_attrs) {
-            Node::Item(item) => match item.kind {
-                ItemKind::Fn(sig, _, body_id) => {
-                    if !(is_entrypoint_fn(cx, item.owner_id.to_def_id()) || in_external_macro(cx.tcx.sess, item.span)) {
-                        let body = cx.tcx.hir().body(body_id);
-
-                        let panic_info = FindPanicUnwrap::find_span(cx, cx.tcx.typeck(item.owner_id), body.value);
-                        missing_headers::check(
+            Node::Item(item) => {
+                too_long_first_doc_paragraph::check(cx, attrs, item.kind, headers.first_paragraph_len);
+                match item.kind {
+                    ItemKind::Fn(sig, _, body_id) => {
+                        if !(is_entrypoint_fn(cx, item.owner_id.to_def_id())
+                            || in_external_macro(cx.tcx.sess, item.span))
+                        {
+                            let body = cx.tcx.hir().body(body_id);
+
+                            let panic_info = FindPanicUnwrap::find_span(cx, cx.tcx.typeck(item.owner_id), body.value);
+                            missing_headers::check(
+                                cx,
+                                item.owner_id,
+                                sig,
+                                headers,
+                                Some(body_id),
+                                panic_info,
+                                self.check_private_items,
+                            );
+                        }
+                    },
+                    ItemKind::Trait(_, unsafety, ..) => match (headers.safety, unsafety) {
+                        (false, Safety::Unsafe) => span_lint(
                             cx,
-                            item.owner_id,
-                            sig,
-                            headers,
-                            Some(body_id),
-                            panic_info,
-                            self.check_private_items,
-                        );
-                    }
-                },
-                ItemKind::Trait(_, unsafety, ..) => match (headers.safety, unsafety) {
-                    (false, Safety::Unsafe) => span_lint(
-                        cx,
-                        MISSING_SAFETY_DOC,
-                        cx.tcx.def_span(item.owner_id),
-                        "docs for unsafe trait missing `# Safety` section",
-                    ),
-                    (true, Safety::Safe) => span_lint(
-                        cx,
-                        UNNECESSARY_SAFETY_DOC,
-                        cx.tcx.def_span(item.owner_id),
-                        "docs for safe trait have unnecessary `# Safety` section",
-                    ),
+                            MISSING_SAFETY_DOC,
+                            cx.tcx.def_span(item.owner_id),
+                            "docs for unsafe trait missing `# Safety` section",
+                        ),
+                        (true, Safety::Safe) => span_lint(
+                            cx,
+                            UNNECESSARY_SAFETY_DOC,
+                            cx.tcx.def_span(item.owner_id),
+                            "docs for safe trait have unnecessary `# Safety` section",
+                        ),
+                        _ => (),
+                    },
                     _ => (),
-                },
-                _ => (),
+                }
             },
             Node::TraitItem(trait_item) => {
                 if let TraitItemKind::Fn(sig, ..) = trait_item.kind
@@ -547,6 +587,7 @@ struct DocHeaders {
     safety: bool,
     errors: bool,
     panics: bool,
+    first_paragraph_len: usize,
 }
 
 /// Does some pre-processing on raw, desugared `#[doc]` attributes such as parsing them and
@@ -586,8 +627,9 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
         acc
     });
     doc.pop();
+    let doc = doc.trim();
 
-    if doc.trim().is_empty() {
+    if doc.is_empty() {
         if let Some(span) = span_of_fragments(&fragments) {
             span_lint_and_help(
                 cx,
@@ -611,7 +653,7 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
         cx,
         valid_idents,
         parser.into_offset_iter(),
-        &doc,
+        doc,
         Fragments {
             fragments: &fragments,
             doc: &doc,
@@ -653,6 +695,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
     let mut paragraph_range = 0..0;
     let mut code_level = 0;
     let mut blockquote_level = 0;
+    let mut is_first_paragraph = true;
 
     let mut containers = Vec::new();
 
@@ -720,6 +763,10 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
                 }
                 ticks_unbalanced = false;
                 paragraph_range = range;
+                if is_first_paragraph {
+                    headers.first_paragraph_len = doc[paragraph_range.clone()].chars().count();
+                    is_first_paragraph = false;
+                }
             },
             End(TagEnd::Heading(_) | TagEnd::Paragraph | TagEnd::Item) => {
                 if let End(TagEnd::Heading(_)) = event {
diff --git a/clippy_lints/src/doc/too_long_first_doc_paragraph.rs b/clippy_lints/src/doc/too_long_first_doc_paragraph.rs
@@ -0,0 +1,85 @@
+use rustc_ast::ast::Attribute;
+use rustc_errors::Applicability;
+use rustc_hir::ItemKind;
+use rustc_lint::LateContext;
+
+use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
+use clippy_utils::source::snippet_opt;
+
+use super::TOO_LONG_FIRST_DOC_PARAGRAPH;
+
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    attrs: &[Attribute],
+    item_kind: ItemKind<'_>,
+    mut first_paragraph_len: usize,
+) {
+    if first_paragraph_len <= 100
+        || !matches!(
+            item_kind,
+            ItemKind::Static(..)
+                | ItemKind::Const(..)
+                | ItemKind::Fn(..)
+                | ItemKind::Macro(..)
+                | ItemKind::Mod(..)
+                | ItemKind::TyAlias(..)
+                | ItemKind::Enum(..)
+                | ItemKind::Struct(..)
+                | ItemKind::Union(..)
+                | ItemKind::Trait(..)
+                | ItemKind::TraitAlias(..)
+        )
+    {
+        return;
+    }
+    let mut spans = Vec::new();
+    let mut should_suggest_empty_doc = false;
+
+    for attr in attrs {
+        if let Some(doc) = attr.doc_str() {
+            spans.push(attr.span);
+            let doc = doc.as_str();
+            let doc = doc.trim();
+            if spans.len() == 1 {
+                // We make this suggestion only if the first doc line ends with a punctuation
+                // because if might just need to add an empty line with `///`.
+                should_suggest_empty_doc = doc.ends_with('.') || doc.ends_with('!') || doc.ends_with('?');
+            }
+            let len = doc.chars().count();
+            if len >= first_paragraph_len {
+                break;
+            }
+            first_paragraph_len -= len;
+        }
+    }
+
+    let &[first_span, .., last_span] = spans.as_slice() else { return };
+
+    if should_suggest_empty_doc
+        && let Some(second_span) = spans.get(1)
+        && let new_span = first_span.with_hi(second_span.lo()).with_lo(first_span.hi())
+        && let Some(snippet) = snippet_opt(cx, new_span)
+    {
+        span_lint_and_then(
+            cx,
+            TOO_LONG_FIRST_DOC_PARAGRAPH,
+            first_span.with_hi(last_span.lo()),
+            "first doc comment paragraph is too long",
+            |diag| {
+                diag.span_suggestion(
+                    new_span,
+                    "add",
+                    format!("{snippet}///\n"),
+                    Applicability::MachineApplicable,
+                );
+            },
+        );
+        return;
+    }
+    span_lint(
+        cx,
+        TOO_LONG_FIRST_DOC_PARAGRAPH,
+        first_span.with_hi(last_span.lo()),
+        "first doc comment paragraph is too long",
+    );
+}
diff --git a/tests/ui/too_long_first_doc_paragraph-fix.fixed b/tests/ui/too_long_first_doc_paragraph-fix.fixed
@@ -0,0 +1,8 @@
+#![warn(clippy::too_long_first_doc_paragraph)]
+
+/// A very short summary.
+///
+/// A much longer explanation that goes into a lot more detail about
+/// how the thing works, possibly with doclinks and so one,
+/// and probably spanning a many rows.
+struct Foo;
diff --git a/tests/ui/too_long_first_doc_paragraph-fix.rs b/tests/ui/too_long_first_doc_paragraph-fix.rs
@@ -0,0 +1,7 @@
+#![warn(clippy::too_long_first_doc_paragraph)]
+
+/// A very short summary.
+/// A much longer explanation that goes into a lot more detail about
+/// how the thing works, possibly with doclinks and so one,
+/// and probably spanning a many rows.
+struct Foo;
diff --git a/tests/ui/too_long_first_doc_paragraph-fix.stderr b/tests/ui/too_long_first_doc_paragraph-fix.stderr
@@ -0,0 +1,19 @@
+error: first doc comment paragraph is too long
+  --> tests/ui/too_long_first_doc_paragraph-fix.rs:3:1
+   |
+LL | / /// A very short summary.
+LL | | /// A much longer explanation that goes into a lot more detail about
+LL | | /// how the thing works, possibly with doclinks and so one,
+LL | | /// and probably spanning a many rows.
+   | |_
+   |
+   = note: `-D clippy::too-long-first-doc-paragraph` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::too_long_first_doc_paragraph)]`
+help: add an empty line
+   |
+LL ~ /// A very short summary.
+LL + ///
+   |
+
+error: aborting due to 1 previous error
+
diff --git a/tests/ui/too_long_first_doc_paragraph.rs b/tests/ui/too_long_first_doc_paragraph.rs
@@ -0,0 +1,47 @@
+//@no-rustfix
+
+#![warn(clippy::too_long_first_doc_paragraph)]
+
+/// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nunc turpis nunc, lacinia
+/// a dolor in, pellentesque aliquet enim. Cras nec maximus sem. Mauris arcu libero,
+/// gravida non lacinia at, rhoncus eu lacus.
+pub struct Bar;
+
+// Should not warn! (not an item visible on mod page)
+/// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nunc turpis nunc, lacinia
+/// a dolor in, pellentesque aliquet enim. Cras nec maximus sem. Mauris arcu libero,
+/// gravida non lacinia at, rhoncus eu lacus.
+impl Bar {}
+
+// Should not warn! (less than 80 characters)
+/// Lorem ipsum dolor sit amet, consectetur adipiscing elit.
+///
+/// Nunc turpis nunc, lacinia
+/// a dolor in, pellentesque aliquet enim. Cras nec maximus sem. Mauris arcu libero,
+/// gravida non lacinia at, rhoncus eu lacus.
+enum Enum {
+    A,
+}
+
+/// Lorem
+/// ipsum dolor sit amet, consectetur adipiscing elit. Nunc turpis nunc, lacinia
+/// a dolor in, pellentesque aliquet enim. Cras nec maximus sem. Mauris arcu libero,
+/// gravida non lacinia at, rhoncus eu lacus.
+union Union {
+    a: u8,
+    b: u8,
+}
+
+// Should not warn! (title)
+/// # bla
+/// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nunc turpis nunc, lacinia
+/// a dolor in, pellentesque aliquet enim. Cras nec maximus sem. Mauris arcu libero,
+/// gravida non lacinia at, rhoncus eu lacus.
+union Union2 {
+    a: u8,
+    b: u8,
+}
+
+fn main() {
+    // test code goes here
+}
diff --git a/tests/ui/too_long_first_doc_paragraph.stderr b/tests/ui/too_long_first_doc_paragraph.stderr
@@ -0,0 +1,22 @@
+error: first doc comment paragraph is too long
+  --> tests/ui/too_long_first_doc_paragraph.rs:5:1
+   |
+LL | / /// Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nunc turpis nunc, lacinia
+LL | | /// a dolor in, pellentesque aliquet enim. Cras nec maximus sem. Mauris arcu libero,
+LL | | /// gravida non lacinia at, rhoncus eu lacus.
+   | |_
+   |
+   = note: `-D clippy::too-long-first-doc-paragraph` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::too_long_first_doc_paragraph)]`
+
+error: first doc comment paragraph is too long
+  --> tests/ui/too_long_first_doc_paragraph.rs:26:1
+   |
+LL | / /// Lorem
+LL | | /// ipsum dolor sit amet, consectetur adipiscing elit. Nunc turpis nunc, lacinia
+LL | | /// a dolor in, pellentesque aliquet enim. Cras nec maximus sem. Mauris arcu libero,
+LL | | /// gravida non lacinia at, rhoncus eu lacus.
+   | |_
+
+error: aborting due to 2 previous errors
+
