mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #12993 - GuillaumeGomez:too_long_first_doc_paragraph, r=Centri3

Browse Source 
Add new `too_long_first_doc_paragraph` first paragraph lint

Fixes https://github.com/rust-lang/rust-clippy/issues/12989.

changelog: Add new `too_long_first_doc_paragraph` first paragraph lint
This commit is contained in:
bors 2024-08-24 15:23:34 +00:00
commit 30e0b69908
13 changed files with 326 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -5914,6 +5914,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

1
clippy_lints/src/declared_lints.rs
View File

@ -144,6 +144,7 @@
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,

112
clippy_lints/src/doc/mod.rs
View File

@ -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 @@
"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<String>,
check_private_items: bool,
@ -448,6 +482,7 @@ pub fn new(conf: &'static Conf) -> Self {
SUSPICIOUS_DOC_COMMENTS,
EMPTY_DOCS,
DOC_LAZY_CONTINUATION,
TOO_LONG_FIRST_DOC_PARAGRAPH,
]);
impl<'tcx> LateLintPass<'tcx> for Documentation {
@ -457,39 +492,50 @@ fn check_attributes(&mut self, cx: &LateContext<'tcx>, attrs: &'tcx [Attribute])
};
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);
Node::Item(item) => {
too_long_first_doc_paragraph::check(
cx,
item,
attrs,
headers.first_paragraph_len,
self.check_private_items,
);
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(
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 +593,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
@ -653,6 +700,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 +768,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 {

91
clippy_lints/src/doc/too_long_first_doc_paragraph.rs Normal file
View File

@ -0,0 +1,91 @@
use rustc_ast::ast::Attribute;
use rustc_errors::Applicability;
use rustc_hir::{Item, ItemKind};
use rustc_lint::LateContext;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::is_from_proc_macro;
use clippy_utils::source::snippet_opt;
use super::TOO_LONG_FIRST_DOC_PARAGRAPH;
pub(super) fn check(
cx: &LateContext<'_>,
item: &Item<'_>,
attrs: &[Attribute],
mut first_paragraph_len: usize,
check_private_items: bool,
) {
if !check_private_items && !cx.effective_visibilities.is_exported(item.owner_id.def_id) {
return;
}
if first_paragraph_len <= 200
|| !matches!(
item.kind,
// This is the list of items which can be documented AND are displayed on the module
// page. So associated items or impl blocks are not part of this list.
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 it 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 is_from_proc_macro(cx, item) {
return;
}
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| {
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)
{
diag.span_suggestion(
new_span,
"add an empty line",
format!("{snippet}///\n"),
Applicability::MachineApplicable,
);
}
},
);
}

21
clippy_utils/src/lib.rs
View File

@ -149,6 +149,7 @@ fn check_attributes_post(&mut self, cx: &rustc_lint::$context<'_>, attrs: &[rust
/// If the given expression is a local binding, find the initializer expression.
/// If that initializer expression is another local binding, find its initializer again.
///
/// This process repeats as long as possible (but usually no more than once). Initializer
/// expressions with adjustments are ignored. If this is not desired, use [`find_binding_init`]
/// instead.
@ -179,6 +180,7 @@ pub fn expr_or_init<'a, 'b, 'tcx: 'b>(cx: &LateContext<'tcx>, mut expr: &'a Expr
}
/// Finds the initializer expression for a local binding. Returns `None` if the binding is mutable.
///
/// By only considering immutable bindings, we guarantee that the returned expression represents the
/// value of the binding wherever it is referenced.
///
@ -431,12 +433,12 @@ pub fn qpath_generic_tys<'tcx>(qpath: &QPath<'tcx>) -> impl Iterator<Item = &'tc
})
}
/// THIS METHOD IS DEPRECATED and will eventually be removed since it does not match against the
/// THIS METHOD IS DEPRECATED. Matches a `QPath` against a slice of segment string literals.
///
/// This method is deprecated and will eventually be removed since it does not match against the
/// entire path or resolved `DefId`. Prefer using `match_def_path`. Consider getting a `DefId` from
/// `QPath::Resolved.1.res.opt_def_id()`.
///
/// Matches a `QPath` against a slice of segment string literals.
///
/// There is also `match_path` if you are dealing with a `rustc_hir::Path` instead of a
/// `rustc_hir::QPath`.
///
@ -485,12 +487,12 @@ pub fn is_path_diagnostic_item<'tcx>(
path_def_id(cx, maybe_path).map_or(false, |id| cx.tcx.is_diagnostic_item(diag_item, id))
}
/// THIS METHOD IS DEPRECATED and will eventually be removed since it does not match against the
/// THIS METHOD IS DEPRECATED. Matches a `Path` against a slice of segment string literals.
///
/// This method is deprecated and will eventually be removed since it does not match against the
/// entire path or resolved `DefId`. Prefer using `match_def_path`. Consider getting a `DefId` from
/// `QPath::Resolved.1.res.opt_def_id()`.
///
/// Matches a `Path` against a slice of segment string literals.
///
/// There is also `match_qpath` if you are dealing with a `rustc_hir::QPath` instead of a
/// `rustc_hir::Path`.
///
@ -905,6 +907,7 @@ pub fn is_default_equivalent_call(cx: &LateContext<'_>, repl_func: &Expr<'_>) ->
}
/// Returns true if the expr is equal to `Default::default()` of it's type when evaluated.
///
/// It doesn't cover all cases, for example indirect function calls (some of std
/// functions are supported) but it is the best we have.
pub fn is_default_equivalent(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
@ -1061,6 +1064,7 @@ fn bitor_assign(&mut self, rhs: Self) {
}
/// Given an expression referencing a local, determines how it would be captured in a closure.
///
/// Note as this will walk up to parent expressions until the capture can be determined it should
/// only be used while making a closure somewhere a value is consumed. e.g. a block, match arm, or
/// function argument (other than a receiver).
@ -2365,8 +2369,9 @@ pub fn fn_def_id_with_node_args<'tcx>(
}
/// Returns `Option<String>` where String is a textual representation of the type encapsulated in
/// the slice iff the given expression is a slice of primitives (as defined in the
/// `is_recursively_primitive_type` function) and `None` otherwise.
/// the slice iff the given expression is a slice of primitives.
///
/// (As defined in the `is_recursively_primitive_type` function.) Returns `None` otherwise.
pub fn is_slice_of_primitives(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<String> {
let expr_type = cx.typeck_results().expr_ty_adjusted(expr);
let expr_kind = expr_type.kind();

9
clippy_utils/src/macros.rs
View File

@ -150,10 +150,11 @@ pub fn first_node_macro_backtrace(cx: &LateContext<'_>, node: &impl HirNode) ->
}
/// If `node` is the "first node" in a macro expansion, returns `Some` with the `ExpnId` of the
/// macro call site (i.e. the parent of the macro expansion). This generally means that `node`
/// is the outermost node of an entire macro expansion, but there are some caveats noted below.
/// This is useful for finding macro calls while visiting the HIR without processing the macro call
/// at every node within its expansion.
/// macro call site (i.e. the parent of the macro expansion).
///
/// This generally means that `node` is the outermost node of an entire macro expansion, but there
/// are some caveats noted below. This is useful for finding macro calls while visiting the HIR
/// without processing the macro call at every node within its expansion.
///
/// If you already have immediate access to the parent node, it is simpler to
/// just check the context of that span directly (e.g. `parent.span.from_expansion()`).

14
clippy_utils/src/source.rs
View File

@ -589,9 +589,10 @@ pub fn snippet_block_with_context<'a>(
(reindent_multiline(snip, true, indent), from_macro)
}
/// Same as `snippet_with_applicability`, but first walks the span up to the given context. This
/// will result in the macro call, rather than the expansion, if the span is from a child context.
/// If the span is not from a child context, it will be used directly instead.
/// Same as `snippet_with_applicability`, but first walks the span up to the given context.
///
/// This will result in the macro call, rather than the expansion, if the span is from a child
/// context. If the span is not from a child context, it will be used directly instead.
///
/// e.g. Given the expression `&vec![]`, getting a snippet from the span for `vec![]` as a HIR node
/// would result in `box []`. If given the context of the address of expression, this function will
@ -634,9 +635,10 @@ fn snippet_with_context_sess<'a>(
}
/// Walks the span up to the target context, thereby returning the macro call site if the span is
/// inside a macro expansion, or the original span if it is not. Note this will return `None` in the
/// case of the span being in a macro expansion, but the target context is from expanding a macro
/// argument.
/// inside a macro expansion, or the original span if it is not.
///
/// Note this will return `None` in the case of the span being in a macro expansion, but the target
/// context is from expanding a macro argument.
///
/// Given the following
///

