fix: undocumented_unsafe_blocks FP on trait/impl items (#13888)
fixes #11709 Continuation of #12672. r? @Alexendoo if you don't mind? changelog: [`undocumented_unsafe_blocks`] fix FP on trait/impl items
This commit is contained in:
commit
62f34f2f58
4 changed files with 241 additions and 47 deletions
|
|
@ -339,6 +339,33 @@ fn is_unsafe_from_proc_macro(cx: &LateContext<'_>, span: Span) -> bool {
|
|||
.is_none_or(|src| !src.starts_with("unsafe"))
|
||||
}
|
||||
|
||||
fn find_unsafe_block_parent_in_expr<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
expr: &'tcx hir::Expr<'tcx>,
|
||||
) -> Option<(Span, HirId)> {
|
||||
match cx.tcx.parent_hir_node(expr.hir_id) {
|
||||
Node::LetStmt(hir::LetStmt { span, hir_id, .. })
|
||||
| Node::Expr(hir::Expr {
|
||||
hir_id,
|
||||
kind: hir::ExprKind::Assign(_, _, span),
|
||||
..
|
||||
}) => Some((*span, *hir_id)),
|
||||
Node::Expr(expr) => find_unsafe_block_parent_in_expr(cx, expr),
|
||||
node if let Some((span, hir_id)) = span_and_hid_of_item_alike_node(&node)
|
||||
&& is_const_or_static(&node) =>
|
||||
{
|
||||
Some((span, hir_id))
|
||||
},
|
||||
|
||||
_ => {
|
||||
if is_branchy(expr) {
|
||||
return None;
|
||||
}
|
||||
Some((expr.span, expr.hir_id))
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// Checks if any parent {expression, statement, block, local, const, static}
|
||||
// has a safety comment
|
||||
fn block_parents_have_safety_comment(
|
||||
|
|
@ -348,21 +375,7 @@ fn block_parents_have_safety_comment(
|
|||
id: HirId,
|
||||
) -> bool {
|
||||
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)),
|
||||
_ => {
|
||||
if is_branchy(expr) {
|
||||
return false;
|
||||
}
|
||||
(expr.span, expr.hir_id)
|
||||
},
|
||||
},
|
||||
Node::Expr(expr) if let Some(inner) = find_unsafe_block_parent_in_expr(cx, expr) => inner,
|
||||
Node::Stmt(hir::Stmt {
|
||||
kind:
|
||||
hir::StmtKind::Let(hir::LetStmt { span, hir_id, .. })
|
||||
|
|
@ -371,12 +384,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,
|
||||
|
|
@ -427,12 +441,12 @@ fn block_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
|
|||
}
|
||||
|
||||
fn include_attrs_in_span(cx: &LateContext<'_>, hir_id: HirId, span: Span) -> Span {
|
||||
span.to(cx
|
||||
.tcx
|
||||
.hir()
|
||||
.attrs(hir_id)
|
||||
.iter()
|
||||
.fold(span, |acc, attr| acc.to(attr.span())))
|
||||
span.to(cx.tcx.hir().attrs(hir_id).iter().fold(span, |acc, attr| {
|
||||
if attr.is_doc_comment() {
|
||||
return acc;
|
||||
}
|
||||
acc.to(attr.span())
|
||||
}))
|
||||
}
|
||||
|
||||
enum HasSafetyComment {
|
||||
|
|
@ -604,31 +618,35 @@ 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 mut span = cx.tcx.hir_body(body).value.span;
|
||||
let mut maybe_global_var = false;
|
||||
for (_, node) in cx.tcx.hir_parent_iter(body.hir_id) {
|
||||
match node {
|
||||
Node::Expr(e) => span = e.span,
|
||||
Node::Block(_) | Node::Arm(_) | Node::Stmt(_) | Node::LetStmt(_) => (),
|
||||
let mut maybe_mod_item = None;
|
||||
|
||||
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::Item(hir::Item {
|
||||
kind: ItemKind::Const(..) | ItemKind::Static(..),
|
||||
..
|
||||
}) => maybe_global_var = true,
|
||||
Node::Item(hir::Item {
|
||||
kind: ItemKind::Mod(_),
|
||||
span: item_span,
|
||||
kind: ItemKind::Mod(mod_),
|
||||
span,
|
||||
..
|
||||
}) => {
|
||||
span = *item_span;
|
||||
break;
|
||||
return maybe_mod_item
|
||||
.and_then(|item| comment_start_before_item_in_mod(cx, mod_, *span, &item))
|
||||
.map(|comment_start| mod_.spans.inner_span.with_lo(comment_start))
|
||||
.or(Some(*span));
|
||||
},
|
||||
Node::Crate(mod_) if maybe_global_var => {
|
||||
span = 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::Item(item) => {
|
||||
maybe_mod_item = Some(*item);
|
||||
},
|
||||
_ => {
|
||||
maybe_mod_item = None;
|
||||
},
|
||||
_ => break,
|
||||
}
|
||||
}
|
||||
Some(span)
|
||||
None
|
||||
}
|
||||
|
||||
fn span_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
|
||||
|
|
@ -717,3 +735,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(..),
|
||||
..
|
||||
})
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -310,5 +310,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:638: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:647: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:657: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:662: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
|
||||
|
||||
|
|
|
|||
|
|
@ -390,5 +390,53 @@ 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:638: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:647: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:657: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:662:40
|
||||
|
|
||||
LL | const NO_SAFETY_IN_IMPL: 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:673:9
|
||||
|
|
||||
LL | unsafe { here_is_another_variable_with_long_name };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= 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:702:9
|
||||
|
|
||||
LL | unsafe { Date::__from_ordinal_date_unchecked(1970, 1) }.into_julian_day_just_make_this_line_longer();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= help: consider adding a safety comment on the preceding line
|
||||
|
||||
error: aborting due to 51 previous errors
|
||||
|
||||
|
|
|
|||
|
|
@ -632,4 +632,75 @@ 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 issue_13024() {
|
||||
let mut just_a_simple_variable_with_a_very_long_name_that_has_seventy_eight_characters = 0;
|
||||
let here_is_another_variable_with_long_name = 100;
|
||||
|
||||
// SAFETY: fail ONLY if `accept-comment-above-statement = false`
|
||||
just_a_simple_variable_with_a_very_long_name_that_has_seventy_eight_characters =
|
||||
unsafe { here_is_another_variable_with_long_name };
|
||||
//~[disabled]^ undocumented_unsafe_blocks
|
||||
}
|
||||
|
||||
// https://docs.rs/time/0.3.36/src/time/offset_date_time.rs.html#35
|
||||
mod issue_11709_regression {
|
||||
use std::num::NonZeroI32;
|
||||
|
||||
struct Date {
|
||||
value: NonZeroI32,
|
||||
}
|
||||
|
||||
impl Date {
|
||||
const unsafe fn __from_ordinal_date_unchecked(year: i32, ordinal: u16) -> Self {
|
||||
Self {
|
||||
// Safety: The caller must guarantee that `ordinal` is not zero.
|
||||
value: unsafe { NonZeroI32::new_unchecked((year << 9) | ordinal as i32) },
|
||||
}
|
||||
}
|
||||
|
||||
const fn into_julian_day_just_make_this_line_longer(self) -> i32 {
|
||||
42
|
||||
}
|
||||
}
|
||||
|
||||
/// The Julian day of the Unix epoch.
|
||||
// SAFETY: fail ONLY if `accept-comment-above-attribute = false`
|
||||
#[allow(unsafe_code)]
|
||||
const UNIX_EPOCH_JULIAN_DAY: i32 =
|
||||
unsafe { Date::__from_ordinal_date_unchecked(1970, 1) }.into_julian_day_just_make_this_line_longer();
|
||||
//~[disabled]^ undocumented_unsafe_blocks
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue