Message-ID: <20250420184700.47144-1-contact@arnaud-lcm.com>
Date: Sun, 20 Apr 2025 20:47:00 +0200
From: Arnaud Lecomte <contact@...aud-lcm.com>
To: Andy Whitcroft <apw@...onical.com>,
	Joe Perches <joe@...ches.com>,
	Dwaipayan Ray <dwaipayanray1@...il.com>,
	Lukas Bulwahn <lukas.bulwahn@...il.com>,
	Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Boqun Feng <boqun.feng@...il.com>,
	Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>,
	Trevor Gross <tmgross@...ch.edu>,
	Danilo Krummrich <dakr@...nel.org>,
	Nathan Chancellor <nathan@...nel.org>,
	Nick Desaulniers <nick.desaulniers+lkml@...il.com>,
	Bill Wendling <morbo@...gle.com>,
	Justin Stitt <justinstitt@...gle.com>
Cc: linux-kernel@...r.kernel.org,
	rust-for-linux@...r.kernel.org,
	llvm@...ts.linux.dev,
	contact@...aud-lcm.com,
	skhan@...uxfoundation.org
Subject: [PATCH v2 0/2] checkpatch.pl: Add warning for // comments on private
 Rust items

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.

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 real patches:
 - https://lore.kernel.org/rust-for-linux/dc63bdc4bff8f084714e2c8ff30e4c0d435e85b7.camel@redhat.com/T/#t
 - https://lore.kernel.org/rust-for-linux/20250418-ptr-as-ptr-v10-0-3d63d27907aa@gmail.com/T/#t
 - https://lore.kernel.org/rust-for-linux/20250420-nova-frts-v1-1-ecd1cca23963@nvidia.com/T/#u	
and this file:
// SPDX-License-Identifier: GPL-2.0

// Simple comment - should not trigger

// Returns a reference to the underlying [`cpufreq::Table`]. - should trigger
#[inline]
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
#[repr(C)]
#[derive(Clone, Copy, Debug, PartialEq, PartialOrd)]
struct Point2D {
  pub x: f32,
  pub y: f32
}

// Test - should not trigger
#[repr(C)]
#[derive(Clone, Copy, Debug, PartialEq, PartialOrd)]
pub struct Point2D {
  pub x: f32,
  pub y: f32
}

// Test - should not trigger
#[repr(C)]
#[derive(Clone, Copy, Debug, PartialEq, PartialOrd)]
pub struct Point2D {
  pub x: f32,
  pub y: f32
}

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

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

// Comment before public macro - should not trigger
#[macro_export]
macro_rules! my_public_macro {

}

// 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 not 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)
#7: FILE: ./test.rs:7:
+// Returns a reference to the underlying [`cpufreq::Table`]. - should trigger
WARNING: Consider using `///` for private item documentation (line 12)
#13: FILE: ./test.rs:13:
+// Improper doc comment for a private function. - should trigger
WARNING: Consider using `///` for private item documentation (line 33)
#36: FILE: ./test.rs:36:
+// Test - should trigger
WARNING: Consider using `///` for private item documentation (line 61)
#62: FILE: ./test.rs:62:
+// Comment before macro - should trigger
WARNING: Consider using `///` for private item documentation (line 80)
#81: FILE: ./test.rs:81:
+// Comment with unsafe word - should trigger
WARNING: Consider using `///` for private item documentation (line 85)
#87: FILE: ./test.rs:87:
+// Comment with code example: - should trigger
WARNING: Consider using `///` for private item documentation (line 86)
#87: FILE: ./test.rs:87:
+// let x = 5; - should trigger
total: 0 errors, 7 warnings, 101 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.

To: Andy Whitcroft <apw@...onical.com>
To: Joe Perches <joe@...ches.com>
To: Dwaipayan Ray <dwaipayanray1@...il.com>
To: Lukas Bulwahn <lukas.bulwahn@...il.com>
To: Miguel Ojeda <ojeda@...nel.org>
To: Alex Gaynor <alex.gaynor@...il.com>
To: Boqun Feng <boqun.feng@...il.com>
To: Gary Guo <gary@...yguo.net>
To: Björn Roy Baron <bjorn3_gh@...tonmail.com>
To: Benno Lossin <benno.lossin@...ton.me>
To: Andreas Hindborg <a.hindborg@...nel.org>
To: Alice Ryhl <aliceryhl@...gle.com>
To: Trevor Gross <tmgross@...ch.edu>
To: Danilo Krummrich <dakr@...nel.org>
To: Nathan Chancellor <nathan@...nel.org>
To: Nick Desaulniers <nick.desaulniers+lkml@...il.com>
To: Bill Wendling <morbo@...gle.com>
To: Justin Stitt <justinstitt@...gle.com>
Cc: linux-kernel@...r.kernel.org
Cc: rust-for-linux@...r.kernel.org
Cc: llvm@...ts.linux.dev
Cc: contact@...aud-lcm.com
Cc: skhan@...uxfoundation.org
Signed-off-by: Arnaud Lecomte <contact@...aud-lcm.com>
---
Changes in v2:
- Tested against real patches
- Link to v1: https://lore.kernel.org/all/20250419-checkpatch-rust-private-item-comment-v1-0-0f8bc109bd5a@arnaud-lcm.com/ 
---
-- 
2.43.0

Powered by blists - more mailing lists