Skip to content

Commit fc4d88b

Browse files
J-ZhengLiJ-ZhengLi
committed
fix [undocumented_unsafe_blocks] FP with trait/impl items
1 parent 62fd1d5 commit fc4d88b

File tree

4 files changed

+172
-26
lines changed

4 files changed

+172
-26
lines changed

‎clippy_lints/src/undocumented_unsafe_blocks.rs

Lines changed: 74 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -343,12 +343,13 @@ fn block_parents_have_safety_comment(
343343
let (span, hir_id) = match cx.tcx.parent_hir_node(id) {
344344
Node::Expr(expr) => match cx.tcx.parent_hir_node(expr.hir_id) {
345345
Node::LetStmt(hir::LetStmt { span, hir_id, .. }) => (*span, *hir_id),
346-
Node::Item(hir::Item {
347-
kind: ItemKind::Const(..) | ItemKind::Static(..),
348-
span,
349-
owner_id,
350-
..
351-
}) => (*span, cx.tcx.local_def_id_to_hir_id(owner_id.def_id)),
346+
347+
node if let Some(item) = ItemAlike::hir(&node)
348+
&& item.is_const_or_static() =>
349+
{
350+
(item.span, cx.tcx.local_def_id_to_hir_id(item.owner_id.def_id))
351+
},
352+
352353
_ => {
353354
if is_branchy(expr) {
354355
return false;
@@ -364,12 +365,13 @@ fn block_parents_have_safety_comment(
364365
..
365366
})
366367
| Node::LetStmt(hir::LetStmt { span, hir_id, .. }) => (*span, *hir_id),
367-
Node::Item(hir::Item {
368-
kind: ItemKind::Const(..) | ItemKind::Static(..),
369-
span,
370-
owner_id,
371-
..
372-
}) => (*span, cx.tcx.local_def_id_to_hir_id(owner_id.def_id)),
368+
369+
node if let Some(item) = ItemAlike::hir(&node)
370+
&& item.is_const_or_static() =>
371+
{
372+
(item.span, cx.tcx.local_def_id_to_hir_id(item.owner_id.def_id))
373+
},
374+
373375
_ => return false,
374376
};
375377
// if unsafe block is part of a let/const/static statement,
@@ -604,21 +606,19 @@ fn get_body_search_span(cx: &LateContext<'_>) -> Option<Span> {
604606
match node {
605607
Node::Expr(e) => span = e.span,
606608
Node::Block(_) | Node::Arm(_) | Node::Stmt(_) | Node::LetStmt(_) => (),
607-
Node::Item(hir::Item {
608-
kind: ItemKind::Const(..) | ItemKind::Static(..),
609-
..
610-
}) => maybe_global_var = true,
611-
Node::Item(hir::Item {
612-
kind: ItemKind::Mod(_),
613-
span: item_span,
614-
..
615-
}) => {
616-
span = *item_span;
617-
break;
618-
},
619609
Node::Crate(mod_) if maybe_global_var => {
620610
span = mod_.spans.inner_span;
621611
},
612+
613+
node if let Some(item) = ItemAlike::hir(&node) => {
614+
if item.is_const_or_static() {
615+
maybe_global_var = true;
616+
} else if let ItemAlikeKind::Item(ItemKind::Mod(_)) = item.kind {
617+
span = item.span;
618+
break;
619+
}
620+
},
621+
622622
_ => break,
623623
}
624624
}
@@ -711,3 +711,53 @@ fn text_has_safety_comment(src: &str, line_starts: &[RelativeBytePos], start_pos
711711
}
712712
}
713713
}
714+
715+
macro_rules! item_like_kind_conversion {
716+
($varient:ident, $ty_convert_from:ty, $lft:lifetime) => {
717+
impl<$lft> ::core::convert::From<$ty_convert_from> for ItemAlikeKind<$lft> {
718+
fn from(value: $ty_convert_from) -> Self {
719+
Self::$varient(value)
720+
}
721+
}
722+
};
723+
}
724+
725+
enum ItemAlikeKind<'hir> {
726+
Item(ItemKind<'hir>),
727+
TraitItem(hir::TraitItemKind<'hir>),
728+
ImplItem(hir::ImplItemKind<'hir>),
729+
}
730+
731+
item_like_kind_conversion!(Item, ItemKind<'hir>, 'hir);
732+
item_like_kind_conversion!(TraitItem, hir::TraitItemKind<'hir>, 'hir);
733+
item_like_kind_conversion!(ImplItem, hir::ImplItemKind<'hir>, 'hir);
734+
735+
/// Representing the hir nodes that are item alike.
736+
///
737+
/// Note this does not includes [`Node::ForeignItem`] for now.
738+
struct ItemAlike<'hir> {
739+
owner_id: hir::OwnerId,
740+
kind: ItemAlikeKind<'hir>,
741+
span: Span,
742+
}
743+
744+
impl<'hir> ItemAlike<'hir> {
745+
fn hir(node: &'hir Node<'hir>) -> Option<Self> {
746+
let (owner_id, kind, span) = match node {
747+
Node::Item(item) => (item.owner_id, item.kind.into(), item.span),
748+
Node::TraitItem(ti) => (ti.owner_id, ti.kind.into(), ti.span),
749+
Node::ImplItem(ii) => (ii.owner_id, ii.kind.into(), ii.span),
750+
_ => return None,
751+
};
752+
Some(Self { owner_id, kind, span })
753+
}
754+
755+
fn is_const_or_static(&self) -> bool {
756+
matches!(
757+
self.kind,
758+
ItemAlikeKind::Item(ItemKind::Const(..) | ItemKind::Static(..))
759+
| ItemAlikeKind::ImplItem(hir::ImplItemKind::Const(..))
760+
| ItemAlikeKind::TraitItem(hir::TraitItemKind::Const(..))
761+
)
762+
}
763+
}

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,5 +312,37 @@ 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:594:52
317+
|
318+
LL | const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 }; // lint?
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:602: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:613:42
333+
|
334+
LL | const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 }; // lint?
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:617: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: aborting due to 39 previous errors
316348

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,5 +392,37 @@ 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:594:52
397+
|
398+
LL | const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 }; // lint?
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:602: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:613:42
413+
|
414+
LL | const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 }; // lint?
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:617: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: aborting due to 49 previous errors
396428

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

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

590+
// trait items and impl items
591+
mod issue_11709 {
592+
trait MyTrait {
593+
//~v ERROR: unsafe block missing a safety comment
594+
const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 }; // lint?
595+
596+
// SAFETY: safe
597+
const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
598+
599+
// SAFETY: unrelated
600+
unsafe fn unsafe_fn() {}
601+
602+
const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 };
603+
//~^ ERROR: unsafe block missing a safety comment
604+
}
605+
606+
struct UnsafeStruct;
607+
608+
impl MyTrait for UnsafeStruct {
609+
// SAFETY: safe in this impl
610+
const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 2 };
611+
612+
//~v ERROR: unsafe block missing a safety comment
613+
const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 }; // lint?
614+
}
615+
616+
impl UnsafeStruct {
617+
const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 };
618+
//~^ ERROR: unsafe block missing a safety comment
619+
}
620+
}
621+
590622
fn main() {}

0 commit comments

Comments
 (0)