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

undocumented_unsafe_blocks does not trigger on unsafe trait impls #8505

Closed
Tracked by #79
djkoloski opened this issue Mar 4, 2022 · 3 comments · Fixed by #8761
Closed
Tracked by #79

undocumented_unsafe_blocks does not trigger on unsafe trait impls #8505

djkoloski opened this issue Mar 4, 2022 · 3 comments · Fixed by #8761
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing I-false-negative Issue: The lint should have been triggered on code, but wasn't

Comments

@djkoloski
Copy link

djkoloski commented Mar 4, 2022

Summary

missing_safety_doc and undocumented_unsafe_blocks complement each other to provide full safety documentation coverage. While missing_safety_doc triggers on unsafe trait definitions and warn that safety docs must be provided, undocumented_unsafe_blocks does not trigger on unsafe trait implementations.

Lint Name

undocumented_unsafe_blocks

Reproducer

I tried this code:

#![warn(clippy::undocumented_unsafe_blocks)]

/// # Safety
///
/// The implementor must justify with a safety comment.
unsafe trait Foo {}

unsafe impl Foo for () {}

/// # Safety
///
/// The caller must justify with a safety comment.
unsafe fn foo() {}

fn main() {
    // SAFETY: It's justified.
    unsafe {
        foo();
    }
}

I expected to see this happen:

warning: unsafe impl missing a safety comment
  --> src/main.rs:8:1

Instead, this happened:

No lint triggered for the unsafe impl.

Version

rustc 1.59.0 (9d1b2106e 2022-02-23)
binary: rustc
commit-hash: 9d1b2106e23b1abd32fce1f17267604a5102f57a
commit-date: 2022-02-23
host: x86_64-apple-darwin
release: 1.59.0
LLVM version: 13.0.0
@djkoloski djkoloski added C-bug Category: Clippy is not doing the correct thing I-false-negative Issue: The lint should have been triggered on code, but wasn't labels Mar 4, 2022
@tamaroning
Copy link
Contributor

@rustbot claim

@tamaroning
Copy link
Contributor

I'm gonna tackle with this after #8450 is merged

@repi
Copy link

repi commented Apr 24, 2022

Would love to see this! We are using undocumented_unsafe_blocks by default and this is an important gap in its coverage

bors added a commit that referenced this issue May 15, 2022
`undocumented_unsafe_blocks` does not trigger on unsafe trait impls

Closes #8505

changelog:
~~`unsafe impl`s from macro invocations don't trigger the lint for now.~~
~~This lint checks unsafe impls from/not from macro expansions~~
This lint checks unsafe impls NOT from macro expansions and checks ones in macro declarations.
@bors bors closed this as completed in a1632ff May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-negative Issue: The lint should have been triggered on code, but wasn't
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants