Skip to content

Commit ace7a0a

Browse files
committed
fix [undocumented_unsafe_blocks] FP with trait/impl items
1 parent 62fd1d5 commit ace7a0a

File tree

4 files changed

+146
-37
lines changed

4 files changed

+146
-37
lines changed

clippy_lints/src/undocumented_unsafe_blocks.rs

Lines changed: 48 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -343,12 +343,13 @@ fn block_parents_have_safety_comment(
343343
let (span, hir_id) = match cx.tcx.parent_hir_node(id) {
344344
Node::Expr(expr) => match cx.tcx.parent_hir_node(expr.hir_id) {
345345
Node::LetStmt(hir::LetStmt { span, hir_id, .. }) => (*span, *hir_id),
346-
Node::Item(hir::Item {
347-
kind: ItemKind::Const(..) | ItemKind::Static(..),
348-
span,
349-
owner_id,
350-
..
351-
}) => (*span, cx.tcx.local_def_id_to_hir_id(owner_id.def_id)),
346+
347+
node if let Some((span, hir_id)) = span_and_hid_of_item_alike_node(&node)
348+
&& is_const_or_static(&node) =>
349+
{
350+
(span, hir_id)
351+
},
352+
352353
_ => {
353354
if is_branchy(expr) {
354355
return false;
@@ -364,12 +365,13 @@ fn block_parents_have_safety_comment(
364365
..
365366
})
366367
| Node::LetStmt(hir::LetStmt { span, hir_id, .. }) => (*span, *hir_id),
367-
Node::Item(hir::Item {
368-
kind: ItemKind::Const(..) | ItemKind::Static(..),
369-
span,
370-
owner_id,
371-
..
372-
}) => (*span, cx.tcx.local_def_id_to_hir_id(owner_id.def_id)),
368+
369+
node if let Some((span, hir_id)) = span_and_hid_of_item_alike_node(&node)
370+
&& is_const_or_static(&node) =>
371+
{
372+
(span, hir_id)
373+
},
374+
373375
_ => return false,
374376
};
375377
// if unsafe block is part of a let/const/static statement,
@@ -597,32 +599,18 @@ fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span
597599

598600
fn get_body_search_span(cx: &LateContext<'_>) -> Option<Span> {
599601
let body = cx.enclosing_body?;
600-
let map = cx.tcx.hir();
601-
let mut span = map.body(body).value.span;
602-
let mut maybe_global_var = false;
603-
for (_, node) in map.parent_iter(body.hir_id) {
604-
match node {
605-
Node::Expr(e) => span = e.span,
606-
Node::Block(_) | Node::Arm(_) | Node::Stmt(_) | Node::LetStmt(_) => (),
607-
Node::Item(hir::Item {
608-
kind: ItemKind::Const(..) | ItemKind::Static(..),
609-
..
610-
}) => maybe_global_var = true,
611-
Node::Item(hir::Item {
612-
kind: ItemKind::Mod(_),
613-
span: item_span,
614-
..
615-
}) => {
616-
span = *item_span;
617-
break;
602+
for (_, parent_node) in cx.tcx.hir().parent_iter(body.hir_id) {
603+
match parent_node {
604+
Node::Crate(mod_) => return Some(mod_.spans.inner_span),
605+
node if let Some((span, _)) = span_and_hid_of_item_alike_node(&node)
606+
&& !is_const_or_static(&node) =>
607+
{
608+
return Some(span);
618609
},
619-
Node::Crate(mod_) if maybe_global_var => {
620-
span = mod_.spans.inner_span;
621-
},
622-
_ => break,
610+
_ => {},
623611
}
624612
}
625-
Some(span)
613+
None
626614
}
627615

628616
fn span_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
@@ -711,3 +699,28 @@ fn text_has_safety_comment(src: &str, line_starts: &[RelativeBytePos], start_pos
711699
}
712700
}
713701
}
702+
703+
fn span_and_hid_of_item_alike_node(node: &Node<'_>) -> Option<(Span, HirId)> {
704+
match node {
705+
Node::Item(item) => Some((item.span, item.owner_id.into())),
706+
Node::TraitItem(ti) => Some((ti.span, ti.owner_id.into())),
707+
Node::ImplItem(ii) => Some((ii.span, ii.owner_id.into())),
708+
_ => return None,
709+
}
710+
}
711+
712+
fn is_const_or_static(node: &Node<'_>) -> bool {
713+
matches!(
714+
node,
715+
Node::Item(hir::Item {
716+
kind: ItemKind::Const(..) | ItemKind::Static(..),
717+
..
718+
}) | Node::ImplItem(hir::ImplItem {
719+
kind: hir::ImplItemKind::Const(..),
720+
..
721+
}) | Node::TraitItem(hir::TraitItem {
722+
kind: hir::TraitItemKind::Const(..),
723+
..
724+
})
725+
)
726+
}

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,5 +312,37 @@ LL | let bar = unsafe {};
312312
|
313313
= help: consider adding a safety comment on the preceding line
314314

315-
error: aborting due to 35 previous errors
315+
error: unsafe block missing a safety comment
316+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:593:52
317+
|
318+
LL | const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 };
319+
| ^^^^^^^^^^^^
320+
|
321+
= help: consider adding a safety comment on the preceding line
322+
323+
error: unsafe block missing a safety comment
324+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:602:41
325+
|
326+
LL | const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
327+
| ^^^^^^^^^^^^
328+
|
329+
= help: consider adding a safety comment on the preceding line
330+
331+
error: unsafe block missing a safety comment
332+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:612:42
333+
|
334+
LL | const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 };
335+
| ^^^^^^^^^^^^
336+
|
337+
= help: consider adding a safety comment on the preceding line
338+
339+
error: unsafe block missing a safety comment
340+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:617:40
341+
|
342+
LL | const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 };
343+
| ^^^^^^^^^^^^
344+
|
345+
= help: consider adding a safety comment on the preceding line
346+
347+
error: aborting due to 39 previous errors
316348

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,5 +392,37 @@ LL | unsafe {}
392392
|
393393
= help: consider adding a safety comment on the preceding line
394394

395-
error: aborting due to 45 previous errors
395+
error: unsafe block missing a safety comment
396+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:593:52
397+
|
398+
LL | const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 };
399+
| ^^^^^^^^^^^^
400+
|
401+
= help: consider adding a safety comment on the preceding line
402+
403+
error: unsafe block missing a safety comment
404+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:602:41
405+
|
406+
LL | const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
407+
| ^^^^^^^^^^^^
408+
|
409+
= help: consider adding a safety comment on the preceding line
410+
411+
error: unsafe block missing a safety comment
412+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:612:42
413+
|
414+
LL | const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 };
415+
| ^^^^^^^^^^^^
416+
|
417+
= help: consider adding a safety comment on the preceding line
418+
419+
error: unsafe block missing a safety comment
420+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:617:40
421+
|
422+
LL | const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 };
423+
| ^^^^^^^^^^^^
424+
|
425+
= help: consider adding a safety comment on the preceding line
426+
427+
error: aborting due to 49 previous errors
396428

tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,4 +587,36 @@ mod issue_11246 {
587587
// Safety: Another safety comment
588588
const FOO: () = unsafe {};
589589

590+
// trait items and impl items
591+
mod issue_11709 {
592+
trait MyTrait {
593+
const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 };
594+
//~^ ERROR: unsafe block missing a safety comment
595+
596+
// SAFETY: safe
597+
const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
598+
599+
// SAFETY: unrelated
600+
unsafe fn unsafe_fn() {}
601+
602+
const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
603+
//~^ ERROR: unsafe block missing a safety comment
604+
}
605+
606+
struct UnsafeStruct;
607+
608+
impl MyTrait for UnsafeStruct {
609+
// SAFETY: safe in this impl
610+
const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 2 };
611+
612+
const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 };
613+
//~^ ERROR: unsafe block missing a safety comment
614+
}
615+
616+
impl UnsafeStruct {
617+
const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 };
618+
//~^ ERROR: unsafe block missing a safety comment
619+
}
620+
}
621+
590622
fn main() {}

0 commit comments

Comments
 (0)