Skip to content
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

Don't look for safety comments in doc tests #12066

Merged
merged 1 commit into from Jan 5, 2024
Merged

Conversation

y21
Copy link
Member

@y21 y21 commented Jan 1, 2024

Fixes #12048.

What happened in the linked issue is that the lint checks for lines that start with // and have SAFETY: somewhere in it above the function item.
This works for regular comments, but when the // is the start of a doc comment (e.g. /// // SAFETY: ...) and it's part of a doc test (i.e. within ```), we probably shouldn't lint that, since the user most likely meant to refer to a different node than the one currently being checked. For example in the linked issue, the safety comment refers to unsafe { *five_pointer }, but the lint believes it's part of the function item.

We also can't really easily test whether the // SAFETY: comment within a doc comment is necessary or not, since I think that would require creating a new compiler session to re-parse the contents of the doc comment. We already do this for one of the doc markdown lints, to look for a main function in doc tests, but I don't know how to feel about doing that in more places, so probably best to just ignore them?

changelog: [unnecessary_safety_comment]: don't look for safety comments in doc tests

@rustbot
Copy link
Collaborator

rustbot commented Jan 1, 2024

r? @Alexendoo

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 1, 2024
@Alexendoo
Copy link
Member

👍

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 5, 2024

📌 Commit ef35e82 has been approved by Alexendoo

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jan 5, 2024

⌛ Testing commit ef35e82 with merge d3dfecb...

@bors
Copy link
Collaborator

bors commented Jan 5, 2024

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

@bors bors merged commit d3dfecb into rust-lang:master Jan 5, 2024
5 checks passed
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.

False positive unnecessary_safety_comment in doc test
4 participants