fix: undocumented_unsafe_blocks FP on trait/impl items (#13888)

fixes #11709

Continuation of #12672. r? @Alexendoo if you don't mind?

changelog: [`undocumented_unsafe_blocks`] fix FP on trait/impl items

Author: xFrednet

clippy_lints/src/undocumented_unsafe_blocks.rs

@@ -339,6 +339,33 @@ fn is_unsafe_from_proc_macro(cx: &LateContext<'_>, span: Span) -> bool {
         .is_none_or(|src| !src.starts_with("unsafe"))
 }
 
+fn find_unsafe_block_parent_in_expr<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx hir::Expr<'tcx>,
+) -> Option<(Span, HirId)> {
+    match cx.tcx.parent_hir_node(expr.hir_id) {
+        Node::LetStmt(hir::LetStmt { span, hir_id, .. })
+        | Node::Expr(hir::Expr {
+            hir_id,
+            kind: hir::ExprKind::Assign(_, _, span),
+            ..
+        }) => Some((*span, *hir_id)),
+        Node::Expr(expr) => find_unsafe_block_parent_in_expr(cx, expr),
+        node if let Some((span, hir_id)) = span_and_hid_of_item_alike_node(&node)
+            && is_const_or_static(&node) =>
+        {
+            Some((span, hir_id))
+        },
+
+        _ => {
+            if is_branchy(expr) {
+                return None;
+            }
+            Some((expr.span, expr.hir_id))
+        },
+    }
+}
+
 // Checks if any parent {expression, statement, block, local, const, static}
 // has a safety comment
 fn block_parents_have_safety_comment(
@@ -348,21 +375,7 @@ fn block_parents_have_safety_comment(
     id: HirId,
 ) -> bool {
     let (span, hir_id) = match cx.tcx.parent_hir_node(id) {
-        Node::Expr(expr) => match cx.tcx.parent_hir_node(expr.hir_id) {
-            Node::LetStmt(hir::LetStmt { span, hir_id, .. }) => (*span, *hir_id),
-            Node::Item(hir::Item {
-                kind: ItemKind::Const(..) | ItemKind::Static(..),
-                span,
-                owner_id,
-                ..
-            }) => (*span, cx.tcx.local_def_id_to_hir_id(owner_id.def_id)),
-            _ => {
-                if is_branchy(expr) {
-                    return false;
-                }
-                (expr.span, expr.hir_id)
-            },
-        },
+        Node::Expr(expr) if let Some(inner) = find_unsafe_block_parent_in_expr(cx, expr) => inner,
         Node::Stmt(hir::Stmt {
             kind:
                 hir::StmtKind::Let(hir::LetStmt { span, hir_id, .. })
@@ -371,12 +384,13 @@ fn block_parents_have_safety_comment(
             ..
         })
         | Node::LetStmt(hir::LetStmt { span, hir_id, .. }) => (*span, *hir_id),
-        Node::Item(hir::Item {
-            kind: ItemKind::Const(..) | ItemKind::Static(..),
-            span,
-            owner_id,
-            ..
-        }) => (*span, cx.tcx.local_def_id_to_hir_id(owner_id.def_id)),
+
+        node if let Some((span, hir_id)) = span_and_hid_of_item_alike_node(&node)
+            && is_const_or_static(&node) =>
+        {
+            (span, hir_id)
+        },
+
         _ => return false,
     };
     // if unsafe block is part of a let/const/static statement,
@@ -427,12 +441,12 @@ fn block_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
 }
 
 fn include_attrs_in_span(cx: &LateContext<'_>, hir_id: HirId, span: Span) -> Span {
-    span.to(cx
-        .tcx
-        .hir()
-        .attrs(hir_id)
-        .iter()
-        .fold(span, |acc, attr| acc.to(attr.span())))
+    span.to(cx.tcx.hir().attrs(hir_id).iter().fold(span, |acc, attr| {
+        if attr.is_doc_comment() {
+            return acc;
+        }
+        acc.to(attr.span())
+    }))
 }
 
 enum HasSafetyComment {
@@ -604,31 +618,35 @@ fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span
 
 fn get_body_search_span(cx: &LateContext<'_>) -> Option<Span> {
     let body = cx.enclosing_body?;
-    let mut span = cx.tcx.hir_body(body).value.span;
-    let mut maybe_global_var = false;
-    for (_, node) in cx.tcx.hir_parent_iter(body.hir_id) {
-        match node {
-            Node::Expr(e) => span = e.span,
-            Node::Block(_) | Node::Arm(_) | Node::Stmt(_) | Node::LetStmt(_) => (),
-            Node::Item(hir::Item {
-                kind: ItemKind::Const(..) | ItemKind::Static(..),
-                ..
-            }) => maybe_global_var = true,
+    let mut maybe_mod_item = None;
+
+    for (_, parent_node) in cx.tcx.hir_parent_iter(body.hir_id) {
+        match parent_node {
+            Node::Crate(mod_) => return Some(mod_.spans.inner_span),
             Node::Item(hir::Item {
-                kind: ItemKind::Mod(_),
-                span: item_span,
+                kind: ItemKind::Mod(mod_),
+                span,
                 ..
             }) => {
-                span = *item_span;
-                break;
+                return maybe_mod_item
+                    .and_then(|item| comment_start_before_item_in_mod(cx, mod_, *span, &item))
+                    .map(|comment_start| mod_.spans.inner_span.with_lo(comment_start))
+                    .or(Some(*span));
+            },
+            node if let Some((span, _)) = span_and_hid_of_item_alike_node(&node)
+                && !is_const_or_static(&node) =>
+            {
+                return Some(span);
+            },
+            Node::Item(item) => {
+                maybe_mod_item = Some(*item);
             },
-            Node::Crate(mod_) if maybe_global_var => {
-                span = mod_.spans.inner_span;
+            _ => {
+                maybe_mod_item = None;
             },
-            _ => break,
         }
     }
-    Some(span)
+    None
 }
 
 fn span_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
@@ -717,3 +735,28 @@ fn text_has_safety_comment(src: &str, line_starts: &[RelativeBytePos], start_pos
         }
     }
 }
+
+fn span_and_hid_of_item_alike_node(node: &Node<'_>) -> Option<(Span, HirId)> {
+    match node {
+        Node::Item(item) => Some((item.span, item.owner_id.into())),
+        Node::TraitItem(ti) => Some((ti.span, ti.owner_id.into())),
+        Node::ImplItem(ii) => Some((ii.span, ii.owner_id.into())),
+        _ => None,
+    }
+}
+
+fn is_const_or_static(node: &Node<'_>) -> bool {
+    matches!(
+        node,
+        Node::Item(hir::Item {
+            kind: ItemKind::Const(..) | ItemKind::Static(..),
+            ..
+        }) | Node::ImplItem(hir::ImplItem {
+            kind: hir::ImplItemKind::Const(..),
+            ..
+        }) | Node::TraitItem(hir::TraitItem {
+            kind: hir::TraitItemKind::Const(..),
+            ..
+        })
+    )
+}

tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.default.stderr

@@ -310,5 +310,37 @@ LL |             let bar = unsafe {};
    |
    = help: consider adding a safety comment on the preceding line
 
-error: aborting due to 35 previous errors
+error: unsafe block missing a safety comment
+  --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:638:52
+   |
+LL |         const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 };
+   |                                                    ^^^^^^^^^^^^
+   |
+   = help: consider adding a safety comment on the preceding line
+
+error: unsafe block missing a safety comment
+  --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:647:41
+   |
+LL |         const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
+   |                                         ^^^^^^^^^^^^
+   |
+   = help: consider adding a safety comment on the preceding line
+
+error: unsafe block missing a safety comment
+  --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:657:42
+   |
+LL |         const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 };
+   |                                          ^^^^^^^^^^^^
+   |
+   = help: consider adding a safety comment on the preceding line
+
+error: unsafe block missing a safety comment
+  --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:662:40
+   |
+LL |         const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 };
+   |                                        ^^^^^^^^^^^^
+   |
+   = help: consider adding a safety comment on the preceding line
+
+error: aborting due to 39 previous errors
 

tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr

@@ -390,5 +390,53 @@ LL |     unsafe {}
    |
    = help: consider adding a safety comment on the preceding line
 
-error: aborting due to 45 previous errors
+error: unsafe block missing a safety comment
+  --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:638:52
+   |
+LL |         const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 };
+   |                                                    ^^^^^^^^^^^^
+   |
+   = help: consider adding a safety comment on the preceding line
+
+error: unsafe block missing a safety comment
+  --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:647:41
+   |
+LL |         const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
+   |                                         ^^^^^^^^^^^^
+   |
+   = help: consider adding a safety comment on the preceding line
+
+error: unsafe block missing a safety comment
+  --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:657:42
+   |
+LL |         const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 };
+   |                                          ^^^^^^^^^^^^
+   |
+   = help: consider adding a safety comment on the preceding line
+
+error: unsafe block missing a safety comment
+  --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:662:40
+   |
+LL |         const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 };
+   |                                        ^^^^^^^^^^^^
+   |
+   = help: consider adding a safety comment on the preceding line
+
+error: unsafe block missing a safety comment
+  --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:673:9
+   |
+LL |         unsafe { here_is_another_variable_with_long_name };
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider adding a safety comment on the preceding line
+
+error: unsafe block missing a safety comment
+  --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:702:9
+   |
+LL |         unsafe { Date::__from_ordinal_date_unchecked(1970, 1) }.into_julian_day_just_make_this_line_longer();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider adding a safety comment on the preceding line
+
+error: aborting due to 51 previous errors
 

tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs

@@ -632,4 +632,75 @@ mod issue_11246 {
 // Safety: Another safety comment
 const FOO: () = unsafe {};
 
+// trait items and impl items
+mod issue_11709 {
+    trait MyTrait {
+        const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 };
+        //~^ ERROR: unsafe block missing a safety comment
+
+        // SAFETY: safe
+        const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
+
+        // SAFETY: unrelated
+        unsafe fn unsafe_fn() {}
+
+        const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
+        //~^ ERROR: unsafe block missing a safety comment
+    }
+
+    struct UnsafeStruct;
+
+    impl MyTrait for UnsafeStruct {
+        // SAFETY: safe in this impl
+        const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 2 };
+
+        const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 };
+        //~^ ERROR: unsafe block missing a safety comment
+    }
+
+    impl UnsafeStruct {
+        const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 };
+        //~^ ERROR: unsafe block missing a safety comment
+    }
+}
+
+fn issue_13024() {
+    let mut just_a_simple_variable_with_a_very_long_name_that_has_seventy_eight_characters = 0;
+    let here_is_another_variable_with_long_name = 100;
+
+    // SAFETY: fail ONLY if `accept-comment-above-statement = false`
+    just_a_simple_variable_with_a_very_long_name_that_has_seventy_eight_characters =
+        unsafe { here_is_another_variable_with_long_name };
+    //~[disabled]^ undocumented_unsafe_blocks
+}
+
+// https://docs.rs/time/0.3.36/src/time/offset_date_time.rs.html#35
+mod issue_11709_regression {
+    use std::num::NonZeroI32;
+
+    struct Date {
+        value: NonZeroI32,
+    }
+
+    impl Date {
+        const unsafe fn __from_ordinal_date_unchecked(year: i32, ordinal: u16) -> Self {
+            Self {
+                // Safety: The caller must guarantee that `ordinal` is not zero.
+                value: unsafe { NonZeroI32::new_unchecked((year << 9) | ordinal as i32) },
+            }
+        }
+
+        const fn into_julian_day_just_make_this_line_longer(self) -> i32 {
+            42
+        }
+    }
+
+    /// The Julian day of the Unix epoch.
+    // SAFETY: fail ONLY if `accept-comment-above-attribute = false`
+    #[allow(unsafe_code)]
+    const UNIX_EPOCH_JULIAN_DAY: i32 =
+        unsafe { Date::__from_ordinal_date_unchecked(1970, 1) }.into_julian_day_just_make_this_line_longer();
+    //~[disabled]^ undocumented_unsafe_blocks
+}
+
 fn main() {}