rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.45k stars 1.54k forks source link

`too_long_first_doc_paragraph` is extremely noisy and painful to fix and should not be enabled by default #13441

Closed alice-i-cecile closed 1 month ago

alice-i-cecile commented 1 month ago

Description

Fixing this lint in Bevy's codebase is extremely onerous: +11k / -7k of trivial but not automatic changes for a very minor improvement. We're going to have to blanket allow this at the workspace level, despite agreeing with the motivation to improve IDE tooltips.

See https://github.com/bevyengine/bevy/pull/15375 for what this looked like in practice.

Version

No response

Additional Labels

No response

alice-i-cecile commented 1 month ago

Hmm. This mostly seems to be an issue with comment wrapping. Closing.

clarfonthey commented 1 month ago

Also just adding here as someone who was poking around trying to fix this: the biggest issue is that if the sentence is on the first line but contains other prose after it, you can't easily separate it without making invasive changes. So, it might be good to have the auto-fixable version be a separate lint: when there's a single sentence on one line, but you can just add in a newline to make it its own paragraph, that's pretty uncontroversial and easy to change.

Here's an example comment demonstrating this:

/// A strong or weak handle to a specific [`Asset`]. If a [`Handle`] is [`Handle::Strong`], the [`Asset`] will be kept
/// alive until the [`Handle`] is dropped. If a [`Handle`] is [`Handle::Weak`], it does not necessarily reference a live [`Asset`],
/// nor will it keep assets alive.

If you want to make this its own line, you could just naïvely split it, like so:

/// A strong or weak handle to a specific [`Asset`].
///
/// If a [`Handle`] is [`Handle::Strong`], the [`Asset`] will be kept
/// alive until the [`Handle`] is dropped. If a [`Handle`] is [`Handle::Weak`], it does not necessarily reference a live [`Asset`],
/// nor will it keep assets alive.

But now, we have this awkwardly formatted comment. It can be especially bad if there's only a single word or two on the line after the first sentence, so now those words are on their own line. We could use rustfmt's wrap_comments functionality, but this is unstable and as we saw, incredibly prone to error and churn.

Basically: yes, it's good to have a single line summary, but if you don't have one, you basically have to reflow the entire paragraph or manually do things, which isn't very great.