[PATCH] rust: fix clippy::too-long-first-doc-paragraph

[PATCH] rust: fix clippy::too-long-first-doc-paragraph

[Benno Lossin]

When running `make LLVM=1 CLIPPY=1` on my machine, I get this error:

    error: first doc comment paragraph is too long
      --> rust/kernel/driver.rs:13:1
       |
    13 | / /// The [`RegistrationOps`] trait serves as generic interface for subsystems (e.g., PCI, Platform,
    14 | | /// Amba, etc.) to provide the corresponding subsystem specific implementation to register /
    15 | | /// unregister a driver of the particular type (`RegType`).
    16 | | ///
    17 | | /// For instance, the PCI subsystem would set `RegType` to `bindings::pci_driver` and call
       | |_
       |
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_long_first_doc_paragraph
       = 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)]`

Thus add a short one-line description.

Fixes: ea7e18289f44 ("rust: implement generic driver registration")
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
The error also occurs in v6.14-rc1, so it must have slipped through our
testing, which I find a bit strange. Also nobody reported it for rc1, so
maybe this is only something that I encountered?
---
 rust/kernel/driver.rs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index 2a16d5e64e6c..65c9c1776556 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -10,6 +10,8 @@
 use core::pin::Pin;
 use macros::{pin_data, pinned_drop};
 
+/// Generic interface for subsystem driver registrations.
+///
 /// The [`RegistrationOps`] trait serves as generic interface for subsystems (e.g., PCI, Platform,
 /// Amba, etc.) to provide the corresponding subsystem specific implementation to register /
 /// unregister a driver of the particular type (`RegType`).

base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
-- 
2.46.0

Re: [PATCH] rust: fix clippy::too-long-first-doc-paragraph

[Charalampos Mitrodimas]

Benno Lossin <benno.lossin@proton.me> writes:

> When running `make LLVM=1 CLIPPY=1` on my machine, I get this error:
>
>     error: first doc comment paragraph is too long
>       --> rust/kernel/driver.rs:13:1
>        |
>     13 | / /// The [`RegistrationOps`] trait serves as generic interface for subsystems (e.g., PCI, Platform,
>     14 | | /// Amba, etc.) to provide the corresponding subsystem specific implementation to register /
>     15 | | /// unregister a driver of the particular type (`RegType`).
>     16 | | ///
>     17 | | /// For instance, the PCI subsystem would set `RegType` to `bindings::pci_driver` and call
>        | |_
>        |
>        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_long_first_doc_paragraph
>        = 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)]`
>
> Thus add a short one-line description.
>
> Fixes: ea7e18289f44 ("rust: implement generic driver registration")
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
> The error also occurs in v6.14-rc1, so it must have slipped through our
> testing, which I find a bit strange. Also nobody reported it for rc1, so
> maybe this is only something that I encountered?
> ---
>  rust/kernel/driver.rs | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
> index 2a16d5e64e6c..65c9c1776556 100644
> --- a/rust/kernel/driver.rs
> +++ b/rust/kernel/driver.rs
> @@ -10,6 +10,8 @@
>  use core::pin::Pin;
>  use macros::{pin_data, pinned_drop};
>  
> +/// Generic interface for subsystem driver registrations.
> +///
>  /// The [`RegistrationOps`] trait serves as generic interface for subsystems (e.g., PCI, Platform,
>  /// Amba, etc.) to provide the corresponding subsystem specific implementation to register /
>  /// unregister a driver of the particular type (`RegType`).
>
> base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3

Hi,

I cannot reproduce this as-is, but adding
"-Wclippy::too_long_first_doc_paragraph" to the "rust_common_flags" in
the Makefile reproduces it. Maybe try adding it there in your patch?

diff --git a/Makefile b/Makefile
index 89628e354..e1b14fb68 100644
--- a/Makefile
+++ b/Makefile
@@ -486,6 +486,7 @@ export rust_common_flags := --edition=2021 \
 			    -Wclippy::undocumented_unsafe_blocks \
 			    -Wclippy::unnecessary_safety_comment \
 			    -Wclippy::unnecessary_safety_doc \
+			    -Wclippy::too_long_first_doc_paragraph \
 			    -Wrustdoc::missing_crate_level_docs \
 			    -Wrustdoc::unescaped_backticks
 

Re: [PATCH] rust: fix clippy::too-long-first-doc-paragraph

[Benno Lossin]

