[PATCH 0/2] checkpatch.pl: add warning for // comments on private Rust items

[PATCH 0/2] checkpatch.pl: add warning for // comments on private Rust items

[Arnaud Lecomte]

Hi,

Background
----------

The Rust-for-Linux project currently lacks enforcement of documentation for private Rust items, 
as highlighted in https://github.com/Rust-for-Linux/linux/issues/1157.
While rustc already lints missing documentation for public items, private items remain unchecked.
This patch series aims to close that gap by ensuring proper documentation practices
for private Rust items in the kernel.
As underlined in this issue:
https://github.com/Rust-for-Linux/linux/issues/1157, the purposes of
this patch serie is to ensure the proper documentation of private rust
items. Public items missing documentation are already linted by rustc. 

The actual solution comes in several parts
------------------------------------------

 1) Patch 1 : Implements detection logic to emit warnings for improperly
 documented private Rust items (e.g., // comments instead of ///).
 2) Patch 2 : Adds an auto-fix mechanism via the --fix option to help
 developers correct documentation issues.

Results
--------------------

The following implementation has been tested against this input file:
// SPDX-License-Identifier: GPL-2.0

// Simple comment - should not trigger

// Returns a reference to the underlying [`cpufreq::Table`]. - should trigger

fn table(&self) -> &cpufreq::Table {
    // SAFETY: The `ptr` is guaranteed by the C code to be valid. - should not trigger
    unsafe { cpufreq::Table::from_raw(self.ptr) }
}

// Improper doc comment for a private function. - should trigger
fn test() -> u32 {
    42
}

/// Proper doc comment for a private function. - should not trigger
fn proper_doc() -> u32 {
    42
}

// TODO: implement more logic - should not trigger
fn todo_marker() -> bool {
    true
}

// Just a regular comment not followed by code - should not trigger

pub fn public_function() {
    // Public function - should not trigger
}

// Test - should trigger

struct Point2D {
  pub x: f32,
  pub y: f32
}

// Test - should not trigger

pub struct Point2D {
  pub x: f32,
  pub y: f32
}

// Test - should not trigger

pub struct Point2D {
  pub x: f32,
  pub y: f32
}

mod test_module {
    // Module inner comment - should not trigger
}

// Comment before macro - should not trigger
macro_rules! my_macro {
    // Comment inside macro - should not trigger
    () => {};
}

// Comment before unsafe block - should not trigger
unsafe {
    // Comment inside unsafe block - should not trigger
    let x = 5;
}

// Comment with unsafe word - should trigger
fn with_unsafe_keyword() {
    println!("test");
}

// Comment with code example: - should trigger
// let x = 5; - should trigger
fn with_code_example() {
    println!("test");
}

// NOTE: important consideration - should not trigger
fn note_marker() -> bool {
    true
}

// Comment with code example: - should trigger
/// let x = 5; - should not trigger
fn with_mixed_comments() {
    println!("test");
}

which led to this output:
WARNING: Consider using `///` for private item documentation (line 5)

+// Returns a reference to the underlying [`cpufreq::Table`]. - should trigger
WARNING: Consider using `///` for private item documentation (line 12)

+// Improper doc comment for a private function. - should trigger
WARNING: Consider using `///` for private item documentation (line 33)

+// Test - should trigger
WARNING: Consider using `///` for private item documentation (line 74)

+// Comment with unsafe word - should trigger
WARNING: Consider using `///` for private item documentation (line 79)

+// Comment with code example: - should trigger
WARNING: Consider using `///` for private item documentation (line 80)

+// let x = 5; - should trigger
WARNING: Consider using `///` for private item documentation (line 91)

+// Comment with code example: - should trigger
total: 0 errors, 7 warnings, 95 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

test.rs has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

Link: https://github.com/Rust-for-Linux/linux/issues/1157
Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Arnaud Lecomte <contact@arnaud-lcm.com>
---
Arnaud Lecomte (2):
      checkpatch.pl: warn about // comments on private Rust items
      checkpatch.pl: add `--fix` support for `//` comments on private Rust items

 scripts/checkpatch.pl | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)
---
base-commit: 250b341caf5e0e099dcf789be652178ca57a68cd
change-id: 20250419-checkpatch-rust-private-item-comment-bb3a5281e5da

Best regards,
-- 
Arnaud Lecomte <contact@arnaud-lcm.com>

[PATCH 0/2] checkpatch.pl: add warning for // comments on private Rust items

[Arnaud Lecomte]

Hi,

Background
----------

The Rust-for-Linux project currently lacks enforcement of documentation for private Rust items, 
as highlighted in https://github.com/Rust-for-Linux/linux/issues/1157.
While rustc already lints missing documentation for public items, private items remain unchecked.
This patch series aims to close that gap by ensuring proper documentation practices
for private Rust items in the kernel.
As underlined in this issue:
https://github.com/Rust-for-Linux/linux/issues/1157, the purposes of
this patch serie is to ensure the proper documentation of private rust
items. Public items missing documentation are already linted by rustc. 

The actual solution comes in several parts
------------------------------------------

 1) Patch 1 : Implements detection logic to emit warnings for improperly
 documented private Rust items (e.g., // comments instead of ///).
 2) Patch 2 : Adds an auto-fix mechanism via the --fix option to help
 developers correct documentation issues.

Results
--------------------

The following implementation has been tested against this input file:
// SPDX-License-Identifier: GPL-2.0

// Simple comment - should not trigger

// Returns a reference to the underlying [`cpufreq::Table`]. - should trigger

fn table(&self) -> &cpufreq::Table {
    // SAFETY: The `ptr` is guaranteed by the C code to be valid. - should not trigger
    unsafe { cpufreq::Table::from_raw(self.ptr) }
}

