Skip to content

Commit fdaa425

Browse files
committed
Auto merge of #9648 - llogiq:fix-undocumented-unsafe-blocks, r=Jarcho
fix `undocumented-unsafe-blocks` false positive This fixes #9142 by iterating over the parent nodes as long as within a block, expression, statement, local, const or static. --- changelog: none
2 parents 0ab512c + e19fe89 commit fdaa425

File tree

3 files changed

+81
-4
lines changed

3 files changed

+81
-4
lines changed

clippy_lints/src/undocumented_unsafe_blocks.rs

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ impl LateLintPass<'_> for UndocumentedUnsafeBlocks {
6868
&& !in_external_macro(cx.tcx.sess, block.span)
6969
&& !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, block.hir_id)
7070
&& !is_unsafe_from_proc_macro(cx, block.span)
71-
&& !block_has_safety_comment(cx, block)
71+
&& !block_has_safety_comment(cx, block.span)
72+
&& !block_parents_have_safety_comment(cx, block.hir_id)
7273
{
7374
let source_map = cx.tcx.sess.source_map();
7475
let span = if source_map.is_multiline(block.span) {
@@ -126,8 +127,41 @@ fn is_unsafe_from_proc_macro(cx: &LateContext<'_>, span: Span) -> bool {
126127
.map_or(true, |src| !src.starts_with("unsafe"))
127128
}
128129

130+
// Checks if any parent {expression, statement, block, local, const, static}
131+
// has a safety comment
132+
fn block_parents_have_safety_comment(cx: &LateContext<'_>, id: hir::HirId) -> bool {
133+
if let Some(node) = get_parent_node(cx.tcx, id) {
134+
return match node {
135+
Node::Expr(expr) => !is_branchy(expr) && span_in_body_has_safety_comment(cx, expr.span),
136+
Node::Stmt(hir::Stmt {
137+
kind:
138+
hir::StmtKind::Local(hir::Local { span, .. })
139+
| hir::StmtKind::Expr(hir::Expr { span, .. })
140+
| hir::StmtKind::Semi(hir::Expr { span, .. }),
141+
..
142+
})
143+
| Node::Local(hir::Local { span, .. })
144+
| Node::Item(hir::Item {
145+
kind: hir::ItemKind::Const(..) | ItemKind::Static(..),
146+
span,
147+
..
148+
}) => span_in_body_has_safety_comment(cx, *span),
149+
_ => false,
150+
};
151+
}
152+
false
153+
}
154+
155+
/// Checks if an expression is "branchy", e.g. loop, match/if/etc.
156+
fn is_branchy(expr: &hir::Expr<'_>) -> bool {
157+
matches!(
158+
expr.kind,
159+
hir::ExprKind::If(..) | hir::ExprKind::Loop(..) | hir::ExprKind::Match(..)
160+
)
161+
}
162+
129163
/// Checks if the lines immediately preceding the block contain a safety comment.
130-
fn block_has_safety_comment(cx: &LateContext<'_>, block: &hir::Block<'_>) -> bool {
164+
fn block_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
131165
// This intentionally ignores text before the start of a function so something like:
132166
// ```
133167
// // SAFETY: reason
@@ -136,7 +170,7 @@ fn block_has_safety_comment(cx: &LateContext<'_>, block: &hir::Block<'_>) -> boo
136170
// won't work. This is to avoid dealing with where such a comment should be place relative to
137171
// attributes and doc comments.
138172

139-
span_from_macro_expansion_has_safety_comment(cx, block.span) || span_in_body_has_safety_comment(cx, block.span)
173+
span_from_macro_expansion_has_safety_comment(cx, span) || span_in_body_has_safety_comment(cx, span)
140174
}
141175

142176
/// Checks if the lines immediately preceding the item contain a safety comment.

tests/ui/undocumented_unsafe_blocks.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,4 +490,23 @@ unsafe impl CrateRoot for () {}
490490
// SAFETY: ok
491491
unsafe impl CrateRoot for (i32) {}
492492

493+
fn issue_9142() {
494+
// SAFETY: ok
495+
let _ =
496+
// we need this comment to avoid rustfmt putting
497+
// it all on one line
498+
unsafe {};
499+
500+
// SAFETY: this is more than one level away, so it should warn
501+
let _ = {
502+
if unsafe { true } {
503+
todo!();
504+
} else {
505+
let bar = unsafe {};
506+
todo!();
507+
bar
508+
}
509+
};
510+
}
511+
493512
fn main() {}

tests/ui/undocumented_unsafe_blocks.stderr

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,5 +263,29 @@ LL | unsafe impl CrateRoot for () {}
263263
|
264264
= help: consider adding a safety comment on the preceding line
265265

266-
error: aborting due to 31 previous errors
266+
error: unsafe block missing a safety comment
267+
--> $DIR/undocumented_unsafe_blocks.rs:498:9
268+
|
269+
LL | unsafe {};
270+
| ^^^^^^^^^
271+
|
272+
= help: consider adding a safety comment on the preceding line
273+
274+
error: unsafe block missing a safety comment
275+
--> $DIR/undocumented_unsafe_blocks.rs:502:12
276+
|
277+
LL | if unsafe { true } {
278+
| ^^^^^^^^^^^^^^^
279+
|
280+
= help: consider adding a safety comment on the preceding line
281+
282+
error: unsafe block missing a safety comment
283+
--> $DIR/undocumented_unsafe_blocks.rs:505:23
284+
|
285+
LL | let bar = unsafe {};
286+
| ^^^^^^^^^
287+
|
288+
= help: consider adding a safety comment on the preceding line
289+
290+
error: aborting due to 34 previous errors
267291

0 commit comments

Comments
 (0)