On 16.02.25 01:12, Charalampos Mitrodimas wrote:
> Benno Lossin <benno.lossin@proton.me> writes:
> 
>> When running `make LLVM=1 CLIPPY=1` on my machine, I get this error:
>>
>>     error: first doc comment paragraph is too long
>>       --> rust/kernel/driver.rs:13:1
>>        |
>>     13 | / /// The [`RegistrationOps`] trait serves as generic interface for subsystems (e.g., PCI, Platform,
>>     14 | | /// Amba, etc.) to provide the corresponding subsystem specific implementation to register /
>>     15 | | /// unregister a driver of the particular type (`RegType`).
>>     16 | | ///
>>     17 | | /// For instance, the PCI subsystem would set `RegType` to `bindings::pci_driver` and call
>>        | |_
>>        |
>>        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_long_first_doc_paragraph
>>        = 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)]`
>>
>> Thus add a short one-line description.
>>
>> Fixes: ea7e18289f44 ("rust: implement generic driver registration")
>> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
>> ---
>> The error also occurs in v6.14-rc1, so it must have slipped through our
>> testing, which I find a bit strange. Also nobody reported it for rc1, so
>> maybe this is only something that I encountered?
>> ---
>>  rust/kernel/driver.rs | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
>> index 2a16d5e64e6c..65c9c1776556 100644
>> --- a/rust/kernel/driver.rs
>> +++ b/rust/kernel/driver.rs
>> @@ -10,6 +10,8 @@
>>  use core::pin::Pin;
>>  use macros::{pin_data, pinned_drop};
>>
>> +/// Generic interface for subsystem driver registrations.
>> +///
>>  /// The [`RegistrationOps`] trait serves as generic interface for subsystems (e.g., PCI, Platform,
>>  /// Amba, etc.) to provide the corresponding subsystem specific implementation to register /
>>  /// unregister a driver of the particular type (`RegType`).
>>
>> base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3
> 
> Hi,
> 
> I cannot reproduce this as-is, but adding
> "-Wclippy::too_long_first_doc_paragraph" to the "rust_common_flags" in
> the Makefile reproduces it. Maybe try adding it there in your patch?

I have done some more digging and bisected my Rust version. It turns out
I was on a rather old nightly from last September. I also don't get the
error on a newer compiler version. My bisection identified that the
error last occurs in nightly 2024-10-18, so from 2024-10-19 onwards it
compiles without the error.
So probably `-Wclippy::all` implied the `too_long_first_doc_paragraph`
lint in that version. That is probably because of [1]. It changes the
lint from style to nursery.

[1]: https://github.com/rust-lang/rust-clippy/pull/13551

So we don't need this patch, as it seems this never hit stable. However,
there is already a patch fixing what this lint reports: 2f390cc58943
("rust: provide proper code documentation titles").

I think it's a good lint, so maybe we should turn it on?

---
Cheers,
Benno

> 
> diff --git a/Makefile b/Makefile
> index 89628e354..e1b14fb68 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -486,6 +486,7 @@ export rust_common_flags := --edition=2021 \
>  			    -Wclippy::undocumented_unsafe_blocks \
>  			    -Wclippy::unnecessary_safety_comment \
>  			    -Wclippy::unnecessary_safety_doc \
> +			    -Wclippy::too_long_first_doc_paragraph \
>  			    -Wrustdoc::missing_crate_level_docs \
>  			    -Wrustdoc::unescaped_backticks
> 

Re: [PATCH] rust: fix clippy::too-long-first-doc-paragraph

[Miguel Ojeda]

On Sun, Feb 16, 2025 at 1:18 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> I have done some more digging and bisected my Rust version. It turns out
> I was on a rather old nightly from last September. I also don't get the
> error on a newer compiler version. My bisection identified that the
> error last occurs in nightly 2024-10-18, so from 2024-10-19 onwards it
> compiles without the error.

Thanks Benno, that is very useful and saved me some work.

> So probably `-Wclippy::all` implied the `too_long_first_doc_paragraph`
> lint in that version. That is probably because of [1]. It changes the
> lint from style to nursery.
>
> [1]: https://github.com/rust-lang/rust-clippy/pull/13551

Yeah, sometimes that happens when working regularly with nightly over time.

There is an upside, trying to look at it positively, which is that it
gives us something to look into from time to time (i.e. the new lint)
and give feedback, and sometimes we can clean it up anyway, even if
the lint is broken and we cannot enable it just yet :)

> So we don't need this patch, as it seems this never hit stable. However,
> there is already a patch fixing what this lint reports: 2f390cc58943
> ("rust: provide proper code documentation titles").
>
> I think it's a good lint, so maybe we should turn it on?

Agreed, we should -- it is something we tell people from time to time
to fix in review, so hopefully it will make patches better before the
review phase.

...well, assuming it works well enough, given it was moved to nursery,
e.g. in the linked Zulip thread in your link above they point to
https://github.com/rust-lang/rust-clippy/issues/13315 and
https://github.com/rust-lang/rust-clippy/issues/13538, which we may or
may not care about.

Thanks!

Cheers,
Miguel

Re: [PATCH] rust: fix clippy::too-long-first-doc-paragraph

[Charalampos Mitrodimas]

Benno Lossin <benno.lossin@proton.me> writes:

> On 16.02.25 01:12, Charalampos Mitrodimas wrote:
>> Benno Lossin <benno.lossin@proton.me> writes:
>> 
[..snip..]
>> Hi,
>> 
>> I cannot reproduce this as-is, but adding
>> "-Wclippy::too_long_first_doc_paragraph" to the "rust_common_flags" in
>> the Makefile reproduces it. Maybe try adding it there in your patch?
>
> I have done some more digging and bisected my Rust version. It turns out
> I was on a rather old nightly from last September. I also don't get the
> error on a newer compiler version. My bisection identified that the
> error last occurs in nightly 2024-10-18, so from 2024-10-19 onwards it
> compiles without the error.
> So probably `-Wclippy::all` implied the `too_long_first_doc_paragraph`
> lint in that version. That is probably because of [1]. It changes the
> lint from style to nursery.
>
> [1]: https://github.com/rust-lang/rust-clippy/pull/13551
>
> So we don't need this patch, as it seems this never hit stable. However,
> there is already a patch fixing what this lint reports: 2f390cc58943
> ("rust: provide proper code documentation titles").
>
> I think it's a good lint, so maybe we should turn it on?

Given the critical importance of readable documentation, I agree.

>
> ---
> Cheers,
> Benno

Re: [PATCH] rust: fix clippy::too-long-first-doc-paragraph

[Benno Lossin]

On 16.02.25 13:40, Miguel Ojeda wrote:
> On Sun, Feb 16, 2025 at 1:18 PM Benno Lossin <benno.lossin@proton.me> wrote:
>>
>> I have done some more digging and bisected my Rust version. It turns out
>> I was on a rather old nightly from last September. I also don't get the
>> error on a newer compiler version. My bisection identified that the
>> error last occurs in nightly 2024-10-18, so from 2024-10-19 onwards it
>> compiles without the error.
> 
> Thanks Benno, that is very useful and saved me some work.

My pleasure :)

>> So probably `-Wclippy::all` implied the `too_long_first_doc_paragraph`
>> lint in that version. That is probably because of [1]. It changes the
>> lint from style to nursery.
>>
>> [1]: https://github.com/rust-lang/rust-clippy/pull/13551
> 
> Yeah, sometimes that happens when working regularly with nightly over time.
> 
> There is an upside, trying to look at it positively, which is that it
> gives us something to look into from time to time (i.e. the new lint)
> and give feedback, and sometimes we can clean it up anyway, even if
> the lint is broken and we cannot enable it just yet :)

Yeah that is true.

>> So we don't need this patch, as it seems this never hit stable. However,
>> there is already a patch fixing what this lint reports: 2f390cc58943
>> ("rust: provide proper code documentation titles").
>>
>> I think it's a good lint, so maybe we should turn it on?
> 
> Agreed, we should -- it is something we tell people from time to time
> to fix in review, so hopefully it will make patches better before the
> review phase.
> 
> ...well, assuming it works well enough, given it was moved to nursery,
> e.g. in the linked Zulip thread in your link above they point to
> https://github.com/rust-lang/rust-clippy/issues/13315 and

I don't think that this one is bad. Of course it would be nice if only
the rendered form would be considered, but there might also be people
reading it directly in the code, so it could actually be a good idea to
also keep the raw text short.
In the case of long links, one can easily work around it by just not
using an inline link. (so having [text with link] and then after the
paragraph `[text with link]: link`.)

> https://github.com/rust-lang/rust-clippy/issues/13538, which we may or
> may not care about.

This one just seems like a nice-to-have, but not a deal breaker.

IMO we can enable the lint (since the instance fixed in this patch is
the only error I get with that lint) and just see how it behaves.

---
Cheers,
Benno

Re: [PATCH] rust: fix clippy::too-long-first-doc-paragraph

[Miguel Ojeda]

On Sun, Feb 16, 2025 at 2:30 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> I don't think that this one is bad. Of course it would be nice if only
> the rendered form would be considered, but there might also be people
> reading it directly in the code, so it could actually be a good idea to
> also keep the raw text short.
>
> In the case of long links, one can easily work around it by just not
> using an inline link. (so having [text with link] and then after the
> paragraph `[text with link]: link`.)

Agreed, I think this one should not be an issue and could in fact be a
good thing.

> This one just seems like a nice-to-have, but not a deal breaker.

Yeah.

> IMO we can enable the lint (since the instance fixed in this patch is
> the only error I get with that lint) and just see how it behaves.

Yeah, I am happy trying that one. If you want to submit the patch
formally, then we can see how it goes.

In general, though, I think for "nursery" lints we may want to be
extra careful, especially if it may look like they will have potential
behavior changes that end up triggering even more changes later until
they settle (e.g. suddenly they change the threshold length or other
heuristics). For now we don't have that much code, so it is OK.

I think eventually we may end up putting some extra lints into a
separate group that can be enabled opt-in, and then promote them as
time passes.

Cheers,
Miguel