`undocumented_unsafe_blocks` false positive when unsafe block is on a separate line from a `let`

[DropDemBits]

Summary

When a single-line unsafe block is long enough to be formatted on a new line,
it appears that the safety comment isn't found.

Lint Name

undocumented_unsafe_blocks

Reproducer

I tried this code:

#![deny(clippy::undocumented_unsafe_blocks)]

fn main() {
    let aaaaaa = 1;
    // SAFETY: A safety comment
    let _some_var_name =
        unsafe { path::to::aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaa, A::CONTEXT_INFO) };
}

pub(crate) mod path {
    pub(crate) mod to {
        pub(crate) unsafe fn aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(_: u32, _: ()) -> u32{
            1
        }
    }
}

struct A {}

impl A {
    const CONTEXT_INFO: () = ();
}

I saw this happen:

error: unsafe block missing a safety comment
 --> src/main.rs:7:9
  |
7 |         unsafe { path::to::aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaa, A::CONTEXT_INFO) };
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: consider adding a safety comment on the preceding line
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks
note: the lint level is defined here
 --> src/main.rs:1:9
  |
1 | #![deny(clippy::undocumented_unsafe_blocks)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I expected to see this happen:

Lint shouldn't fire

Version

rustc 1.71.0-nightly (1c42cb4ef 2023-04-26)
binary: rustc
commit-hash: 1c42cb4ef0544fbfaa500216e53382d6b079c001
commit-date: 2023-04-26
host: x86_64-pc-windows-msvc
release: 1.71.0-nightly
LLVM version: 16.0.2

Additional Labels

No response

[lochetti]

here it goes a bit smaller reproducer

#![deny(clippy::undocumented_unsafe_blocks)]
#![allow(clippy::missing_safety_doc)]

fn main() {
    // Safety: A safety comment
    let _some_variable_with_a_very_long_name =
        unsafe { very_long_function_name_to_break_the_line(3) };
}

pub unsafe fn very_long_function_name_to_break_the_line(_: u32) -> u32 {
    1
}

I'll try to take a look at this issue in the next days.
@rustbot claim

[lochetti]

I just want to be sure that we are OK in allowing the Safety comment to precede the let line instead of the unsafe block itself in those cases, before I continue, here.
For me, this change is making sense because what would fire the lint or not is the lenght of the variable/function name. And it would be strange for the developer to have the lint fired just because the variable/function name changed. But since I am very new here and I didn't participate in the genesis of this lint, I would prefer to ask for help.

@xFrednet do you have any opinion on this matter? Or do you think that some other team member could have a strong opinion?

[xFrednet]

The lint documentation clearly states that the safety comment should be above the unsafe block. I'm guessing that this was chosen to prevent the comment being several lines above the actual block. But I do see the point in allowing it in this case. I would recommend adding a configuration to the lint. I too potential ways a configuration might work:

1. We add a config value, like safety-comment-range: u32, that gives a range of how high the comment can be above the unsafe block
2. We add a config value, like accept-comment-above-statement: bool, that allows the safety comment to be placed above the statement, containing the unsafe block.

From these options, I think number 2 is favorable.

You can take a look at "Adding configuration to a lint" If you want to work on this :)