Skip to content

Commit 710d697

Browse files
J-ZhengLiprofetia
J-ZhengLi
authored and
profetia
committed
fix [undocumented_unsafe_blocks] FP with trait/impl items
1 parent 998c780 commit 710d697

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

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

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

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