Skip to content

Fix SAFETY comment tag casing in undocumented_unsafe_blocks #8138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 20, 2021

Conversation

wooster0
Copy link

@wooster0 wooster0 commented Dec 17, 2021

This changes the lint introduced in #7748 to suggest adding a SAFETY comment instead of a Safety comment.

Searching for // Safety: in rust-lang/rust yields 67 results while // SAFETY: yields 1072.
I think it's safe to say that this comment tag is written in upper case, just like TODO, FIXME and so on are. As such I would expect this lint to follow the official convention as well.

Note that I intentionally introduced some casing diversity in tests/ui/undocumented_unsafe_blocks.rs to test more cases than just Safety:.

changelog: Capitalize SAFETY comment in [undocumented_unsafe_blocks]

@rust-highfive
Copy link

r? @giraffate

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 17, 2021
@xFrednet
Copy link
Member

xFrednet commented Dec 18, 2021

I like standardizing the capitalization. 👍 You might also want to take a look at the missing_safety_doc lint. That one lints missing safety heading in doc comments. 🙃

@wooster0
Copy link
Author

@xFrednet I think in that case /// # Safety is fine as-is because it denotes a markdown section like e.g. /// # Examples does (which is not uppercased either). I believe that and this comment tag // SAFETY: which is directly followed by the explanation about why it is safe are two slightly different things.

@giraffate
Copy link
Contributor

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Dec 20, 2021

📌 Commit eba4413 has been approved by giraffate

@bors
Copy link
Contributor

bors commented Dec 20, 2021

⌛ Testing commit eba4413 with merge 7905130...

@bors
Copy link
Contributor

bors commented Dec 20, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing 7905130 to master...

@bors bors merged commit 7905130 into rust-lang:master Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants