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

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

[Patrick Miller]

[-- Attachment #1.1: Type: text/plain, Size: 1173 bytes --]

Adds a check for documentation in rust file. Warns if certain known
documentation headers are not plural.

Signed-off-by: Patrick Miller <paddymills@proton.me>
Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://github.com/Rust-for-Linux/linux/issues/1110

---
 scripts/checkpatch.pl | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 39032224d504..0e99d11eeb04 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3900,6 +3900,14 @@ 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);
 		}
 
+# check that document section headers are plural in rust files
+        if (   $realfile =~ /\.rs$/
+            && $rawline =~ /^\+\s*\/\/\/\s+#+\s+(Example|Invariant)\s*$/ )
+        {
+            WARN( "RUST_DOC_HEADER",
+                "Rust doc he
aders should be plural\n" . $herecurr );
+        }
+
 # 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.0


[-- Attachment #1.2: publickey - paddymills@proton.me - 0xDCA74891.asc --]
[-- Type: application/pgp-keys, Size: 705 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

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

[Greg KH]

On Fri, Sep 06, 2024 at 04:45:49PM +0000, Patrick Miller wrote:
> Adds a check for documentation in rust file. Warns if certain known
> documentation headers are not plural.
> 
> Signed-off-by: Patrick Miller <paddymills@proton.me>
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Link: https://github.com/Rust-for-Linux/linux/issues/1110
> 
> ---
>  scripts/checkpatch.pl | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 39032224d504..0e99d11eeb04 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3900,6 +3900,14 @@ 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);
>  		}
>  
> +# check that document section headers are plural in rust files
> +        if (   $realfile =~ /\.rs$/
> +            && $rawline =~ /^\+\s*\/\/\/\s+#+\s+(Example|Invariant)\s*$/ )
> +        {
> +            WARN( "RUST_DOC_HEADER",
> +                "Rust doc he
> aders should be plural\n" . $herecurr );

Something went wrong, your patch lost the tabs and it had a line wrap?

And why is Rust unique for plurals here?  What if there really is only
one example?

thanks,

greg k-h

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

[Patrick Miller]

[-- Attachment #1.1: Type: text/plain, Size: 1890 bytes --]






On Friday, September 6th, 2024 at 1:00 PM, Greg KH <gregkh@linuxfoundation.org> wrote:

> 

> 

> On Fri, Sep 06, 2024 at 04:45:49PM +0000, Patrick Miller wrote:
> 

> > Adds a check for documentation in rust file. Warns if certain known
> > documentation headers are not plural.
> > 

> > Signed-off-by: Patrick Miller paddymills@proton.me
> > Suggested-by: Miguel Ojeda ojeda@kernel.org
> > Link: https://github.com/Rust-for-Linux/linux/issues/1110
> > 

> > ---
> > scripts/checkpatch.pl | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> > 

> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 39032224d504..0e99d11eeb04 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -3900,6 +3900,14 @@ 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);
> > }
> > 

> > +# check that document section headers are plural in rust files
> > + if ( $realfile =~ /\.rs$/
> > + && $rawline =~ /^\+\s*\/\/\/\s+#+\s+(Example|Invariant)\s*$/ )
> > + {
> > + WARN( "RUST_DOC_HEADER",
> > + "Rust doc he
> > aders should be plural\n" . $herecurr );
> 

> 

> Something went wrong, your patch lost the tabs and it had a line wrap?
My bad! Editor settings strikes again.

> 

> And why is Rust unique for plurals here? What if there really is only
> one example?
Per Miguel Ojeda (who suggested the issue): "We prefer plurals when writing code documentation sections: # Examples, # Invariants, # Guarantees and # Panics, so that it is consistent and so that one can add more examples without having to change the section name."
> 

> thanks,
> 

> greg k-h

I apologize, first time submitting. Do I submit a new email or add the fixed patch to this one?

[-- Attachment #1.2: publickey - paddymills@proton.me - 0xDCA74891.asc --]
[-- Type: application/pgp-keys, Size: 653 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

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

[Greg KH]

On Fri, Sep 06, 2024 at 05:29:28PM +0000, Patrick Miller wrote:
> 
> 
> 
> 
> 
> On Friday, September 6th, 2024 at 1:00 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > 
> 
> > 
> 
> > On Fri, Sep 06, 2024 at 04:45:49PM +0000, Patrick Miller wrote:
> > 
> 
> > > Adds a check for documentation in rust file. Warns if certain known
> > > documentation headers are not plural.
> > > 
> 
> > > Signed-off-by: Patrick Miller paddymills@proton.me
> > > Suggested-by: Miguel Ojeda ojeda@kernel.org
> > > Link: https://github.com/Rust-for-Linux/linux/issues/1110
> > > 
> 
> > > ---
> > > scripts/checkpatch.pl | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > > 
> 
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > index 39032224d504..0e99d11eeb04 100755
> > > --- a/scripts/checkpatch.pl
> > > +++ b/scripts/checkpatch.pl
> > > @@ -3900,6 +3900,14 @@ 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);
> > > }
> > > 
> 
> > > +# check that document section headers are plural in rust files
> > > + if ( $realfile =~ /\.rs$/
> > > + && $rawline =~ /^\+\s*\/\/\/\s+#+\s+(Example|Invariant)\s*$/ )
> > > + {
> > > + WARN( "RUST_DOC_HEADER",
> > > + "Rust doc he
> > > aders should be plural\n" . $herecurr );
> > 
> 
> > 
> 
> > Something went wrong, your patch lost the tabs and it had a line wrap?
> My bad! Editor settings strikes again.
> 
> > 
> 
> > And why is Rust unique for plurals here? What if there really is only
> > one example?
> Per Miguel Ojeda (who suggested the issue): "We prefer plurals when writing code documentation sections: # Examples, # Invariants, # Guarantees and # Panics, so that it is consistent and so that one can add more examples without having to change the section name."
> > 
> 
> > thanks,
> > 
> 
> > greg k-h
> 
> I apologize, first time submitting. Do I submit a new email or add the fixed patch to this one?

As this one can't apply, you need to send a whole new one :)


thanks,

greg k-h

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

[Patrick Miller]

[-- Attachment #1.1: Type: text/plain, Size: 1363 bytes --]

Adds a check for documentation in rust file. Warns if certain known
documentation headers are not plural. Even though some sections may
be singular (i.e. only one example), the header should still be plural
so that more examples can be added later without needing to change the
header.

Fixed the whitespace issue on my previous patch.

Signed-off-by: Patrick Miller <paddymills@proton.me>
Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://github.com/Rust-for-Linux/linux/issues/1110

---
 scripts/checkpatch.pl | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 39032224d504..cb5ecdb6df9b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3900,6 +3900,13 @@ 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);
 		}
 
+# check that document sec
tion headers are plural in rust files
+		if ($realfile =~ /\.rs$/
+			&& $rawline =~ /^\+\s*\/\/\/\s+#+\s+(Example|Invariant|Guarantee|Panic)\s*$/) {
+			WARN( "RUST_DOC_HEADER",
+				"Rust doc headers should be plural\n" . $herecurr );
+		}
+
 # 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.0


[-- Attachment #1.2: publickey - paddymills@proton.me - 0xDCA74891.asc --]
[-- Type: application/pgp-keys, Size: 705 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

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

[Joe Perches]

On Fri, 2024-09-06 at 18:05 +0000, Patrick Miller wrote:
> Adds a check for documentation in rust file. Warns if certain known
> documentation headers are not plural. Even though some sections may
> be singular (i.e. only one example), the header should still be plural
> so that more examples can be added later without needing to change the
> header.
> 
> Fixed the whitespace issue on my previous patch.

Well, one of them.

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -3900,6 +3900,13 @@ 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);
>  		}
>  
> +# check that document section headers are plural in rust files
> +		if ($realfile =~ /\.rs$/
> +			&& $rawline =~ /^\+\s*\/\/\/\s+#+\s+(Example|Invariant|Guarantee|Panic)\s*$/) {
> +			WARN( "RUST_DOC_HEADER",
> +				"Rust doc headers should be plural\n" . $herecurr );

There is no autoformatter for checkpatch/perl/etc.

Continuation && on previous line
No space after open paren
Align to open paren
No space before close paren

I think this is an overly trivial addition.

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

[Patrick Miller]

[-- Attachment #1.1: Type: text/plain, Size: 1729 bytes --]






On Saturday, September 7th, 2024 at 6:53 AM, Joe Perches <joe@perches.com> wrote:

> 

> 

> On Fri, 2024-09-06 at 18:05 +0000, Patrick Miller wrote:
> 

> > Adds a check for documentation in rust file. Warns if certain known
> > documentation headers are not plural. Even though some sections may
> > be singular (i.e. only one example), the header should still be plural
> > so that more examples can be added later without needing to change the
> > header.
> > 

> > Fixed the whitespace issue on my previous patch.
> 

> 

> Well, one of them.
> 

> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> 

> []
> 

> > @@ -3900,6 +3900,13 @@ 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);
> > }
> > 

> > +# check that document section headers are plural in rust files
> > + if ($realfile =~ /\.rs$/
> > + && $rawline =~ /^\+\s*\/\/\/\s+#+\s+(Example|Invariant|Guarantee|Panic)\s*$/) {
> > + WARN( "RUST_DOC_HEADER",
> > + "Rust doc headers should be plural\n" . $herecurr );
> 

> 

> There is no autoformatter for checkpatch/perl/etc.
> 

> Continuation && on previous line
Will fix this. Is there a code style document for the perl scripts?
> No space after open paren
> Align to open paren
> No space before close paren
I based my coding style off of the if statement directly above mine,
although I did miss the continuation of &&

> 

> I think this is an overly trivial addition.
I am responding to an issue that is part of rust-for-linux. Maybe
Miguel Ojeda has something to add on this matter?

[-- Attachment #1.2: publickey - paddymills@proton.me - 0xDCA74891.asc --]
[-- Type: application/pgp-keys, Size: 653 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

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

[Patrick Miller]

[-- Attachment #1.1: Type: text/plain, Size: 2157 bytes --]






On Saturday, September 7th, 2024 at 7:57 AM, Patrick Miller <paddymills@proton.me> wrote:

> 

> 

> 

> 

> 

> 

> 

> On Saturday, September 7th, 2024 at 6:53 AM, Joe Perches joe@perches.com wrote:
> 

> > On Fri, 2024-09-06 at 18:05 +0000, Patrick Miller wrote:
> > 

> > > Adds a check for documentation in rust file. Warns if certain known
> > > documentation headers are not plural. Even though some sections may
> > > be singular (i.e. only one example), the header should still be plural
> > > so that more examples can be added later without needing to change the
> > > header.
> > > 

> > > Fixed the whitespace issue on my previous patch.
> > 

> > Well, one of them.
> > 

> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > 

> > []
> > 

> > > @@ -3900,6 +3900,13 @@ 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);
> > > }
> > > 

> > > +# check that document section headers are plural in rust files
> > > + if ($realfile =~ /\.rs$/
> > > + && $rawline =~ /^\+\s*\/\/\/\s+#+\s+(Example|Invariant|Guarantee|Panic)\s*$/) {
> > > + WARN( "RUST_DOC_HEADER",
> > > + "Rust doc headers should be plural\n" . $herecurr );
> > 

> > There is no autoformatter for checkpatch/perl/etc.
Can you elaborate on what you mean by this? I'm not following.
Is there documentation you can point me to that I need to review?
> > 

> > Continuation && on previous line
> 

> Will fix this. Is there a code style document for the perl scripts?
> 

> > No space after open paren
> > Align to open paren
> > No space before close paren
> 

> I based my coding style off of the if statement directly above mine,
> although I did miss the continuation of &&
Sorry, looked at this again and realize you're talking about the `WARN` statement.
> 

> > I think this is an overly trivial addition.
> 

> I am responding to an issue that is part of rust-for-linux. Maybe
> Miguel Ojeda has something to add on this matter?

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

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

[Joe Perches]

On Sat, 2024-09-07 at 14:22 +0000, Patrick Miller wrote:
> > > > +# check that document section headers are plural in rust files
> > > > + if ($realfile =~ /\.rs$/
> > > > + && $rawline =~ /^\+\s*\/\/\/\s+#+\s+(Example|Invariant|Guarantee|Panic)\s*$/) {
> > > > + WARN( "RUST_DOC_HEADER",
> > > > + "Rust doc headers should be plural\n" . $herecurr );
[]
> > There is no autoformatter for checkpatch/perl/etc.
> Can you elaborate on what you mean by this? I'm not following.
> Is there documentation you can point me to that I need to review?
> > 
> 

[]
> > Will fix this. Is there a code style document for the perl scripts?

No.  It mostly follows the c style though.

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

[Benno Lossin]

On 07.09.24 12:53, Joe Perches wrote:
> On Fri, 2024-09-06 at 18:05 +0000, Patrick Miller wrote:
>> Adds a check for documentation in rust file. Warns if certain known
>> documentation headers are not plural. Even though some sections may
>> be singular (i.e. only one example), the header should still be plural
>> so that more examples can be added later without needing to change the
>> header.
>>
>> Fixed the whitespace issue on my previous patch.
> 
> Well, one of them.
> 
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
>> @@ -3900,6 +3900,13 @@ 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);
>>  		}
>>
>> +# check that document section headers are plural in rust files
>> +		if ($realfile =~ /\.rs$/
>> +			&& $rawline =~ /^\+\s*\/\/\/\s+#+\s+(Example|Invariant|Guarantee|Panic)\s*$/) {
>> +			WARN( "RUST_DOC_HEADER",
>> +				"Rust doc headers should be plural\n" . $herecurr );
> 
> There is no autoformatter for checkpatch/perl/etc.
> 
> Continuation && on previous line
> No space after open paren
> Align to open paren
> No space before close paren
> 
> I think this is an overly trivial addition.

We want this so we don't have to manually remind people to fix their
documentation sections. Quoting Miguel from the issue [1]:

> We prefer plurals when writing code documentation sections:
> `# Examples`, `# Invariants`, `# Guarantees` and `# Panics`, so that
> it is consistent and so that one can add more examples without having
> to change the section name.

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

---
Cheers,
Benno

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

[Benno Lossin]

Hi,

Thanks for the patch!

On 06.09.24 20:05, Patrick Miller wrote:
> Adds a check for documentation in rust file. Warns if certain known
> documentation headers are not plural. Even though some sections may
> be singular (i.e. only one example), the header should still be plural
> so that more examples can be added later without needing to change the
> header.
> 
> Fixed the whitespace issue on my previous patch.

This line shouldn't be part of the commit message, as it only makes
sense if you know there existed a previous version. You can put a
changelog underneath the `---` line, that part won't be put into the
final commit message.

I am not familiar with perl, but the regex looks good to me.

---
Cheers,
Benno

> Signed-off-by: Patrick Miller <paddymills@proton.me>
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Link: https://github.com/Rust-for-Linux/linux/issues/1110
> 
> ---
>  scripts/checkpatch.pl | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 39032224d504..cb5ecdb6df9b 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3900,6 +3900,13 @@ 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);
>  		}
> 
> +# check that document sec
> tion headers are plural in rust files
> +		if ($realfile =~ /\.rs$/
> +			&& $rawline =~ /^\+\s*\/\/\/\s+#+\s+(Example|Invariant|Guarantee|Panic)\s*$/) {
> +			WARN( "RUST_DOC_HEADER",
> +				"Rust doc headers should be plural\n" . $herecurr );
> +		}
> +
>  # 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.0
> 

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

[Benno Lossin]

On 09.09.24 18:31, Benno Lossin wrote:
> Hi,
> 
> Thanks for the patch!
> 
> On 06.09.24 20:05, Patrick Miller wrote:
>> Adds a check for documentation in rust file. Warns if certain known
>> documentation headers are not plural. Even though some sections may
>> be singular (i.e. only one example), the header should still be plural
>> so that more examples can be added later without needing to change the
>> header.
>>
>> Fixed the whitespace issue on my previous patch.
> 
> This line shouldn't be part of the commit message, as it only makes
> sense if you know there existed a previous version. You can put a
> changelog underneath the `---` line, that part won't be put into the
> final commit message.

Also one more thing that I forgot, when sending a new version of a
patch, please mark it in the patch title with `[PATCH v2 2/2]` (you can
do it with `git format-patch -v2`).

Also don't immediately resend it, but wait for other people to also
provide feedback.

---
Cheers,
Benno

> I am not familiar with perl, but the regex looks good to me.
> 
> ---
> Cheers,
> Benno
> 
>> Signed-off-by: Patrick Miller <paddymills@proton.me>
>> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
>> Link: https://github.com/Rust-for-Linux/linux/issues/1110
>>
>> ---
>>  scripts/checkpatch.pl | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index 39032224d504..cb5ecdb6df9b 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -3900,6 +3900,13 @@ 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);
>>  		}
>>
>> +# check that document sec
>> tion headers are plural in rust files
>> +		if ($realfile =~ /\.rs$/
>> +			&& $rawline =~ /^\+\s*\/\/\/\s+#+\s+(Example|Invariant|Guarantee|Panic)\s*$/) {
>> +			WARN( "RUST_DOC_HEADER",
>> +				"Rust doc headers should be plural\n" . $herecurr );
>> +		}
>> +
>>  # 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.0
>>
> 
>