Skip to content

Commit f955e9c

Browse files
committed
fix undocumented-unsafe-blocks false positive
1 parent 9a6eca5 commit f955e9c

File tree

3 files changed

+80
-4
lines changed

3 files changed

+80
-4
lines changed

clippy_lints/src/undocumented_unsafe_blocks.rs

Lines changed: 47 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,51 @@ 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+
for (_, node) in cx.tcx.hir().parent_iter(id) {
134+
match node {
135+
Node::Expr(expr) => {
136+
if !is_branchy(expr) && span_in_body_has_safety_comment(cx, expr.span) {
137+
return true;
138+
}
139+
},
140+
Node::Stmt(hir::Stmt {
141+
kind:
142+
hir::StmtKind::Local(hir::Local { span, .. })
143+
| hir::StmtKind::Expr(hir::Expr { span, .. })
144+
| hir::StmtKind::Semi(hir::Expr { span, .. }),
145+
..
146+
})
147+
| Node::Local(hir::Local { span, .. })
148+
| Node::Item(hir::Item {
149+
kind: hir::ItemKind::Const(..) | ItemKind::Static(..),
150+
span,
151+
..
152+
}) => {
153+
if span_in_body_has_safety_comment(cx, *span) {
154+
return true;
155+
}
156+
},
157+
_ => {
158+
break;
159+
},
160+
}
161+
}
162+
false
163+
}
164+
165+
/// Checks if an expression is "branchy", e.g. loop, match/if/etc.
166+
fn is_branchy(expr: &hir::Expr<'_>) -> bool {
167+
matches!(
168+
expr.kind,
169+
hir::ExprKind::If(..) | hir::ExprKind::Loop(..) | hir::ExprKind::Match(..)
170+
)
171+
}
172+
129173
/// Checks if the lines immediately preceding the block contain a safety comment.
130-
fn block_has_safety_comment(cx: &LateContext<'_>, block: &hir::Block<'_>) -> bool {
174+
fn block_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
131175
// This intentionally ignores text before the start of a function so something like:
132176
// ```
133177
// // SAFETY: reason
@@ -136,7 +180,7 @@ fn block_has_safety_comment(cx: &LateContext<'_>, block: &hir::Block<'_>) -> boo
136180
// won't work. This is to avoid dealing with where such a comment should be place relative to
137181
// attributes and doc comments.
138182

139-
span_from_macro_expansion_has_safety_comment(cx, block.span) || span_in_body_has_safety_comment(cx, block.span)
183+
span_from_macro_expansion_has_safety_comment(cx, span) || span_in_body_has_safety_comment(cx, span)
140184
}
141185

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

tests/ui/undocumented_unsafe_blocks.rs

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

493+
fn issue_9142() {
494+
// SAFETY: ok
495+
let _ = unsafe {};
496+
497+
// SAFETY: ok
498+
let _ = {
499+
if unsafe { true } {
500+
todo!();
501+
} else {
502+
let bar = unsafe {};
503+
todo!();
504+
bar
505+
}
506+
};
507+
}
508+
493509
fn main() {}

tests/ui/undocumented_unsafe_blocks.stderr

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,5 +263,21 @@ 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:499:12
268+
|
269+
LL | if unsafe { true } {
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:23
276+
|
277+
LL | let bar = unsafe {};
278+
| ^^^^^^^^^
279+
|
280+
= help: consider adding a safety comment on the preceding line
281+
282+
error: aborting due to 33 previous errors
267283

0 commit comments

Comments
 (0)