Extend clippy::missing_safety_doc to unsafe fields

[jswrenn]

changelog: [missing_safety_doc]: lint unsafe fields

Makes progress towards rust-lang/rust#132922

[rustbot]

Some changes occurred in clippy_lints/src/doc

cc @notriddle

[rustbot]

r? @samueltardieu

rustbot has assigned @samueltardieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: 7 candidates
• 7 candidates expanded to 7 candidates
• Random selection from Jarcho, dswij, llogiq, samueltardieu

[github-actions]

No changes for a5b2489

[samueltardieu]

Can you have a look at the result of lintcheck at https://github.com/rust-lang/rust-clippy/actions/runs/23604878022#user-content-unnecessary-safety-doc? I wonder whether we should signal UNNECESSARY_SAFETY_DOC on fields which are not frozen, such as UnsafeCell and things derived from it.

@rustbot author

View changes since this review

[jswrenn]

That's perhaps a candidate for a field that should be marked unsafe, since marking the field unsafe will enforce that reading that field (e.g., via UnsafeField::into_inner) can only occur within an unsafe context.

More generally: Freeze-ness is orthogonal to safety; e.g.:

struct Even {
    // # Must be even
    unsafe n: AtomicU8,
}

[jswrenn]

Is there anything I can do to help get this merged? I'm at RustWeek if you'd like to chat!

[samueltardieu]

That's perhaps a candidate for a field that should be marked unsafe, since marking the field unsafe will enforce that reading that field (e.g., via UnsafeField::into_inner) can only occur within an unsafe context.

So maybe the lint could suggest that if the unsafe_fields feature is enabled as an alternative in an extra help message, and not lint if the feature is not enabled (because it makes sense to me to allow a SAFETY comment on a field whose main use is through unsafe methods, even though we don't want to make it mandatory).

WDYT?

[jswrenn]

Huh. Yeah, the current state of the PR is problematic. If feature(unsafe_fields) isn't and cannot be enabled (e.g., you run nightly clippy but target stable Rust), the lint basically nudges you to delete your "unnecessary" SAFETY documentation.

I'll revise the lint to only fire if the unsafe_fields feature is enable, and change the diagnostic to encourage adding unsafe rather than deleting one's SAFETY documentation.

[jswrenn]

Take a look and let me know what you think.

The naming and diagnostics of clippy::unnecessary_safety_doc is a little unfortunate with fields and traits too: in these cases as well, the lint probably should generally nudge you towards using unsafe rather than towards deleting documentation. I've left these cases unchanged, however.

[samueltardieu]

LGTM!