new lint: suspicious_doc_comments

[y21]

Fixes #10485.

This PR adds a new lint (suspicious_doc_comments) that triggers when the user writes ///! or /**!.
This is almost certainly a mistake and the user probably meant to write an inner doc comment (//!, /*!) to document the module or crate that this comment is contained in.

changelog: [suspicious_doc_comments]: new lint

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @dswij (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[bors]

☔ The latest upstream changes (presumably #10481) made this pull request unmergeable. Please resolve the merge conflicts.

[dswij]

Thanks for the PR! and welcome to Clippy.

Some minor comments and suggestions.

Also, not sure if I personally love the name outer_doc_comment_bang, but combined with the allow attribute, it looks neat. I don't know if there is a better name for it though.

[y21]

Also, not sure if I personally love the name outer_doc_comment_bang, but combined with the allow attribute, it looks neat. I don't know if there is a better name for it though.

I agree, I had a hard time coming up with a name for this lint. It seems like the linked issue that initially suggested this lint actually has a name suggestion (suspicious_doc_comment), totally missed that. I like that one. I'd be fine with renaming this to suspicious_doc_comment.

[y21]

Oops, I didn't mean to start a new review there 😅. The comment I left was in response to changing it from "merging" the spans together to linting the lines individually.

[y21]

I changed it to use mutlispan_sugg instead. Would suspicious_doc_comment be a better name for this lint or is this one fine?

[dswij]

Lint looks good! Just a couple small question/comment, and it'd be good to merge aftewards.

I think suspicious_doc_comments is nice because we have other similarly-named lints. Also note that the name would be plural because of Clippy's lint naming convention

[Alexendoo]

This error seems to be common which makes this a very useful lint

From the search, there's a few styles that would be good to add as tests

Both of these would be joined doc comments but in the first the intention would be to have two separate comments still

https://github.com/Twinklebear/tray_rust/blob/9359cb1868a723431214c9fa1fc8c1aef54adae3/src/sampler/morton.rs#L1-L4
https://github.com/openebs/mayastor/blob/aebfd876be16fa2676f311c36276efd6ad3aa019/sysfs/src/lib.rs#L1-L2

Various gaps

https://github.com/massalabs/massa/blob/b27e8416efac19e8ec470e96bb49c74687092ac3/massa-async-pool/src/changes.rs#L14-L16
https://github.com/Keats/jsonwebtoken/blob/8fba79b25459eacc33a80e1ee37ff8eba64079ca/src/jwk.rs#L2-L6

////! is probably a commented out outer doc comment

https://github.com/tanakh/reqwest/blob/147831375613a5e508487b2d85a99104ae1505af/src/lib.rs#L184-L185

It would also be good to see if it works on items that don't generally have doc comments applied to them, like

///! a
use x;

///! b
macro_rules! x {() => {}}

You don't have to explicitly handle all of these, particularly the first two are kind of conflicting, but it'd be good to have tests for them anyway

[y21]

https://github.com/Keats/jsonwebtoken/blob/8fba79b25459eacc33a80e1ee37ff8eba64079ca/src/jwk.rs#L2-L6

This is one of the "known-but-deliberately-ignored" problems, I think, so I don't think I can add a test for this, can I? // run-rustfix would make this fail the test

[Alexendoo]

You can put them in a separate suspicious_doc_comments_unfixable.rs without // run-rustfix

[y21]

Re the label: is there something left to do? I think I have a test for all the cases that were mentioned

[dswij]

@y21 oh, there's a conflict, so I thought you are still working on it :)

[y21]

Fixed the conflicts and cleaned up the commit history a bit.

[bors]

☔ The latest upstream changes (presumably #10601) made this pull request unmergeable. Please resolve the merge conflicts.

[dswij]

@y21 Huge thanks for this! Should be a really nice, useful lint

@bors r+

[bors]

📌 Commit 5d01e6e has been approved by dswij

It is now in the queue for this repository.

[bors]

⌛ Testing commit 5d01e6e with merge 4a2cb5a...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: dswij
Pushing 4a2cb5a to master...