rust-lang
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
r? @Centri3
rustbot has assigned @Centri3. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
I increased the limit to 100 and I also applied suggestions from @flip1995 to suggest to add an empty doc line to separate the first sentence from the rest when applicable.
Any idea how I could disable just this lint on clippy tests by any chance?
I would rather take a closer look at these and improve the lint a bit. E.g. this seems like a FP to me: https://github.com/rust-lang/rust-clippy/actions/runs/9715600206/job/26817360799?pr=12993#step:5:3958
error: first doc comment paragraph is too long
--> tests/ui/index_refutable_slice/if_let_slice_binding.rs:168:1
|
LL | / /// The lint will ignore bindings with sub patterns as it would be hard
LL | | /// to build correct suggestions for these instances :)
| |_
This is still a really short comment and IMO fine to have in documentation.
But it's not a short description. In this case it should be:
/// The lint will ignore bindings with sub patterns.
For the second part (the "why?"), it should be part of the rest of the doc comment.
I disagree. If the explanation for the first statement is as short as this, I'd rather be able to read it in the docs.rs overview, than having to click on the item to try to understand why this is the case.
I think it is hard to argue that the first (long) version is bad doc writing. This is a screenshot when using half my screen. It's barely 2 lines in the summary, but I have all information I need, without having to click on the item.
In any case, I don't think this should warn on private items, that won't be shown in the documentation. Or at least, this should be configurable, with the default being not to lint on private items.
I can config for the length of the line (default to 120?) and if it should warn on private items (default to false?). What do you think?
I think that would be a good step. However, I don't know how much it helps to make the line length configurable.
I don't either. We either need to pick the maximum length or allow users to change it however they want. But it seems like we can't agree on a line length here already. :laughing:
I think it's a wise principle for lints to err on the side of being conservative (that is, giving the user's code the benefit of the doubt). A lint that triggers too often will soon be silenced, whereas a lint that's a little too easy-going is still useful.
I looked again at a few samples from the standard library, and the longest intro paragraphs that seemed to me to keep the spirit of "one short sentence" were no longer than about 150 characters. Example:
Returns vector content as a slice of
T, along with the remaining spare capacity of the vector as a slice ofMaybeUninit<T>.
Clearly if that sentence were longer, it would not be short! But it also seems quite reasonable to me. Adding a 50% safety margin would give us a limit of 225.
There are lots of cases in the stdlib docs where this limit is exceeded, but I think those cases should trigger the lint! For example:
Consumes and leaks the
Vec, returning a mutable reference to the contents,&'a mut [T]. Note that the typeTmust outlive the chosen lifetime'a. If the type has only static references, or none at all, then this may be chosen to be'static.
The second and third sentences here should really move to the next paragraph, so the lint would already be delivering value here. A limit of 250 characters would have missed this, so that's clearly too high. A limit of less than 200 would already be incorrectly catching lots of cases that, on inspection, are quite reasonable.
Thus, anything within the 200-250 range is defensible, but it seems hard to argue for a limit outside that.
Let's go for 200 then. We can always add the config later on if needed.
200 sounds good to me. The default comment width is 80 in rustfmt. So 200 is 2.5x of that. Which is in line with @bitfield argument to add a 50% safety margin, just from a different starting point. 2 lines of first line documentation seems fine to me, as weird as that sounds.
If you think there's value in making this configurable, go for it. It doesn't cost much.
I still haven't looked at the implementation, but if the lint produces a suggestion, I think a good idea would be to suggest a break after the first . or other punctuation found in the too-long-line.
Updated the limit to 200. Added configuration for private items and fixed dogfood.
Lintcheck diff summary looks good and those all seem to be cases where the documentation could be improved. Sometimes trivially.
I'll let @Centri3 do the code review of the lint, but the behavior now seems good to me.
:umbrella: The latest upstream changes (presumably #13088) made this pull request unmergeable. Please resolve the merge conflicts.
:pushpin: Commit a2033428c4ca9883a2b7cbbd1f167d481b0d240c has been approved by Centri3
It is now in the queue for this repository.
:hourglass: Testing commit a2033428c4ca9883a2b7cbbd1f167d481b0d240c with merge 30e0b69908332ea5556511b42f66e50e9fd3fbc8...
:sunny: Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test Approved by: Centri3 Pushing 30e0b69908332ea5556511b42f66e50e9fd3fbc8 to master...