Auto merge of #11375 - J-ZhengLi:issue11246, r=Centri3

fix fp when [`undocumented_unsafe_blocks`] not able to detect comment on globally defined const/static variables

fixes: #11246

changelog: fix detection on global variables for [`undocumented_unsafe_blocks`]

clippy_lints/src/undocumented_unsafe_blocks.rs

@@ -341,44 +341,21 @@ fn block_parents_have_safety_comment(
     id: hir::HirId,
 ) -> bool {
     if let Some(node) = get_parent_node(cx.tcx, id) {
-        return match node {
+        let (span, hir_id) = match node {
-            Node::Expr(expr) => {
+            Node::Expr(expr) => match get_parent_node(cx.tcx, expr.hir_id) {
-                if let Some(
+                Some(Node::Local(hir::Local { span, hir_id, .. })) => (*span, *hir_id),
-                    Node::Local(hir::Local { span, .. })
+                Some(Node::Item(hir::Item {
-                    | Node::Item(hir::Item {
+                    kind: hir::ItemKind::Const(..) | ItemKind::Static(..),
-                        kind: hir::ItemKind::Const(..) | ItemKind::Static(..),
+                    span,
-                        span,
+                    owner_id,
-                        ..
+                    ..
-                    }),
+                })) => (*span, cx.tcx.hir().local_def_id_to_hir_id(owner_id.def_id)),
-                ) = get_parent_node(cx.tcx, expr.hir_id)
+                _ => {
-                {
+                    if is_branchy(expr) {
-                    let hir_id = match get_parent_node(cx.tcx, expr.hir_id) {
+                        return false;
-                        Some(Node::Local(hir::Local { hir_id, .. })) => *hir_id,
+                    }
-                        Some(Node::Item(hir::Item { owner_id, .. })) => {
+                    (expr.span, expr.hir_id)
-                            cx.tcx.hir().local_def_id_to_hir_id(owner_id.def_id)
+                },
-                        },
-                        _ => unreachable!(),
-                    };
-
-                    // if unsafe block is part of a let/const/static statement,
-                    // and accept_comment_above_statement is set to true
-                    // we accept the safety comment in the line the precedes this statement.
-                    accept_comment_above_statement
-                        && span_with_attrs_in_body_has_safety_comment(
-                            cx,
-                            *span,
-                            hir_id,
-                            accept_comment_above_attributes,
-                        )
-                } else {
-                    !is_branchy(expr)
-                        && span_with_attrs_in_body_has_safety_comment(
-                            cx,
-                            expr.span,
-                            expr.hir_id,
-                            accept_comment_above_attributes,
-                        )
-                }
             },
             Node::Stmt(hir::Stmt {
                 kind:
@@ -387,28 +364,27 @@ fn block_parents_have_safety_comment(
                     | hir::StmtKind::Semi(hir::Expr { span, hir_id, .. }),
                 ..
             })
-            | Node::Local(hir::Local { span, hir_id, .. }) => {
+            | Node::Local(hir::Local { span, hir_id, .. }) => (*span, *hir_id),
-                span_with_attrs_in_body_has_safety_comment(cx, *span, *hir_id, accept_comment_above_attributes)
-            },
             Node::Item(hir::Item {
                 kind: hir::ItemKind::Const(..) | ItemKind::Static(..),
                 span,
                 owner_id,
                 ..
-            }) => span_with_attrs_in_body_has_safety_comment(
+            }) => (*span, cx.tcx.hir().local_def_id_to_hir_id(owner_id.def_id)),
-                cx,
+            _ => return false,
-                *span,
-                cx.tcx.hir().local_def_id_to_hir_id(owner_id.def_id),
-                accept_comment_above_attributes,
-            ),
-            _ => false,
         };
+        // if unsafe block is part of a let/const/static statement,
+        // and accept_comment_above_statement is set to true
+        // we accept the safety comment in the line the precedes this statement.
+        accept_comment_above_statement
+            && span_with_attrs_has_safety_comment(cx, span, hir_id, accept_comment_above_attributes)
+    } else {
+        false
     }
-    false
 }
 
 /// Extends `span` to also include its attributes, then checks if that span has a safety comment.
-fn span_with_attrs_in_body_has_safety_comment(
+fn span_with_attrs_has_safety_comment(
     cx: &LateContext<'_>,
     span: Span,
     hir_id: HirId,
@@ -420,7 +396,7 @@ fn span_with_attrs_in_body_has_safety_comment(
         span
     };
 
-    span_in_body_has_safety_comment(cx, span)
+    span_has_safety_comment(cx, span)
 }
 
 /// Checks if an expression is "branchy", e.g. loop, match/if/etc.
@@ -444,7 +420,7 @@ fn block_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
     matches!(
         span_from_macro_expansion_has_safety_comment(cx, span),
         HasSafetyComment::Yes(_)
-    ) || span_in_body_has_safety_comment(cx, span)
+    ) || span_has_safety_comment(cx, span)
 }
 
 fn include_attrs_in_span(cx: &LateContext<'_>, hir_id: HirId, span: Span) -> Span {
@@ -639,29 +615,36 @@ fn get_body_search_span(cx: &LateContext<'_>) -> Option<Span> {
     let body = cx.enclosing_body?;
     let map = cx.tcx.hir();
     let mut span = map.body(body).value.span;
+    let mut maybe_global_var = false;
     for (_, node) in map.parent_iter(body.hir_id) {
         match node {
             Node::Expr(e) => span = e.span,
-            Node::Block(_)
+            Node::Block(_) | Node::Arm(_) | Node::Stmt(_) | Node::Local(_) => (),
-            | Node::Arm(_)
+            Node::Item(hir::Item {
-            | Node::Stmt(_)
-            | Node::Local(_)
-            | Node::Item(hir::Item {
                 kind: hir::ItemKind::Const(..) | ItemKind::Static(..),
                 ..
-            }) => (),
+            }) => maybe_global_var = true,
+            Node::Item(hir::Item {
+                kind: hir::ItemKind::Mod(_),
+                span: item_span,
+                ..
+            }) => {
+                span = *item_span;
+                break;
+            },
+            Node::Crate(mod_) if maybe_global_var => {
+                span = mod_.spans.inner_span;
+            },
             _ => break,
         }
     }
     Some(span)
 }
 
-fn span_in_body_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
+fn span_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
     let source_map = cx.sess().source_map();
     let ctxt = span.ctxt();
-    if ctxt == SyntaxContext::root()
+    if ctxt.is_root() && let Some(search_span) = get_body_search_span(cx) {
-        && let Some(search_span) = get_body_search_span(cx)
-    {
         if let Ok(unsafe_line) = source_map.lookup_line(span.lo())
             && let Some(body_span) = walk_span_to_context(search_span, SyntaxContext::root())
             && let Ok(body_line) = source_map.lookup_line(body_span.lo())

tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs

@@ -564,4 +564,18 @@ fn issue_8679<T: Copy>() {
     unsafe {}
 }
 
+mod issue_11246 {
+    // Safety: foo
+    const _: () = unsafe {};
+
+    // Safety: A safety comment
+    const FOO: () = unsafe {};
+
+    // Safety: bar
+    static BAR: u8 = unsafe { 0 };
+}
+
+// Safety: Another safety comment
+const FOO: () = unsafe {};
+
 fn main() {}