Skip to content

Commit 7c39d37

Browse files
Propagate accept-comment-above-attributes to statements (#15213)
Turns out that things like `#[attr]return unsafe { func(); }` didn't work because we weren't checking above attributes on statements, only on blocks! fixes #13189 Useful for Rust-For-Linux changelog:[`undocumented_unsafe_blocks`]: Make sure to propagate `accept-comment-above-attributes` to **all** statements.
2 parents b28049d + e0e881f commit 7c39d37

File tree

3 files changed

+37
-4
lines changed

3 files changed

+37
-4
lines changed

clippy_lints/src/undocumented_unsafe_blocks.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks {
143143
if let Some(tail) = block.expr
144144
&& !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, tail.hir_id)
145145
&& !tail.span.in_external_macro(cx.tcx.sess.source_map())
146-
&& let HasSafetyComment::Yes(pos) = stmt_has_safety_comment(cx, tail.span, tail.hir_id)
146+
&& let HasSafetyComment::Yes(pos) =
147+
stmt_has_safety_comment(cx, tail.span, tail.hir_id, self.accept_comment_above_attributes)
147148
&& let Some(help_span) = expr_has_unnecessary_safety_comment(cx, tail, pos)
148149
{
149150
span_lint_and_then(
@@ -167,7 +168,8 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks {
167168
};
168169
if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, stmt.hir_id)
169170
&& !stmt.span.in_external_macro(cx.tcx.sess.source_map())
170-
&& let HasSafetyComment::Yes(pos) = stmt_has_safety_comment(cx, stmt.span, stmt.hir_id)
171+
&& let HasSafetyComment::Yes(pos) =
172+
stmt_has_safety_comment(cx, stmt.span, stmt.hir_id, self.accept_comment_above_attributes)
171173
&& let Some(help_span) = expr_has_unnecessary_safety_comment(cx, expr, pos)
172174
{
173175
span_lint_and_then(
@@ -534,7 +536,12 @@ fn item_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) -> HasSaf
534536

535537
/// Checks if the lines immediately preceding the item contain a safety comment.
536538
#[allow(clippy::collapsible_match)]
537-
fn stmt_has_safety_comment(cx: &LateContext<'_>, span: Span, hir_id: HirId) -> HasSafetyComment {
539+
fn stmt_has_safety_comment(
540+
cx: &LateContext<'_>,
541+
span: Span,
542+
hir_id: HirId,
543+
accept_comment_above_attributes: bool,
544+
) -> HasSafetyComment {
538545
match span_from_macro_expansion_has_safety_comment(cx, span) {
539546
HasSafetyComment::Maybe => (),
540547
has_safety_comment => return has_safety_comment,
@@ -549,6 +556,13 @@ fn stmt_has_safety_comment(cx: &LateContext<'_>, span: Span, hir_id: HirId) -> H
549556
_ => return HasSafetyComment::Maybe,
550557
};
551558

559+
// if span_with_attrs_has_safety_comment(cx, span, hir_id, accept_comment_above_attrib
560+
// }
561+
let mut span = span;
562+
if accept_comment_above_attributes {
563+
span = include_attrs_in_span(cx, hir_id, span);
564+
}
565+
552566
let source_map = cx.sess().source_map();
553567
if let Some(comment_start) = comment_start
554568
&& let Ok(unsafe_line) = source_map.lookup_line(span.lo())

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -450,5 +450,13 @@ help: consider removing the safety comment
450450
LL | // SAFETY: unnecessary_safety_comment triggers here
451451
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
452452

453-
error: aborting due to 52 previous errors
453+
error: unsafe block missing a safety comment
454+
--> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:733:12
455+
|
456+
LL | return unsafe { h() };
457+
| ^^^^^^^^^^^^^^
458+
|
459+
= help: consider adding a safety comment on the preceding line
460+
461+
error: aborting due to 53 previous errors
454462

tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -723,4 +723,15 @@ fn issue_13039() {
723723
_ = unsafe { foo() }
724724
}
725725

726+
fn rfl_issue15034() -> i32 {
727+
unsafe fn h() -> i32 {
728+
1i32
729+
}
730+
// This shouldn't lint with accept-comment-above-attributes! Thus fixing a false positive!
731+
// SAFETY: My safety comment!
732+
#[allow(clippy::unnecessary_cast)]
733+
return unsafe { h() };
734+
//~[disabled]^ ERROR: unsafe block missing a safety comment
735+
}
736+
726737
fn main() {}

0 commit comments

Comments
 (0)