Skip to content

Commit 6a10bf2

Browse files
J-ZhengLiJ-ZhengLi
committed
fix [undocumented_unsafe_blocks] FP with trait/impl items
1 parent 87f8a5b commit 6a10bf2

File tree

4 files changed

+171
-37
lines changed

4 files changed

+171
-37
lines changed

‎clippy_lints/src/undocumented_unsafe_blocks.rs‎

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

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

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

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

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,5 +312,45 @@ 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:592: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:601: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:611: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:616: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: unsafe block missing a safety comment
348+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:627:9
349+
|
350+
LL | unsafe { here_is_another_variable_with_long_name };
351+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
352+
|
353+
= help: consider adding a safety comment on the preceding line
354+
355+
error: aborting due to 40 previous errors
316356

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

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,5 +392,45 @@ 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:592: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:601: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:611: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:616: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: unsafe block missing a safety comment
428+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:627:9
429+
|
430+
LL | unsafe { here_is_another_variable_with_long_name };
431+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
432+
|
433+
= help: consider adding a safety comment on the preceding line
434+
435+
error: aborting due to 50 previous errors
396436

‎tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs‎

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

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

0 commit comments

Comments
 (0)