Skip to content

fix undocumented-unsafe-blocks false positive #9648

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions clippy_lints/src/undocumented_unsafe_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ impl LateLintPass<'_> for UndocumentedUnsafeBlocks {
&& !in_external_macro(cx.tcx.sess, block.span)
&& !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, block.hir_id)
&& !is_unsafe_from_proc_macro(cx, block.span)
&& !block_has_safety_comment(cx, block)
&& !block_has_safety_comment(cx, block.span)
&& !block_parents_have_safety_comment(cx, block.hir_id)
{
let source_map = cx.tcx.sess.source_map();
let span = if source_map.is_multiline(block.span) {
Expand Down Expand Up @@ -126,8 +127,41 @@ fn is_unsafe_from_proc_macro(cx: &LateContext<'_>, span: Span) -> bool {
.map_or(true, |src| !src.starts_with("unsafe"))
}

// Checks if any parent {expression, statement, block, local, const, static}
// has a safety comment
fn block_parents_have_safety_comment(cx: &LateContext<'_>, id: hir::HirId) -> bool {
if let Some(node) = get_parent_node(cx.tcx, id) {
return match node {
Node::Expr(expr) => !is_branchy(expr) && span_in_body_has_safety_comment(cx, expr.span),
Node::Stmt(hir::Stmt {
kind:
hir::StmtKind::Local(hir::Local { span, .. })
| hir::StmtKind::Expr(hir::Expr { span, .. })
| hir::StmtKind::Semi(hir::Expr { span, .. }),
..
})
| Node::Local(hir::Local { span, .. })
| Node::Item(hir::Item {
kind: hir::ItemKind::Const(..) | ItemKind::Static(..),
span,
..
}) => span_in_body_has_safety_comment(cx, *span),
_ => false,
};
}
false
}

/// Checks if an expression is "branchy", e.g. loop, match/if/etc.
fn is_branchy(expr: &hir::Expr<'_>) -> bool {
matches!(
expr.kind,
hir::ExprKind::If(..) | hir::ExprKind::Loop(..) | hir::ExprKind::Match(..)
)
}

/// Checks if the lines immediately preceding the block contain a safety comment.
fn block_has_safety_comment(cx: &LateContext<'_>, block: &hir::Block<'_>) -> bool {
fn block_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
// This intentionally ignores text before the start of a function so something like:
// ```
// // SAFETY: reason
Expand All @@ -136,7 +170,7 @@ fn block_has_safety_comment(cx: &LateContext<'_>, block: &hir::Block<'_>) -> boo
// won't work. This is to avoid dealing with where such a comment should be place relative to
// attributes and doc comments.

span_from_macro_expansion_has_safety_comment(cx, block.span) || span_in_body_has_safety_comment(cx, block.span)
span_from_macro_expansion_has_safety_comment(cx, span) || span_in_body_has_safety_comment(cx, span)
}

/// Checks if the lines immediately preceding the item contain a safety comment.
Expand Down
19 changes: 19 additions & 0 deletions tests/ui/undocumented_unsafe_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,4 +490,23 @@ unsafe impl CrateRoot for () {}
// SAFETY: ok
unsafe impl CrateRoot for (i32) {}

fn issue_9142() {
// SAFETY: ok
let _ =
// we need this comment to avoid rustfmt putting
// it all on one line
unsafe {};

// SAFETY: this is more than one level away, so it should warn
let _ = {
if unsafe { true } {
todo!();
} else {
let bar = unsafe {};
todo!();
bar
}
};
}

fn main() {}
26 changes: 25 additions & 1 deletion tests/ui/undocumented_unsafe_blocks.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -263,5 +263,29 @@ LL | unsafe impl CrateRoot for () {}
|
= help: consider adding a safety comment on the preceding line

error: aborting due to 31 previous errors
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:498:9
|
LL | unsafe {};
| ^^^^^^^^^
|
= help: consider adding a safety comment on the preceding line

error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:502:12
|
LL | if unsafe { true } {
| ^^^^^^^^^^^^^^^
|
= help: consider adding a safety comment on the preceding line

error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:505:23
|
LL | let bar = unsafe {};
| ^^^^^^^^^
|
= help: consider adding a safety comment on the preceding line

error: aborting due to 34 previous errors