Message-ID: <06dde2dd-5d88-49d4-9e46-72a2e12ab1c2@arnaud-lcm.com>
Date: Tue, 22 Apr 2025 16:37:46 +0200
From: Arnaud Lecomte <contact@...aud-lcm.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc: 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>,
 linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
 llvm@...ts.linux.dev, skhan@...uxfoundation.org
Subject: Re: [PATCH v3 1/2] checkpatch.pl: warn about // comments on private
 Rust items


On 22/04/2025 15:46, Miguel Ojeda wrote:
> On Tue, Apr 22, 2025 at 2:58 PM Arnaud Lecomte <contact@...aud-lcm.com> wrote:
>> The detection uses multiple heuristics to identify likely doc comments:
>>    - Comments containing markdown
> Markdown is required in both documentation and comments, so I don't
> think we can use some of those, e.g. inline code spans (i.e.
> backticks) are common (and actually expected) in comments. Something
> like a title (i.e. `#`) or intra-doc links are uncommon, though.

Let's then maybe reduce the score of this heuristic and remove intra-doc 
links and titles detection. After reviewing, it indeed doesn't really 
make sense.

>>    - Comments starting with an imperative tone
> Some people document using the third-person, e.g. some functions say
> "Returns ..." like you have below. (It is not easy to enforce
> kernel-wide a single style here, thus so far we don't.)
>
> (Looking briefly at the code) Ah, I think you are covering both, good.

As mentioned earlier, we can reduce the score of any heuristic which 
could lead to any important false positive.
In my opinion, as long as the heuristic is relevant, we always have the 
possibility to diminish the score associated with the heuristic, hence 
preventing unnecessary false positives.

>>    - Comments with references: @see, @link, ...
> Do you mean Markdown references? Or javadoc-like ones?
>
> (Looking again at the code...) I think you are referring to actually
> strings like `@...`. Hmm... I don't think we have those -- the only
> `@` I would expect in a comment are thinks like emails or the
> `rustdoc` syntax to disambiguate the "kind" of item, e.g.
> `type@...ThreadSafe`. I do see a `@...len` somewhere, but that should
> have been an inline code span, and anyway it is not a `@...` or
> `@...k`. But I may be confused here?
>
I think that you are definitely more experienced with what's done 
commonly in rust code. Let's maybe change this heuristic definition with 
@ related to types or some other annotation. Do you have some example I 
could have a look to come in the next version with a relevant list of @  
we can encounter.
>> Comments are only flagged if they:
>>   - Appear above private items
>>   - Don't contain obvious non-doc patterns (TODO, FIXME)
>>   - Score sufficiently on heuristics
> Nice work! I wasn't expecting something with actual weighted scoring,
> but if the maintainers are OK with something as involved as that, then
> I guess it is fine. We may need to tweak the scoring in the future,
> but it may be a good experiment.

I think it is a nice approach due to the granularity it offers. This 
will drastically reduce the number of false positives.

> Thanks!
>
> Cheers,
> Miguel


Thanks for your feedbacks,

Arnaud

Powered by blists - more mailing lists