// Improper doc comment for a private function. - should trigger
fn test() -> u32 {
    42
}

/// Proper doc comment for a private function. - should not trigger
fn proper_doc() -> u32 {
    42
}

// TODO: implement more logic - should not trigger
fn todo_marker() -> bool {
    true
}

// Just a regular comment not followed by code - should not trigger

pub fn public_function() {
    // Public function - should not trigger
}

// Test - should trigger

struct Point2D {
  pub x: f32,
  pub y: f32
}

// Test - should not trigger

pub struct Point2D {
  pub x: f32,
  pub y: f32
}

// Test - should not trigger

pub struct Point2D {
  pub x: f32,
  pub y: f32
}

mod test_module {
    // Module inner comment - should not trigger
}

// Comment before macro - should not trigger
macro_rules! my_macro {
    // Comment inside macro - should not trigger
    () => {};
}

// Comment before unsafe block - should not trigger
unsafe {
    // Comment inside unsafe block - should not trigger
    let x = 5;
}

// Comment with unsafe word - should trigger
fn with_unsafe_keyword() {
    println!("test");
}

// Comment with code example: - should trigger
// let x = 5; - should trigger
fn with_code_example() {
    println!("test");
}

// NOTE: important consideration - should not trigger
fn note_marker() -> bool {
    true
}

// Comment with code example: - should trigger
/// let x = 5; - should not trigger
fn with_mixed_comments() {
    println!("test");
}

which led to this output:
WARNING: Consider using `///` for private item documentation (line 5)

+// Returns a reference to the underlying [`cpufreq::Table`]. - should trigger
WARNING: Consider using `///` for private item documentation (line 12)

+// Improper doc comment for a private function. - should trigger
WARNING: Consider using `///` for private item documentation (line 33)

+// Test - should trigger
WARNING: Consider using `///` for private item documentation (line 74)

+// Comment with unsafe word - should trigger
WARNING: Consider using `///` for private item documentation (line 79)

+// Comment with code example: - should trigger
WARNING: Consider using `///` for private item documentation (line 80)

+// let x = 5; - should trigger
WARNING: Consider using `///` for private item documentation (line 91)

+// Comment with code example: - should trigger
total: 0 errors, 7 warnings, 95 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

test.rs has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

Link: https://github.com/Rust-for-Linux/linux/issues/1157
Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Arnaud Lecomte <contact@arnaud-lcm.com>
---
Arnaud Lecomte (2):
      checkpatch.pl: warn about // comments on private Rust items
      checkpatch.pl: add `--fix` support for `//` comments on private Rust items

 scripts/checkpatch.pl | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)
---
base-commit: 250b341caf5e0e099dcf789be652178ca57a68cd
change-id: 20250419-checkpatch-rust-private-item-comment-bb3a5281e5da

Best regards,
-- 
Arnaud Lecomte <contact@arnaud-lcm.com>

Re: [PATCH 0/2] checkpatch.pl: add warning for // comments on private Rust items

[Benno Lossin]

On Sat Apr 19, 2025 at 10:24 PM CEST, Arnaud Lecomte wrote:
> Hi,
>
> Background
> ----------
>
> The Rust-for-Linux project currently lacks enforcement of documentation for private Rust items,
> as highlighted in https://github.com/Rust-for-Linux/linux/issues/1157.
> While rustc already lints missing documentation for public items, private items remain unchecked.
> This patch series aims to close that gap by ensuring proper documentation practices
> for private Rust items in the kernel.
> As underlined in this issue:
> https://github.com/Rust-for-Linux/linux/issues/1157, the purposes of
> this patch serie is to ensure the proper documentation of private rust
> items. Public items missing documentation are already linted by rustc.
>
> The actual solution comes in several parts
> ------------------------------------------
>
>  1) Patch 1 : Implements detection logic to emit warnings for improperly
>  documented private Rust items (e.g., // comments instead of ///).
>  2) Patch 2 : Adds an auto-fix mechanism via the --fix option to help
>  developers correct documentation issues.
>
> Results
> --------------------

Thanks for this helpful example, I'd recommend you to run your modified
version on real patches from the list and/or on already existing commits
in the kernel.

>
> The following implementation has been tested against this input file:
> // SPDX-License-Identifier: GPL-2.0

[...]

> pub struct Point2D {
>   pub x: f32,
>   pub y: f32
> }
>
> mod test_module {
>     // Module inner comment - should not trigger
> }
>
> // Comment before macro - should not trigger
> macro_rules! my_macro {

I think we should also trigger this for macros. All macros are private
by default and only made public with the `#[macro_export]` annotation.

>     // Comment inside macro - should not trigger
>     () => {};
> }
>
> // Comment before unsafe block - should not trigger
> unsafe {
>     // Comment inside unsafe block - should not trigger
>     let x = 5;
> }
>
> // Comment with unsafe word - should trigger
> fn with_unsafe_keyword() {
>     println!("test");
> }
>
> // Comment with code example: - should trigger
> // let x = 5; - should trigger

Code examples are usually wrapped in '```', so they are another
indicator that it probably should be a doc-comment.

> fn with_code_example() {
>     println!("test");
> }
>
> // NOTE: important consideration - should not trigger
> fn note_marker() -> bool {
>     true
> }
>
> // Comment with code example: - should trigger

I'm not 100% convinced that this should trigger. We have several cases
of the following (in some cases the function is public, I don't
remember if we have a private case):

    /// Normal function docs...
    // We probably should refactor XYZ.
    fn foo() {}

There it should not trigger.

---
Cheers,
Benno

> /// let x = 5; - should not trigger
> fn with_mixed_comments() {
>     println!("test");
> }