diff --git a/CHANGELOG.md b/CHANGELOG.md index fe9fbd486c9c..0e60134c80f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5864,6 +5864,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 index 3b3d0db79dcc..25f1e412b367 100644 --- 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 index 3d875e7ac2d3..68a252452194 100644 --- a/clippy_lints/src/doc/mod.rs +++ b/clippy_lints/src/doc/mod.rs @@ -1,4 +1,5 @@ mod lazy_continuation; +mod too_long_first_doc_paragraph; use clippy_utils::attrs::is_doc_hidden; use clippy_utils::diagnostics::{span_lint, span_lint_and_help}; use clippy_utils::macros::{is_panic, root_macro_call_first_node}; @@ -420,6 +421,37 @@ 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: FxHashSet, @@ -447,6 +479,7 @@ impl_lint_pass!(Documentation => [ SUSPICIOUS_DOC_COMMENTS, EMPTY_DOCS, DOC_LAZY_CONTINUATION, + TOO_LONG_FIRST_DOC_PARAGRAPH, ]); impl<'tcx> LateLintPass<'tcx> for Documentation { @@ -456,39 +489,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 @@ -546,6 +584,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 @@ -585,8 +624,9 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet, 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, @@ -610,7 +650,7 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet, attrs: &[ cx, valid_idents, parser.into_offset_iter(), - &doc, + doc, Fragments { fragments: &fragments, doc: &doc, @@ -652,6 +692,7 @@ fn check_doc<'a, Events: Iterator, Range, Range { if let End(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 new file mode 100644 index 000000000000..9034ba417eb3 --- /dev/null +++ b/clippy_lints/src/doc/too_long_first_doc_paragraph.rs @@ -0,0 +1,88 @@ +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) = match spans.as_slice() { + &[first_span, .., last_span] => (first_span, last_span), + _ => 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 new file mode 100644 index 000000000000..b3f66f527938 --- /dev/null +++ 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 new file mode 100644 index 000000000000..f0ece9523de4 --- /dev/null +++ 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 new file mode 100644 index 000000000000..34171f4cf142 --- /dev/null +++ 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 + | +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 new file mode 100644 index 000000000000..88a8f6d38310 --- /dev/null +++ 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 new file mode 100644 index 000000000000..7f48e5cf884e --- /dev/null +++ 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 +