about summary refs log tree commit diff
diff options
context:
space:
mode:
authoryanglsh <yanglsh@shanghaitech.edu.cn>2025-07-26 16:10:30 +0800
committeryanglsh <yanglsh@shanghaitech.edu.cn>2025-08-21 11:30:57 +0800
commitf4c6ab0d144cb975cbdabbf707f0382a8e40cdc7 (patch)
tree73fc2e784cccfd38c09a6f6bf01a599f5c7cfc98
parent9a2076ed87b8a220e246c34d7d7a2cbc4d0bf282 (diff)
downloadrust-f4c6ab0d144cb975cbdabbf707f0382a8e40cdc7.tar.gz
rust-f4c6ab0d144cb975cbdabbf707f0382a8e40cdc7.zip
fix: `unnecessary_safety_comment` does not lint for the first line
-rw-r--r--clippy_lints/src/undocumented_unsafe_blocks.rs38
-rw-r--r--tests/ui-cargo/undocumented_unsafe_blocks/fail/Cargo.stderr26
-rw-r--r--tests/ui-cargo/undocumented_unsafe_blocks/fail/Cargo.toml12
-rw-r--r--tests/ui-cargo/undocumented_unsafe_blocks/fail/src/main.rs7
4 files changed, 75 insertions, 8 deletions
diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs
index cf603c6190b..c031dbb8cc8 100644
--- a/clippy_lints/src/undocumented_unsafe_blocks.rs
+++ b/clippy_lints/src/undocumented_unsafe_blocks.rs
@@ -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;
@@ -518,10 +519,12 @@ 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 {
@@ -529,7 +532,8 @@ fn item_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) -> HasSaf
         } 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),
@@ -592,12 +596,27 @@ 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 {
@@ -605,15 +624,18 @@ fn comment_start_before_item_in_mod(
                 // ^------------------------------------------^ 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()));
                 }
             }
         }
@@ -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)
diff --git a/tests/ui-cargo/undocumented_unsafe_blocks/fail/Cargo.stderr b/tests/ui-cargo/undocumented_unsafe_blocks/fail/Cargo.stderr
new file mode 100644
index 00000000000..59a7146ac90
--- /dev/null
+++ b/tests/ui-cargo/undocumented_unsafe_blocks/fail/Cargo.stderr
@@ -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
diff --git a/tests/ui-cargo/undocumented_unsafe_blocks/fail/Cargo.toml b/tests/ui-cargo/undocumented_unsafe_blocks/fail/Cargo.toml
new file mode 100644
index 00000000000..36bb3472df0
--- /dev/null
+++ b/tests/ui-cargo/undocumented_unsafe_blocks/fail/Cargo.toml
@@ -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"
diff --git a/tests/ui-cargo/undocumented_unsafe_blocks/fail/src/main.rs b/tests/ui-cargo/undocumented_unsafe_blocks/fail/src/main.rs
new file mode 100644
index 00000000000..5cafcff99dd
--- /dev/null
+++ b/tests/ui-cargo/undocumented_unsafe_blocks/fail/src/main.rs
@@ -0,0 +1,7 @@
+// SAFETY: ...
+mod x {}
+
+// SAFETY: ...
+mod y {}
+
+fn main() {}