[PATCH v6 0/4] Clean up rustdocs and add new checkpatch checks

[PATCH v6 0/4] Clean up rustdocs and add new checkpatch checks

[Hridesh MG]

Clean up the rustdocs as suggested by two issues[1][2]. The first issue
is to convert all rustdoc headers to plural in order to maintain
consistency and avoid situations where further additions to the doc
could render the grammar incorrect. The second issue aims to remove
consecutive empty rustdoc lines.

Also add two new checks (patches 2 and 4) to checkpatch to detect any
future occurrences of the above two issues and optionally fix them using
--fix.

This patchset is a merger of two different patchsets [3] and [4] with a
few improvements made on top of them which are listed on their
individual changelogs.

[1]: https://github.com/Rust-for-Linux/linux/issues/1110
[2]: https://github.com/Rust-for-Linux/linux/issues/1109

[3]: https://lore.kernel.org/rust-for-linux/20241002022749.390836-1-paddymills@proton.me/
[4]: https://lore.kernel.org/rust-for-linux/dbb63b5698aa507bbe3dec54b4458a3f151899d3.1727606659.git.hridesh699@gmail.com/

Hridesh MG (2):
  rust: kernel: clean up empty `///` lines
  checkpatch: warn on empty rust doc comments

Patrick Miller (2):
  rust: make section names plural
  checkpatch: warn on known non-plural rust doc headers

 rust/kernel/block/mq/request.rs |  1 -
 rust/kernel/init.rs             |  2 +-
 rust/kernel/list/arc.rs         |  2 +-
 rust/kernel/rbtree.rs           |  1 -
 rust/kernel/sync/arc.rs         |  2 +-
 rust/macros/lib.rs              |  2 +-
 scripts/checkpatch.pl           | 22 ++++++++++++++++++++++
 7 files changed, 26 insertions(+), 6 deletions(-)

-- 
2.46.1

[PATCH v6 1/4] rust: make section names plural

[Hridesh MG]

From: Patrick Miller <paddymills@proton.me>

