Skip to content

Commit 78e2f90

Browse files
J-ZhengLiprofetia
J-ZhengLi
authored and
profetia
committed
fix [undocumented_unsafe_blocks] FP with trait/impl items
1 parent 4129f5c commit 78e2f90

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

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

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

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

‎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)