Skip to content

Commit e54c9be

Browse files
J-ZhengLiprofetia
authored andcommitted
fix [undocumented_unsafe_blocks] FP with trait/impl items
1 parent 0f20a12 commit e54c9be

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
@@ -350,12 +350,13 @@ fn block_parents_have_safety_comment(
350350
let (span, hir_id) = match cx.tcx.parent_hir_node(id) {
351351
Node::Expr(expr) => match cx.tcx.parent_hir_node(expr.hir_id) {
352352
Node::LetStmt(hir::LetStmt { span, hir_id, .. }) => (*span, *hir_id),
353-
Node::Item(hir::Item {
354-
kind: ItemKind::Const(..) | ItemKind::Static(..),
355-
span,
356-
owner_id,
357-
..
358-
}) => (*span, cx.tcx.local_def_id_to_hir_id(owner_id.def_id)),
353+
354+
node if let Some((span, hir_id)) = span_and_hid_of_item_alike_node(&node)
355+
&& is_const_or_static(&node) =>
356+
{
357+
(span, hir_id)
358+
},
359+
359360
_ => {
360361
if is_branchy(expr) {
361362
return false;
@@ -371,12 +372,13 @@ fn block_parents_have_safety_comment(
371372
..
372373
})
373374
| Node::LetStmt(hir::LetStmt { span, hir_id, .. }) => (*span, *hir_id),
374-
Node::Item(hir::Item {
375-
kind: ItemKind::Const(..) | ItemKind::Static(..),
376-
span,
377-
owner_id,
378-
..
379-
}) => (*span, cx.tcx.local_def_id_to_hir_id(owner_id.def_id)),
375+
376+
node if let Some((span, hir_id)) = span_and_hid_of_item_alike_node(&node)
377+
&& is_const_or_static(&node) =>
378+
{
379+
(span, hir_id)
380+
},
381+
380382
_ => return false,
381383
};
382384
// if unsafe block is part of a let/const/static statement,
@@ -604,32 +606,18 @@ fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span
604606

605607
fn get_body_search_span(cx: &LateContext<'_>) -> Option<Span> {
606608
let body = cx.enclosing_body?;
607-
let map = cx.tcx.hir();
608-
let mut span = map.body(body).value.span;
609-
let mut maybe_global_var = false;
610-
for (_, node) in map.parent_iter(body.hir_id) {
611-
match node {
612-
Node::Expr(e) => span = e.span,
613-
Node::Block(_) | Node::Arm(_) | Node::Stmt(_) | Node::LetStmt(_) => (),
614-
Node::Item(hir::Item {
615-
kind: ItemKind::Const(..) | ItemKind::Static(..),
616-
..
617-
}) => maybe_global_var = true,
618-
Node::Item(hir::Item {
619-
kind: ItemKind::Mod(_),
620-
span: item_span,
621-
..
622-
}) => {
623-
span = *item_span;
624-
break;
609+
for (_, parent_node) in cx.tcx.hir().parent_iter(body.hir_id) {
610+
match parent_node {
611+
Node::Crate(mod_) => return Some(mod_.spans.inner_span),
612+
node if let Some((span, _)) = span_and_hid_of_item_alike_node(&node)
613+
&& !is_const_or_static(&node) =>
614+
{
615+
return Some(span);
625616
},
626-
Node::Crate(mod_) if maybe_global_var => {
627-
span = mod_.spans.inner_span;
628-
},
629-
_ => break,
617+
_ => {},
630618
}
631619
}
632-
Some(span)
620+
None
633621
}
634622

635623
fn span_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
@@ -718,3 +706,28 @@ fn text_has_safety_comment(src: &str, line_starts: &[RelativeBytePos], start_pos
718706
}
719707
}
720708
}
709+
710+
fn span_and_hid_of_item_alike_node(node: &Node<'_>) -> Option<(Span, HirId)> {
711+
match node {
712+
Node::Item(item) => Some((item.span, item.owner_id.into())),
713+
Node::TraitItem(ti) => Some((ti.span, ti.owner_id.into())),
714+
Node::ImplItem(ii) => Some((ii.span, ii.owner_id.into())),
715+
_ => None,
716+
}
717+
}
718+
719+
fn is_const_or_static(node: &Node<'_>) -> bool {
720+
matches!(
721+
node,
722+
Node::Item(hir::Item {
723+
kind: ItemKind::Const(..) | ItemKind::Static(..),
724+
..
725+
}) | Node::ImplItem(hir::ImplItem {
726+
kind: hir::ImplItemKind::Const(..),
727+
..
728+
}) | Node::TraitItem(hir::TraitItem {
729+
kind: hir::TraitItemKind::Const(..),
730+
..
731+
})
732+
)
733+
}

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
@@ -310,5 +310,45 @@ LL | let bar = unsafe {};
310310
|
311311
= help: consider adding a safety comment on the preceding line
312312

313-
error: aborting due to 35 previous errors
313+
error: unsafe block missing a safety comment
314+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:592:52
315+
|
316+
LL | const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 };
317+
| ^^^^^^^^^^^^
318+
|
319+
= help: consider adding a safety comment on the preceding line
320+
321+
error: unsafe block missing a safety comment
322+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:601:41
323+
|
324+
LL | const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
325+
| ^^^^^^^^^^^^
326+
|
327+
= help: consider adding a safety comment on the preceding line
328+
329+
error: unsafe block missing a safety comment
330+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:611:42
331+
|
332+
LL | const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 };
333+
| ^^^^^^^^^^^^
334+
|
335+
= help: consider adding a safety comment on the preceding line
336+
337+
error: unsafe block missing a safety comment
338+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:616:40
339+
|
340+
LL | const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 };
341+
| ^^^^^^^^^^^^
342+
|
343+
= help: consider adding a safety comment on the preceding line
344+
345+
error: unsafe block missing a safety comment
346+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:627:9
347+
|
348+
LL | unsafe { here_is_another_variable_with_long_name };
349+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
350+
|
351+
= help: consider adding a safety comment on the preceding line
352+
353+
error: aborting due to 40 previous errors
314354

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
@@ -390,5 +390,45 @@ LL | unsafe {}
390390
|
391391
= help: consider adding a safety comment on the preceding line
392392

393-
error: aborting due to 45 previous errors
393+
error: unsafe block missing a safety comment
394+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:592:52
395+
|
396+
LL | const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 };
397+
| ^^^^^^^^^^^^
398+
|
399+
= help: consider adding a safety comment on the preceding line
400+
401+
error: unsafe block missing a safety comment
402+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:601:41
403+
|
404+
LL | const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
405+
| ^^^^^^^^^^^^
406+
|
407+
= help: consider adding a safety comment on the preceding line
408+
409+
error: unsafe block missing a safety comment
410+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:611:42
411+
|
412+
LL | const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 };
413+
| ^^^^^^^^^^^^
414+
|
415+
= help: consider adding a safety comment on the preceding line
416+
417+
error: unsafe block missing a safety comment
418+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:616:40
419+
|
420+
LL | const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 };
421+
| ^^^^^^^^^^^^
422+
|
423+
= help: consider adding a safety comment on the preceding line
424+
425+
error: unsafe block missing a safety comment
426+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:627:9
427+
|
428+
LL | unsafe { here_is_another_variable_with_long_name };
429+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
430+
|
431+
= help: consider adding a safety comment on the preceding line
432+
433+
error: aborting due to 50 previous errors
394434

tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs

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

635+
// trait items and impl items
636+
mod issue_11709 {
637+
trait MyTrait {
638+
const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 };
639+
//~^ ERROR: unsafe block missing a safety comment
640+
641+
// SAFETY: safe
642+
const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
643+
644+
// SAFETY: unrelated
645+
unsafe fn unsafe_fn() {}
646+
647+
const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
648+
//~^ ERROR: unsafe block missing a safety comment
649+
}
650+
651+
struct UnsafeStruct;
652+
653+
impl MyTrait for UnsafeStruct {
654+
// SAFETY: safe in this impl
655+
const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 2 };
656+
657+
const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 };
658+
//~^ ERROR: unsafe block missing a safety comment
659+
}
660+
661+
impl UnsafeStruct {
662+
const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 };
663+
//~^ ERROR: unsafe block missing a safety comment
664+
}
665+
}
666+
667+
fn issue_13024() {
668+
let mut just_a_simple_variable_with_a_very_long_name_that_has_seventy_eight_characters = 0;
669+
let here_is_another_variable_with_long_name = 100;
670+
671+
// SAFETY: good
672+
just_a_simple_variable_with_a_very_long_name_that_has_seventy_eight_characters =
673+
unsafe { here_is_another_variable_with_long_name };
674+
}
675+
635676
fn main() {}

0 commit comments

Comments
 (0)