20
clippy_utils/src/ty.rs
View File

@ -160,8 +160,10 @@ pub fn get_type_diagnostic_name(cx: &LateContext<'_>, ty: Ty<'_>) -> Option<Symb
}
/// Returns true if `ty` is a type on which calling `Clone` through a function instead of
/// as a method, such as `Arc::clone()` is considered idiomatic. Lints should avoid suggesting to
/// replace instances of `ty::Clone()` by `.clone()` for objects of those types.
/// as a method, such as `Arc::clone()` is considered idiomatic.
///
/// Lints should avoid suggesting to replace instances of `ty::Clone()` by `.clone()` for objects
/// of those types.
pub fn should_call_clone_as_function(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
matches!(
get_type_diagnostic_name(cx, ty),
@ -398,8 +400,10 @@ fn is_normalizable_helper<'tcx>(
}
/// Returns `true` if the given type is a non aggregate primitive (a `bool` or `char`, any
/// integer or floating-point number type). For checking aggregation of primitive types (e.g.
/// tuples and slices of primitive type) see `is_recursively_primitive_type`
/// integer or floating-point number type).
///
/// For checking aggregation of primitive types (e.g. tuples and slices of primitive type) see
/// `is_recursively_primitive_type`
pub fn is_non_aggregate_primitive_type(ty: Ty<'_>) -> bool {
matches!(ty.kind(), ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_))
}
@ -471,9 +475,10 @@ pub fn match_type(cx: &LateContext<'_>, ty: Ty<'_>, path: &[&str]) -> bool {
}
}
/// Checks if the drop order for a type matters. Some std types implement drop solely to
/// deallocate memory. For these types, and composites containing them, changing the drop order
/// won't result in any observable side effects.
/// Checks if the drop order for a type matters.
///
/// Some std types implement drop solely to deallocate memory. For these types, and composites
/// containing them, changing the drop order won't result in any observable side effects.
pub fn needs_ordered_drop<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
fn needs_ordered_drop_inner<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, seen: &mut FxHashSet<Ty<'tcx>>) -> bool {
if !seen.insert(ty) {
@ -1306,6 +1311,7 @@ pub fn deref_chain<'cx, 'tcx>(cx: &'cx LateContext<'tcx>, ty: Ty<'tcx>) -> impl
}
/// Checks if a Ty<'_> has some inherent method Symbol.
///
/// This does not look for impls in the type's `Deref::Target` type.
/// If you need this, you should wrap this call in `clippy_utils::ty::deref_chain().any(...)`.
pub fn get_adt_inherent_method<'a>(cx: &'a LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> Option<&'a AssocItem> {

9
tests/ui/too_long_first_doc_paragraph-fix.fixed Normal file
View File

@ -0,0 +1,9 @@
#![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. Blablabla, it needs to be over
/// 200 characters so I needed to write something longeeeeeeer.
pub struct Foo;

8
tests/ui/too_long_first_doc_paragraph-fix.rs Normal file
View File

@ -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. Blablabla, it needs to be over
/// 200 characters so I needed to write something longeeeeeeer.
pub struct Foo;

20
tests/ui/too_long_first_doc_paragraph-fix.stderr Normal file
View File

@ -0,0 +1,20 @@
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. Blablabla, it needs to be over
LL | | /// 200 characters so I needed to write something longeeeeeeer.
| |_
|
= 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

53
tests/ui/too_long_first_doc_paragraph.rs Normal file
View File

@ -0,0 +1,53 @@
//@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.
pub 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.
pub 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.
pub union Union2 {
a: u8,
b: u8,
}
// Should not warn! (not public)
/// 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.
fn f() {}
fn main() {
// test code goes here
}

22
tests/ui/too_long_first_doc_paragraph.stderr Normal file
View File

@ -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