Skip to content
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
38 changes: 30 additions & 8 deletions clippy_lints/src/undocumented_unsafe_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,8 @@ fn item_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) -> HasSaf
},
Node::Stmt(stmt) => {
if let Node::Block(block) = cx.tcx.parent_hir_node(stmt.hir_id) {
walk_span_to_context(block.span, SyntaxContext::root()).map(Span::lo)
walk_span_to_context(block.span, SyntaxContext::root())
.map(|sp| CommentStartBeforeItem::Offset(sp.lo()))
} else {
// Problem getting the parent node. Pretend a comment was found.
return HasSafetyComment::Maybe;
Expand All @@ -518,18 +519,21 @@ fn item_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) -> HasSaf
};

let source_map = cx.sess().source_map();
// If the comment is in the first line of the file, there is no preceding line
if let Some(comment_start) = comment_start
&& let Ok(unsafe_line) = source_map.lookup_line(item.span.lo())
&& let Ok(comment_start_line) = source_map.lookup_line(comment_start)
&& Arc::ptr_eq(&unsafe_line.sf, &comment_start_line.sf)
&& let Ok(comment_start_line) = source_map.lookup_line(comment_start.into())
&& let include_first_line_of_file = matches!(comment_start, CommentStartBeforeItem::Start)
&& (include_first_line_of_file || Arc::ptr_eq(&unsafe_line.sf, &comment_start_line.sf))
&& let Some(src) = unsafe_line.sf.src.as_deref()
{
return if comment_start_line.line >= unsafe_line.line {
HasSafetyComment::No
} else {
match text_has_safety_comment(
src,
&unsafe_line.sf.lines()[comment_start_line.line + 1..=unsafe_line.line],
&unsafe_line.sf.lines()
[(comment_start_line.line + usize::from(!include_first_line_of_file))..=unsafe_line.line],
unsafe_line.sf.start_pos,
) {
Some(b) => HasSafetyComment::Yes(b),
Expand Down Expand Up @@ -592,28 +596,46 @@ fn stmt_has_safety_comment(
HasSafetyComment::Maybe
}

#[derive(Clone, Copy, Debug)]
enum CommentStartBeforeItem {
Offset(BytePos),
Start,
}

impl From<CommentStartBeforeItem> for BytePos {
fn from(value: CommentStartBeforeItem) -> Self {
match value {
CommentStartBeforeItem::Offset(loc) => loc,
CommentStartBeforeItem::Start => BytePos(0),
}
}
}

fn comment_start_before_item_in_mod(
cx: &LateContext<'_>,
parent_mod: &hir::Mod<'_>,
parent_mod_span: Span,
item: &hir::Item<'_>,
) -> Option<BytePos> {
) -> Option<CommentStartBeforeItem> {
parent_mod.item_ids.iter().enumerate().find_map(|(idx, item_id)| {
if *item_id == item.item_id() {
if idx == 0 {
// mod A { /* comment */ unsafe impl T {} ... }
// ^------------------------------------------^ returns the start of this span
// ^---------------------^ finally checks comments in this range
if let Some(sp) = walk_span_to_context(parent_mod_span, SyntaxContext::root()) {
return Some(sp.lo());
return Some(CommentStartBeforeItem::Offset(sp.lo()));
}
} else {
// some_item /* comment */ unsafe impl T {}
// ^-------^ returns the end of this span
// ^---------------^ finally checks comments in this range
let prev_item = cx.tcx.hir_item(parent_mod.item_ids[idx - 1]);
if prev_item.span.is_dummy() {
return Some(CommentStartBeforeItem::Start);
}
if let Some(sp) = walk_span_to_context(prev_item.span, SyntaxContext::root()) {
return Some(sp.hi());
return Some(CommentStartBeforeItem::Offset(sp.hi()));
}
}
}
Expand Down Expand Up @@ -668,7 +690,7 @@ fn get_body_search_span(cx: &LateContext<'_>) -> Option<Span> {
}) => {
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))
.map(|comment_start| mod_.spans.inner_span.with_lo(comment_start.into()))
.or(Some(*span));
},
node if let Some((span, _)) = span_and_hid_of_item_alike_node(&node)
Expand Down
26 changes: 26 additions & 0 deletions tests/ui-cargo/undocumented_unsafe_blocks/fail/Cargo.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error: module has unnecessary safety comment
--> src/main.rs:2:1
|
2 | mod x {}
| ^^^^^^^^
|
help: consider removing the safety comment
--> src/main.rs:1:1
|
1 | // SAFETY: ...
| ^^^^^^^^^^^^^^
= note: requested on the command line with `-D clippy::unnecessary-safety-comment`

error: module has unnecessary safety comment
--> src/main.rs:5:1
|
5 | mod y {}
| ^^^^^^^^
|
help: consider removing the safety comment
--> src/main.rs:4:1
|
4 | // SAFETY: ...
| ^^^^^^^^^^^^^^

error: could not compile `undocumented_unsafe_blocks` (bin "undocumented_unsafe_blocks") due to 2 previous errors
12 changes: 12 additions & 0 deletions tests/ui-cargo/undocumented_unsafe_blocks/fail/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Reproducing #14553 requires the `# Safety` comment to be in the first line of
# the file. Since `unnecessary_safety_comment` is not enabled by default, we
# will set it up here.

[package]
name = "undocumented_unsafe_blocks"
edition = "2024"
publish = false
version = "0.1.0"

[lints.clippy]
unnecessary_safety_comment = "deny"
7 changes: 7 additions & 0 deletions tests/ui-cargo/undocumented_unsafe_blocks/fail/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// SAFETY: ...
mod x {}

// SAFETY: ...
mod y {}

fn main() {}