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.

Sign up for GitHub

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

Jump to bottom

New lint: Require # Safety section in pub unsafe fn docs #4535

Merged
merged 1 commit into from
Sep 19, 2019
Merged

New lint: Require # Safety section in pub unsafe fn docs #4535

merged 1 commit into from
Sep 19, 2019

Conversation

llogiq
Copy link
Contributor

@llogiq llogiq commented Sep 11, 2019

changelog: add missing_safety_doc lint

This fixes #2207

flip1995
flip1995 requested changes Sep 12, 2019
View reviewed changes
clippy_lints/src/doc.rs Outdated Show resolved Hide resolved
clippy_lints/src/doc.rs Outdated Show resolved Hide resolved
clippy_lints/src/doc.rs Outdated Show resolved Hide resolved
clippy_lints/src/doc.rs Show resolved Hide resolved
tests/ui/doc_unsafe.rs Outdated Show resolved Hide resolved
@flip1995 flip1995 added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Sep 12, 2019
@llogiq llogiq force-pushed the unsafe-doc branch 5 times, most recently from 692fea2 to 6b6bdb8 Compare September 12, 2019 19:12
@llogiq
Copy link
Contributor Author

llogiq commented Sep 12, 2019

Thanks for the review!

r? @flip1995

flip1995
flip1995 reviewed Sep 13, 2019
View reviewed changes
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two NITs in the doc, then this should be ready to go

clippy_lints/src/doc.rs Outdated Show resolved Hide resolved
clippy_lints/src/doc.rs Outdated Show resolved Hide resolved
phansch
phansch reviewed Sep 13, 2019
View reviewed changes
tests/ui/doc_unsafe.rs
pub unsafe fn apocalypse(universe: &mut ()) {
unimplemented!();
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add a test that the lint doesn't trigger on non-async functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean an async function? How would that work?

Copy link
Member

@phansch phansch Sep 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah sorry, I meant adding a test for a non-unsafe function :D

@llogiq
Copy link
Contributor Author

llogiq commented Sep 13, 2019

Thanks again, @flip1995

@flip1995
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Sep 16, 2019

📌 Commit 38f252a has been approved by flip1995

@bors
Copy link
Contributor

bors commented Sep 16, 2019

⌛ Testing commit 38f252a with merge 2ff1202...

bors added a commit that referenced this pull request Sep 16, 2019
@bors
Auto merge of #4535 - rust-lang:unsafe-doc, r=flip1995
2ff1202 
New lint: Require `# Safety` section in pub unsafe fn docs

changelog: add `missing_safety_doc` lint

This fixes #2207
@bors
Copy link
Contributor

bors commented Sep 16, 2019

💔 Test failed - checks-travis

@flip1995
Copy link
Member

@bors rollup

@flip1995
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Sep 19, 2019

📌 Commit 70a7dab has been approved by flip1995

flip1995 added a commit to flip1995/rust-clippy that referenced this pull request Sep 19, 2019
@flip1995
Rollup merge of rust-lang#4535 - rust-lang:unsafe-doc, r=flip1995
93381fb 
New lint: Require `# Safety` section in pub unsafe fn docs

changelog: add `missing_safety_doc` lint

This fixes rust-lang#2207
@flip1995 flip1995 mentioned this pull request Sep 19, 2019
Closed
bors added a commit that referenced this pull request Sep 19, 2019
@bors
Auto merge of #4553 - flip1995:rollup-0chhiy3, r=flip1995
f84e26d 
Rollup of 4 pull requests

Successful merges:

 - #4511 (New lint: mem_replace_with_uninit)
 - #4535 (New lint: Require `# Safety` section in pub unsafe fn docs)
 - #4539 (Changes cast-lossless to a pedantic lint)
 - #4544 (#4542 remove machine applicable suggestion)

Failed merges:

r? @ghost

changelog: none
bors added a commit that referenced this pull request Sep 19, 2019
@bors
Auto merge of #4535 - rust-lang:unsafe-doc, r=flip1995
f783ff3 
New lint: Require `# Safety` section in pub unsafe fn docs

changelog: add `missing_safety_doc` lint

This fixes #2207
@bors
Copy link
Contributor

bors commented Sep 19, 2019

⌛ Testing commit 70a7dab with merge f783ff3...

@bors
Copy link
Contributor

bors commented Sep 19, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing f783ff3 to master...

@bors bors merged commit 70a7dab into master Sep 19, 2019
@flip1995 flip1995 deleted the unsafe-doc branch October 28, 2019 08:14
@flip1995 flip1995 mentioned this pull request Dec 4, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phansch phansch phansch left review comments

@flip1995 flip1995 flip1995 approved these changes

Assignees
No one assigned
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rust API Guideline: Unsafe functions are documented with a "Safety" section
4 participants
@llogiq @flip1995 @bors @phansch