Skip to content

Commit

Permalink
fix [undocumented_unsafe_blocks] FP with trait/impl items
Browse files Browse the repository at this point in the history
  • Loading branch information
J-ZhengLi committed Apr 14, 2024
1 parent 62fd1d5 commit 6992351
Show file tree
Hide file tree
Showing 4 changed files with 146 additions and 37 deletions.
83 changes: 48 additions & 35 deletions clippy_lints/src/undocumented_unsafe_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,12 +343,13 @@ fn block_parents_have_safety_comment(
let (span, hir_id) = match cx.tcx.parent_hir_node(id) {
Node::Expr(expr) => match cx.tcx.parent_hir_node(expr.hir_id) {
Node::LetStmt(hir::LetStmt { span, hir_id, .. }) => (*span, *hir_id),
Node::Item(hir::Item {
kind: ItemKind::Const(..) | ItemKind::Static(..),
span,
owner_id,
..
}) => (*span, cx.tcx.local_def_id_to_hir_id(owner_id.def_id)),

node if let Some((span, hir_id)) = span_and_hid_of_item_alike_node(&node)
&& is_const_or_static(&node) =>
{
(span, hir_id)
},

_ => {
if is_branchy(expr) {
return false;
Expand All @@ -364,12 +365,13 @@ fn block_parents_have_safety_comment(
..
})
| Node::LetStmt(hir::LetStmt { span, hir_id, .. }) => (*span, *hir_id),
Node::Item(hir::Item {
kind: ItemKind::Const(..) | ItemKind::Static(..),
span,
owner_id,
..
}) => (*span, cx.tcx.local_def_id_to_hir_id(owner_id.def_id)),

node if let Some((span, hir_id)) = span_and_hid_of_item_alike_node(&node)
&& is_const_or_static(&node) =>
{
(span, hir_id)
},

_ => return false,
};
// if unsafe block is part of a let/const/static statement,
Expand Down Expand Up @@ -597,32 +599,18 @@ fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span

fn get_body_search_span(cx: &LateContext<'_>) -> Option<Span> {
let body = cx.enclosing_body?;
let map = cx.tcx.hir();
let mut span = map.body(body).value.span;
let mut maybe_global_var = false;
for (_, node) in map.parent_iter(body.hir_id) {
match node {
Node::Expr(e) => span = e.span,
Node::Block(_) | Node::Arm(_) | Node::Stmt(_) | Node::LetStmt(_) => (),
Node::Item(hir::Item {
kind: ItemKind::Const(..) | ItemKind::Static(..),
..
}) => maybe_global_var = true,
Node::Item(hir::Item {
kind: ItemKind::Mod(_),
span: item_span,
..
}) => {
span = *item_span;
break;
for (_, parent_node) in cx.tcx.hir().parent_iter(body.hir_id) {
match parent_node {
Node::Crate(mod_) => return Some(mod_.spans.inner_span),
node if let Some((span, _)) = span_and_hid_of_item_alike_node(&node)
&& !is_const_or_static(&node) =>
{
return Some(span);
},
Node::Crate(mod_) if maybe_global_var => {
span = mod_.spans.inner_span;
},
_ => break,
_ => {},
}
}
Some(span)
None
}

fn span_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
Expand Down Expand Up @@ -711,3 +699,28 @@ fn text_has_safety_comment(src: &str, line_starts: &[RelativeBytePos], start_pos
}
}
}

fn span_and_hid_of_item_alike_node(node: &Node<'_>) -> Option<(Span, HirId)> {
match node {
Node::Item(item) => Some((item.span, item.owner_id.into())),
Node::TraitItem(ti) => Some((ti.span, ti.owner_id.into())),
Node::ImplItem(ii) => Some((ii.span, ii.owner_id.into())),
_ => None,
}
}

fn is_const_or_static(node: &Node<'_>) -> bool {
matches!(
node,
Node::Item(hir::Item {
kind: ItemKind::Const(..) | ItemKind::Static(..),
..
}) | Node::ImplItem(hir::ImplItem {
kind: hir::ImplItemKind::Const(..),
..
}) | Node::TraitItem(hir::TraitItem {
kind: hir::TraitItemKind::Const(..),
..
})
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -312,5 +312,37 @@ LL | let bar = unsafe {};
|
= help: consider adding a safety comment on the preceding line

error: aborting due to 35 previous errors
error: unsafe block missing a safety comment
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:593:52
|
LL | const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 };
| ^^^^^^^^^^^^
|
= help: consider adding a safety comment on the preceding line

error: unsafe block missing a safety comment
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:602:41
|
LL | const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
| ^^^^^^^^^^^^
|
= help: consider adding a safety comment on the preceding line

error: unsafe block missing a safety comment
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:612:42
|
LL | const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 };
| ^^^^^^^^^^^^
|
= help: consider adding a safety comment on the preceding line

error: unsafe block missing a safety comment
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:617:40
|
LL | const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 };
| ^^^^^^^^^^^^
|
= help: consider adding a safety comment on the preceding line

error: aborting due to 39 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -392,5 +392,37 @@ LL | unsafe {}
|
= help: consider adding a safety comment on the preceding line

error: aborting due to 45 previous errors
error: unsafe block missing a safety comment
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:593:52
|
LL | const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 };
| ^^^^^^^^^^^^
|
= help: consider adding a safety comment on the preceding line

error: unsafe block missing a safety comment
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:602:41
|
LL | const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
| ^^^^^^^^^^^^
|
= help: consider adding a safety comment on the preceding line

error: unsafe block missing a safety comment
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:612:42
|
LL | const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 };
| ^^^^^^^^^^^^
|
= help: consider adding a safety comment on the preceding line

error: unsafe block missing a safety comment
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:617:40
|
LL | const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 };
| ^^^^^^^^^^^^
|
= help: consider adding a safety comment on the preceding line

error: aborting due to 49 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -587,4 +587,36 @@ mod issue_11246 {
// Safety: Another safety comment
const FOO: () = unsafe {};

// trait items and impl items
mod issue_11709 {
trait MyTrait {
const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 };
//~^ ERROR: unsafe block missing a safety comment

// SAFETY: safe
const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 1 };

// SAFETY: unrelated
unsafe fn unsafe_fn() {}

const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
//~^ ERROR: unsafe block missing a safety comment
}

struct UnsafeStruct;

impl MyTrait for UnsafeStruct {
// SAFETY: safe in this impl
const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 2 };

const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 };
//~^ ERROR: unsafe block missing a safety comment
}

impl UnsafeStruct {
const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 };
//~^ ERROR: unsafe block missing a safety comment
}
}

fn main() {}

0 comments on commit 6992351

Please sign in to comment.