Update existing rust documentation section headers to use plural names.

Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://github.com/Rust-for-Linux/linux/issues/1110
Signed-off-by: Patrick Miller <paddymills@proton.me>
Signed-off-by: Hridesh MG <hridesh699@gmail.com>
---
 rust/kernel/init.rs     | 2 +-
 rust/kernel/list/arc.rs | 2 +-
 rust/kernel/sync/arc.rs | 2 +-
 rust/macros/lib.rs      | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index 25057cbed40b..2036846fc579 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -744,7 +744,7 @@ macro_rules! try_init {
 /// Asserts that a field on a struct using `#[pin_data]` is marked with `#[pin]` ie. that it is
 /// structurally pinned.
 ///
-/// # Example
+/// # Examples
 ///
 /// This will succeed:
 /// ```
diff --git a/rust/kernel/list/arc.rs b/rust/kernel/list/arc.rs
index 3483d8c232c4..e85d026ace4b 100644
--- a/rust/kernel/list/arc.rs
+++ b/rust/kernel/list/arc.rs
@@ -461,7 +461,7 @@ impl<T, U, const ID: u64> core::ops::DispatchFromDyn<ListArc<U, ID>> for ListArc
 
 /// A utility for tracking whether a [`ListArc`] exists using an atomic.
 ///
-/// # Invariant
+/// # Invariants
 ///
 /// If the boolean is `false`, then there is no [`ListArc`] for this value.
 #[repr(transparent)]
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 9325cc5a16a4..d814032e0ccb 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -433,7 +433,7 @@ fn from(item: Pin<UniqueArc<T>>) -> Self {
 /// There are no mutable references to the underlying [`Arc`], and it remains valid for the
 /// lifetime of the [`ArcBorrow`] instance.
 ///
-/// # Example
+/// # Examples
 ///
 /// ```
 /// use kernel::sync::{Arc, ArcBorrow};
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index a626b1145e5c..3879e1162866 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -307,7 +307,7 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
 /// literals (lifetimes and documentation strings are not supported). There is a difference in
 /// supported modifiers as well.
 ///
-/// # Example
+/// # Examples
 ///
 /// ```ignore
 /// use kernel::macro::paste;
-- 
2.46.1

[PATCH v6 2/4] checkpatch: warn on known non-plural rust doc headers

[Hridesh MG]

From: Patrick Miller <paddymills@proton.me>

Add a check for documentation in rust files. Warn if certain known
documentation headers are not plural.

The rust maintainers prefer document headers to be plural. This is to
enforce consistency as well as to protect against errors when additions
are made. For instance, if a header says "Example" for a single example,
adding a second example may cause confusion. The maintainers wish to
avoid reminding users to update their documentation for such cases.

Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://github.com/Rust-for-Linux/linux/issues/1110
Signed-off-by: Patrick Miller <paddymills@proton.me>
Signed-off-by: Hridesh MG <hridesh699@gmail.com>
---
v1: https://lore.kernel.org/rust-for-linux/2024090628-bankable-refusal-5f20@gregkh/T/#t
v2: https://lore.kernel.org/rust-for-linux/92be0b48-cde9-4241-8ef9-7fe4d7c42466@proton.me/T/#t
  - fixed whitespace that was formatted due to editor settings 
v3: https://lore.kernel.org/rust-for-linux/da34f89c-f94c-43aa-946c-57fec3597974@proton.me/T/#t
  - move && to previous line and remove whitespace in WARN per Joe Perches
  - reformat following C coding style
v4: https://lore.kernel.org/rust-for-linux/20240914181618.837227-2-paddymills@proton.me/
  - add @fix option (credit: Joe Perches)
  - add Error to list of checked section headers
  - make check for rust file its own if statement because more rust
      checks are planned
v5: https://lore.kernel.org/rust-for-linux/20241002022749.390836-2-paddymills@proton.me/
  - merged Hridesh MG's patch[1] to check against consecutive empty rustdoc comments
  - revised Hridesh MG's patch to match against $prevrawline being new
      or existing
  - added fix to Hridesh MG's patch
v6:
  - undid the merging of Hridesh MG's patch
  - revised opening comment to be more explicit per Miguel Ojeda
  - The --fix option was throwing an uninitialized variable error, fixed
    it and made it automatically convert the headers to plural.

[1]: https://lore.kernel.org/rust-for-linux/bf6544faba2b53ce901b2e031f3d944babcc7018.1727606659.git.hridesh699@gmail.com/
---
 scripts/checkpatch.pl | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4427572b2477..c390a9926cd5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3900,6 +3900,19 @@ sub process {
 			     "Avoid using '.L' prefixed local symbol names for denoting a range of code via 'SYM_*_START/END' annotations; see Documentation/core-api/asm-annotations.rst\n" . $herecurr);
 		}
 
+# checks that only apply to Rust files
+		if ($realfile =~ /\.rs$/) {
+			# check that document section headers are plural
+			if ($rawline =~ /^\+\s*\/\/\/\s+#+\s+(Example|Error|Guarantee|Invariant|Panic)\s*$/i) {
+				if (WARN("RUST_DOC_HEADER",
+				         "Rust doc section names should be plural\n" . $herecurr) &&
+				    $fix) {
+					my $header = $1;
+					$fixed[$fixlinenr] =~ s/\b$header\b/ucfirst(lc($header)) . 's'/e;
+				}
+			}
+		}
+
 # check we are in a valid source file C or perl if not then ignore this hunk
 		next if ($realfile !~ /\.(h|c|pl|dtsi|dts)$/);
 
-- 
2.46.1

[PATCH v6 3/4] rust: kernel: clean up empty `///` lines

[Hridesh MG]

Remove unnecessary empty `///` lines in the rust docs.

Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://github.com/Rust-for-Linux/linux/issues/1109
Reviewed-by: Trevor Gross <tmgross@umich.edu>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Hridesh MG <hridesh699@gmail.com>
---
Changelog:
v1->v2
- Fixed typo in commit title and description
- Removed backslashes in kernel::block::mq::Request
- Link to v1: https://lore.kernel.org/rust-for-linux/20240909161749.147076-1-hridesh699@gmail.com/

v2->v3
- Fixed From: tag in patch header
- Link to v2: https://lore.kernel.org/rust-for-linux/aa1b4059dfac001945745db02b6f6d9db2e5d1cb.1726072795.git.hridesh699@gmail.com/

No changes from v3->v6
---
 rust/kernel/block/mq/request.rs | 1 -
 rust/kernel/rbtree.rs           | 1 -
 2 files changed, 2 deletions(-)

diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index a0e22827f3f4..313334b1bf18 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -30,7 +30,6 @@
 /// D) Request is owned by driver with more than one `ARef` in existence
 ///    (refcount > 2)
 ///
-///
 /// We need to track A and B to ensure we fail tag to request conversions for
 /// requests that are not owned by the driver.
 ///
diff --git a/rust/kernel/rbtree.rs b/rust/kernel/rbtree.rs
index d03e4aa1f481..c0730c1f9707 100644
--- a/rust/kernel/rbtree.rs
+++ b/rust/kernel/rbtree.rs
@@ -1034,7 +1034,6 @@ fn next(&mut self) -> Option<Self::Item> {
 
 /// A memory reservation for a red-black tree node.
 ///
-///
 /// It contains the memory needed to hold a node that can be inserted into a red-black tree. One
 /// can be obtained by directly allocating it ([`RBTreeNodeReservation::new`]).
 pub struct RBTreeNodeReservation<K, V> {
-- 
2.46.1

[PATCH v6 4/4] checkpatch: warn on empty rust doc comments

[Hridesh MG]

Add a check to warn if there are consecutive empty `///` lines in rust
files.

Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://github.com/Rust-for-Linux/linux/issues/1109
Signed-off-by: Hridesh MG <hridesh699@gmail.com>
---
v2: https://lore.kernel.org/rust-for-linux/7877d23adba22e2f89a61efc129ecf1b0627510b.1726072795.git.hridesh699@gmail.com/
  - new patch
v3: https://lore.kernel.org/rust-for-linux/bf6544faba2b53ce901b2e031f3d944babcc7018.1727606659.git.hridesh699@gmail.com/
  - no changes
v6:
  - added --fix support which deletes the unnecessary line
---
 scripts/checkpatch.pl | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c390a9926cd5..baadc345bf87 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3911,6 +3911,15 @@ sub process {
 					$fixed[$fixlinenr] =~ s/\b$header\b/ucfirst(lc($header)) . 's'/e;
 				}
 			}
+
+			# check for consecutive empty rustdoc lines
+			if ($rawline =~ /^\+\s*\/\/\/$/ && $prevrawline =~ /^\+?\s*\/\/\/$/) {
+				if (WARN("RUST_DOC_EMPTY",
+				         "avoid using consecutive empty rustdoc comments\n" . $herecurr) &&
+				    $fix) {
+					fix_delete_line($fixlinenr, $rawline);
+				}
+			}
 		}
 
 # check we are in a valid source file C or perl if not then ignore this hunk
-- 
2.46.1

Re: [PATCH v6 4/4] checkpatch: warn on empty rust doc comments

[Hridesh MG]

Just dropping a gentle ping—please let me know if there’s anything
additional I can do to help move this forward.

Re: [PATCH v6 4/4] checkpatch: warn on empty rust doc comments

[Miguel Ojeda]

On Tue, Oct 29, 2024 at 7:12 AM Hridesh MG <hridesh699@gmail.com> wrote:
>
> Just dropping a gentle ping—please let me know if there’s anything
> additional I can do to help move this forward.

I think Joe wanted to take a look (?), but maybe I misinterpreted his message:

    https://lore.kernel.org/rust-for-linux/9a8ed16ffc65d587ed2ff2d1e95bd61fdca5cc90.camel@perches.com/

The Rust changes (patch #1 and #3) look fine, of course. If needed, I
can take those independently to reduce the series. Otherwise, if
And/Joe want to carry them:

Acked-by: Miguel Ojeda <ojeda@kernel.org>

(Ideally with a slightly better commit message, e.g. one idea is
showing the warning without cleaning those, but up to him to reword or
not if he picks them up)

Thanks!

Cheers,
Miguel

Re: [PATCH v6 4/4] checkpatch: warn on empty rust doc comments

[Hridesh MG]

Apologies for the repeated pings, but I’d greatly appreciate it if
someone could help move this forward

Re: [PATCH v6 3/4] rust: kernel: clean up empty `///` lines

[Miguel Ojeda]

On Tue, Oct 15, 2024 at 5:52 PM Hridesh MG <hridesh699@gmail.com> wrote:
>
> Remove unnecessary empty `///` lines in the rust docs.
>
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Link: https://github.com/Rust-for-Linux/linux/issues/1109
> Reviewed-by: Trevor Gross <tmgross@umich.edu>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Hridesh MG <hridesh699@gmail.com>

Andreas: this is trivial and perhaps you prefer that I pick this sort
of thing automatically, but just in case, could you please Acked-by
the block part? Then I will pick it and hopefully the authors can
restart the discussion on the `checkpatch.pl` side of this series.
Thanks!

Cheers,
Miguel