Add undocumented_unsafe_blocks lint

[Serial-ATA]

changelog: Added a new lint [undocumented_unsafe_blocks]

Fixes #7464, #7238 (?)

[rust-highfive]

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

Please see the contribution instructions for more information.

[xFrednet]

Welcome to the project, it's nice to see that you got a working implementation 🙃.

@flip1995 would you maybe like to review this, as you've also reviewed #7557? Otherwise, I'll have a look at it during the next week 🙃

[flip1995]

Yes, sure. I have time this afternoon.

r? @flip1995

[flip1995]

If I had to guess, I assume, that you didn't look at #7557 before implementing this?

I think your approach and the approach taken in #7557 combined would be ideal. I left comments about that below. IMO the work done in #7557 is really valuable. The PR was only closed, because it was inactive.

[bors]

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

[ShadowJonathan]

Thanks! Just missing one feature from the original suggestion in #7464;

The lint would also raise a warning if that comment is copied verbatim.

...

The following code would still raise a warning (not because of what transmute is doing here, but because the safety comment still has "...", as suggested. The developer would need to update it with an actual sentence.);

// Safety: ...
let totally_a_usize = unsafe {
    std::mem::transmute::<String, usize>(string)
};

[flip1995]

Impl LGTM overall now. A few minor improvements could be made.

[flip1995]

This PR should be almost done. I have some suggestions on how to simplify the code a bit.

The tests look awesome! The only test that is missing is a macro test. Two important tests here would be

• unsafe block in the macro call: m!(unsafe{ ... }
• unsafe block in the macro definition

[ShadowJonathan]

Just for posterity; the behaviour that makes sure that "..." is also checked and errored on is removed, I think that's fine for now to get this PR through, but I'd still like to see it somewhere in the future

[bors]

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

[flip1995]

One more thing that came to mind: What if the unsafe block has multi-line code in it? How does the suggestion look in that case? I expect that the full unsafe block is shown. We may want to avoid this, by using the span_until_char function ( or similar functions)

[flip1995]

Looks great now! Could you address the multi-line unsafe block case? After that this should be good to go. 🚀

[Serial-ATA]

Did I not take care of that? The no_comment_match and internal_comment tests only show the first line now.

[flip1995]

Did I not take care of that? The no_comment_match and internal_comment tests only show the first line now.

Oh I missed that. In that case all good 👍

(no_comment_match doesn't test that, because the unsafe block is still empty and all on one line in this test case, but the other test case does)

[flip1995]

Please squash some commits, so that we can merge this.

[bors]

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

[flip1995]

@bors r+

Thanks for sticking around through all the review iterations. Great work!

[bors]

📌 Commit 412b862 has been approved by flip1995

[bors]

⌛ Testing commit 412b862 with merge 78e7312...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 78e7312 to master...