| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250729200158.35050-1-contact@arnaud-lcm.com>
Date: Tue, 29 Jul 2025 21:01:58 +0100
From: Arnaud Lecomte <contact@...aud-lcm.com>
To: joe@...ches.com
Cc: a.hindborg@...nel.org,
alex.gaynor@...il.com,
aliceryhl@...gle.com,
apw@...onical.com,
bjorn3_gh@...tonmail.com,
boqun.feng@...il.com,
dakr@...nel.org,
dwaipayanray1@...il.com,
gary@...yguo.net,
linux-kernel@...r.kernel.org,
lossin@...nel.org,
lukas.bulwahn@...il.com,
ojeda@...nel.org,
rust-for-linux@...r.kernel.org,
tmgross@...ch.edu,
Arnaud Lecomte <contact@...aud-lcm.com>,
Miguel Ojeda <ojeda@...nel.orgs>
Subject: [PATCH 2/3] checkpatch.pl: warn about // comments on private Rust
items
Add a new checkpatch.pl warning to detect //-style comments above
private Rust items that were likely intended to be doc comments (///).
This helps catch documentation that won't appear in rustdoc output.
The detection uses multiple heuristics to identify likely doc comments:
- Comments containing markdown
- Comments starting with an imperative tone
- Comments with mention of params
- Comments with code block
- Comments with keywords: Returns, ...
- Block comments longer than 3 lines.
Comments are only flagged if they:
- Appear above private items
- Don't contain obvious non-doc patterns (TODO, FIXME)
- Score sufficiently on heuristics
The warning triggered then suggests to convert flagged comments to
doc comments.
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
- https://lore.kernel.org/all/20250502094537.231725-5-fujita.tomonori@gmail.com
and against a test patch containing:
// Returns a value
fn private_function() {}
// Computes the result
struct PrivateStruct;
// Arguments:
// - x: input value
// - y: other input
const PRIVATE_CONST: u32 = 42;
// Returns [`SomeType`] with the result
//
// # Examples
// ```
// let x = example();
// ```
fn with_markdown() {}
// Arguments:
// - `param1`: First parameter
// - `param2`: Second parameter
fn with_parameters(param1: u32, param2: String) {}
// 値を返します
fn non_english_doc() {}
// Returns something
// TODO: Change this later
fn mixed_comments() {}
// Internal helper
fn indented_comment() {}
// From kernel example
// Returns a reference to the underlying [`cpufreq::Table`].
fn table(&self) -> &cpufreq::Table {
// SAFETY: The `ptr` is guaranteed by the C code to be valid.
unsafe { cpufreq::Table::from_raw(self.ptr) }
}
// Trait implementation docs
impl SomeTrait for SomeType {
// Performs the operation
fn method(&self) {}
}
// Cases that should NOT trigger warnings
/// Proper documentation
fn documented_function() {}
// TODO: Implement this later
fn todo_function() {}
// SAFETY: This is unsafe because...
unsafe fn safety_comment() {}
// ------
// Not a doc comment
fn visual_separator() {}
pub fn public_function() {} // Public item (handled by rustc)
// some_code(); let x = 42;
fn looks_like_code() {}
// !!! IMPORTANT !!!
fn punctuation_only() {}
// This is just
// a regular
// multi-line comment
fn multi_line_regular() {}
// [DEBUG] Starting process
fn log_message() {}
// <html>test</html>
fn html_comment() {}
// $ echo command
fn shell_command() {}
which resulted into:
WARNING: Possible documentation comment using `//` instead of `///`.
Consider using `///` for documentation comments on private items.
+// Arguments:
+// - x: input value
+// - y: other input
WARNING: Possible documentation comment using `//` instead of `///`.
Consider using `///` for documentation comments on private items.
+// Arguments:
+// - `param1`: First parameter
+// - `param2`: Second parameter
WARNING: Possible documentation comment using `//` instead of `///`.
Consider using `///` for documentation comments on private items.
+// From kernel example
+// Returns a reference to the underlying [`cpufreq::Table`].
Conclusion
--------------------
I think We achieved a balanced solution that effectively enforces
proper documentation while minimizing false positives.
Closes: https://github.com/Rust-for-Linux/linux/issues/1157
Suggested-by: Miguel Ojeda <ojeda@...nel.orgs>
Signed-off-by: Arnaud Lecomte <contact@...aud-lcm.com>
---
scripts/checkpatch.pl | 111 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 111 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 664f7b7a622c..ac403d270eb4 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2007,6 +2007,97 @@ sub ctx_locate_comment {
chomp($current_comment);
return($current_comment);
}
+
+sub ctx_detect_missing_doc_comment_behind_rust_private_item {
+ my ($linenr) = @_;
+ my $start_line = $linenr - 1;
+ my $idx = $start_line;
+
+ # Skip public items (handled by rustc)
+ return unless $rawlines[$start_line] !~ /^\+?\s*pub(\s|\([^)]*\)\s)/;
+
+ # Check if it's an empty line
+ return unless $rawlines[$start_line] !~ /^\s*\+?\s*$/;
+
+ # Check if it's a comment
+ return unless $rawlines[$start_line] !~ /^\+?\s*\/\/\/?\/?.*$/;
+
+ # Check if it is a private macro
+ if ($rawlines[$start_line] =~ /^\+?\s*macro_rules!\s+([a-zA-Z_][a-zA-Z0-9_]*|#?[a-zA-Z_][a-zA-Z0-9_]*)\s*(?:\{|$)/ && $idx > 0) {
+ return unless $rawlines[$idx -1] !~ /^\+?\s*#\s*\[\s*macro_export\s*\]\s*$/;
+ } else {
+ # Check if it's a Rust item
+ return unless $rawlines[$start_line] =~ /^\+?\s*\b(fn|struct|const|enum|trait|type|mod|impl|use)\b/;
+ }
+
+ # Walk backwards through non-empty lines
+ $idx--;
+ my @comment_lines;
+ while ($idx >= 0 && $rawlines[$idx] !~ /^\s*\+?\s*$/) {
+ # Stop if a doc comment is found
+ last if $rawlines[$idx] =~ m@^\+?\s*///@;
+ # Clean the line first (remove diff markers, trim spaces)
+ my $clean_line = $rawlines[$idx];
+ $clean_line =~ s/^\+?\s*//;
+ $clean_line =~ s/\s*$//;
+ if($clean_line =~ m@^\+?\s*//@) { # Only add comments
+ unshift @comment_lines, $clean_line;
+ }
+ $idx--;
+ }
+
+ # No comments to check
+ return unless @comment_lines;
+
+ # Skip obvious non-doc comments
+ return if grep { /^\s*\/\/\s*(?:TODO|FIXME|XXX|NOTE|HACK|SAFETY):?/i } @comment_lines;
+ return if grep { /^\s*\/\/\s*[[:punct:]]{2,}/ } @comment_lines;
+
+ # Heuristic: Check for documentation keywords
+ my $keywords = qr/Returns|Arguments|Examples?|Panics|Safety|Note|Errors|See\s+Also/i;
+ my $has_keywords = grep { /\/\/\s*$keywords/ } @comment_lines;
+
+ # Heuristic: Markdown syntax
+ my $markdown = qr{
+ `[^`]+` # Inline code
+ | \*\*[^*]+\*\* # Bold
+ | \b_[^_\s][^_]*_\b # Italic
+ | \#[^#\s] # Headings like # Title
+ | ^\s*```[\w]*\s*$ # Code block marker line (allow optional language)
+ }x;
+
+ my $has_markdown = grep { /$markdown/ } @comment_lines;
+
+ # Heuristic: Parameter mentions
+ my $item_line = $rawlines[$start_line];
+ my @params = ($item_line =~ /(\b\w+)\s*:\s*\w+/g); # Extract param names
+ my $param_mentions = 0;
+ if (@params) {
+ $param_mentions = grep { my $line = $_; grep { $line =~ /\b$_\b/ } @params } @comment_lines;
+ }
+
+ # Heuristic: Start with imperative tones
+ my $first_line = $comment_lines[0];
+ my $imperative_tone = $first_line =~ /^\s*\/\/\s*(Returns?|Computes?|Checks?|Converts?|Creates?)\b/i;
+
+ # Heuristic: Code block
+ my $has_code_block = grep { /```|\buse\s+\w+::|^\s*\/\/\s*\w+\(.*\)/ } @comment_lines;
+ # Combine heuristics
+ my $score = 0;
+ $score += 1 if $has_keywords;
+ $score += 2 if $has_markdown;
+ $score += 1 if $param_mentions;
+ $score += 1 if $imperative_tone;
+ $score += 2 if scalar(@comment_lines) >= 3;
+ $score += 2 if $has_code_block;
+
+ # Trigger if score meets threshold
+ return unless $score >= 3;
+
+ return ($idx+1, $start_line);
+}
+
+
sub ctx_has_comment {
my ($first_line, $end_line) = @_;
my $cmt = ctx_locate_comment($first_line, $end_line);
@@ -2970,6 +3061,26 @@ sub process {
}
}
+# check for incorrect use of `//` instead of `\\\` for doc comments in Rust
+ if ($perl_version_ok && $realfile =~ /\.rs$/) {
+ my ($start_l, $end_l) = ctx_detect_missing_doc_comment_behind_rust_private_item($linenr);
+ if (defined($start_l) && defined($end_l)) {
+ my @comment_lines;
+ for (my $i = $start_l; $i <= $end_l; $i++) {
+ next unless $rawlines[$i - 1] =~ m@^\+//([^/].*)$@;
+ push @comment_lines, $rawlines[$i - 1];
+ }
+ my $formatted_comments = join("", map {
+ s/\n$//;
+ "$_\n"
+ } @comment_lines);
+ WARN("MISSING_RUST_DOC",
+ "Possible documentation comment using `//` instead of `///`.\n" .
+ "Consider using `///` for documentation comments on private items.\n" .
+ "$here\n$formatted_comments");
+ }
+ }
+
# Check the patch for a From:
if (decode("MIME-Header", $line) =~ /^From:\s*(.*)/) {
$author = $1;
--
2.43.0
Powered by blists - more mailing lists