Skip to content

Commit

Permalink
Add new too_long_first_doc_paragraph first paragraph lint
Browse files Browse the repository at this point in the history
  • Loading branch information
GuillaumeGomez committed Jul 26, 2024
1 parent 345c94c commit 760d03e
Show file tree
Hide file tree
Showing 9 changed files with 270 additions and 33 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
113 changes: 80 additions & 33 deletions clippy_lints/src/doc/mod.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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 {
Expand Down
85 changes: 85 additions & 0 deletions clippy_lints/src/doc/too_long_first_doc_paragraph.rs
Original file line number Diff line number Diff line change
@@ -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",
);
}
8 changes: 8 additions & 0 deletions tests/ui/too_long_first_doc_paragraph-fix.fixed
Original file line number Diff line number Diff line change
@@ -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;
7 changes: 7 additions & 0 deletions tests/ui/too_long_first_doc_paragraph-fix.rs
Original file line number Diff line number Diff line change
@@ -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;
19 changes: 19 additions & 0 deletions tests/ui/too_long_first_doc_paragraph-fix.stderr
Original file line number Diff line number Diff line change
@@ -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

47 changes: 47 additions & 0 deletions tests/ui/too_long_first_doc_paragraph.rs
Original file line number Diff line number Diff line change
@@ -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
}
22 changes: 22 additions & 0 deletions tests/ui/too_long_first_doc_paragraph.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 760d03e

Please sign in to comment.