fix

3 files changed, 176 insertions, 42 deletions

diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs
index 09efb12f5da..1992f61bdb4 100644
--- a/clippy_lints/src/undocumented_unsafe_blocks.rs
+++ b/clippy_lints/src/undocumented_unsafe_blocks.rs
@@ -140,6 +140,7 @@ fn block_has_safety_comment(cx: &LateContext<'_>, block: &hir::Block<'_>) -> boo
 }
 
 /// Checks if the lines immediately preceding the item contain a safety comment.
+#[allow(clippy::collapsible_match)]
 fn item_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) -> bool {
     if span_from_macro_expansion_has_safety_comment(cx, item.span) || span_in_body_has_safety_comment(cx, item.span) {
         return true;
@@ -147,48 +148,51 @@ fn item_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) -> bool {
 
     if item.span.ctxt() == SyntaxContext::root() {
         if let Some(parent_node) = get_parent_node(cx.tcx, item.hir_id()) {
-            let mut span_before_item = None;
-            let mut hi = false;
-            if let Node::Item(parent_item) = parent_node {
-                if let ItemKind::Mod(parent_mod) = &parent_item.kind {
-                    for (idx, item_id) in parent_mod.item_ids.iter().enumerate() {
-                        if *item_id == item.item_id() {
-                            if idx == 0 {
-                                // mod A { /* comment */ unsafe impl T {} ... }
-                                // ^------------------------------------------^ gets this span
-                                // ^---------------------^ finally checks the text in this range
-                                hi = false;
-                                span_before_item = Some(parent_item.span);
-                            } else {
-                                let prev_item = cx.tcx.hir().item(parent_mod.item_ids[idx - 1]);
-                                // some_item /* comment */ unsafe impl T {}
-                                // ^-------^ gets this span
-                                //         ^---------------^ finally checks the text in this range
-                                hi = true;
-                                span_before_item = Some(prev_item.span);
-                            }
-                            break;
+            let comment_start;
+            match parent_node {
+                Node::Crate(parent_mod) => {
+                    comment_start = comment_start_before_impl_in_mod(cx, parent_mod, parent_mod.spans.inner_span, item);
+                },
+                Node::Item(parent_item) => {
+                    if let ItemKind::Mod(parent_mod) = &parent_item.kind {
+                        comment_start = comment_start_before_impl_in_mod(cx, parent_mod, parent_item.span, item);
+                    } else {
+                        // Doesn't support impls in this position. Pretend a comment was found.
+                        return true;
+                    }
+                },
+                Node::Stmt(stmt) => {
+                    if let Some(stmt_parent) = get_parent_node(cx.tcx, stmt.hir_id) {
+                        match stmt_parent {
+                            Node::Block(block) => {
+                                comment_start = walk_span_to_context(block.span, SyntaxContext::root()).map(Span::lo);
+                            },
+                            _ => {
+                                // Doesn't support impls in this position. Pretend a comment was found.
+                                return true;
+                            },
                         }
+                    } else {
+                        // Doesn't support impls in this position. Pretend a comment was found.
+                        return true;
                     }
-                }
+                },
+                _ => {
+                    // Doesn't support impls in this position. Pretend a comment was found.
+                    return true;
+                },
             }
-            let span_before_item = span_before_item.unwrap();
 
             let source_map = cx.sess().source_map();
-            if let Some(item_span) = walk_span_to_context(item.span, SyntaxContext::root())
-                && let Some(span_before_item) = walk_span_to_context(span_before_item, SyntaxContext::root())
-                && let Ok(unsafe_line) = source_map.lookup_line(item_span.lo())
-                && let Ok(line_before_unsafe) = source_map.lookup_line(if hi {
-                    span_before_item.hi()
-                } else {
-                    span_before_item.lo()
-                })
-                && Lrc::ptr_eq(&unsafe_line.sf, &line_before_unsafe.sf)
+            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)
+                && Lrc::ptr_eq(&unsafe_line.sf, &comment_start_line.sf)
                 && let Some(src) = unsafe_line.sf.src.as_deref()
             {
-                line_before_unsafe.line < unsafe_line.line && text_has_safety_comment(
+                comment_start_line.line < unsafe_line.line && text_has_safety_comment(
                     src,
-                    &unsafe_line.sf.lines[line_before_unsafe.line + 1..=unsafe_line.line],
+                    &unsafe_line.sf.lines[comment_start_line.line + 1..=unsafe_line.line],
                     unsafe_line.sf.start_pos.to_usize(),
                 )
             } else {
@@ -204,6 +208,35 @@ fn item_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) -> bool {
     }
 }
 
+fn comment_start_before_impl_in_mod(
+    cx: &LateContext<'_>,
+    parent_mod: &hir::Mod<'_>,
+    parent_mod_span: Span,
+    imple: &hir::Item<'_>,
+) -> Option<BytePos> {
+    parent_mod.item_ids.iter().enumerate().find_map(|(idx, item_id)| {
+        if *item_id == imple.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());
+                }
+            } 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 let Some(sp) = walk_span_to_context(prev_item.span, SyntaxContext::root()) {
+                    return Some(sp.hi());
+                }
+            }
+        }
+        None
+    })
+}
+
 fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
     let source_map = cx.sess().source_map();
     let ctxt = span.ctxt();
diff --git a/tests/ui/undocumented_unsafe_blocks.rs b/tests/ui/undocumented_unsafe_blocks.rs
index fd64c60384d..33b6a82f9d2 100644
--- a/tests/ui/undocumented_unsafe_blocks.rs
+++ b/tests/ui/undocumented_unsafe_blocks.rs
@@ -344,7 +344,7 @@ mod unsafe_impl_smoke_test {
     unsafe impl A for (i32) {}
 
     mod sub_mod {
-        // error: also works for the first item
+        // error:
         unsafe impl B for (u32) {}
         unsafe trait B {}
     }
@@ -363,24 +363,51 @@ mod unsafe_impl_smoke_test {
 mod unsafe_impl_from_macro {
     unsafe trait T {}
 
+    // error
     macro_rules! no_safety_comment {
         ($t:ty) => {
             unsafe impl T for $t {}
         };
     }
-    // error
+
+    // ok
     no_safety_comment!(());
 
+    // ok
     macro_rules! with_safety_comment {
         ($t:ty) => {
             // SAFETY:
             unsafe impl T for $t {}
         };
     }
+
     // ok
     with_safety_comment!((i32));
 }
 
+mod unsafe_impl_macro_and_not_macro {
+    unsafe trait T {}
+
+    // error
+    macro_rules! no_safety_comment {
+        ($t:ty) => {
+            unsafe impl T for $t {}
+        };
+    }
+
+    // ok
+    no_safety_comment!(());
+
+    // error
+    unsafe impl T for (i32) {}
+
+    // ok
+    no_safety_comment!(u32);
+
+    // error
+    unsafe impl T for (bool) {}
+}
+
 #[rustfmt::skip]
 mod unsafe_impl_valid_comment {
     unsafe trait SaFety {}
@@ -440,4 +467,22 @@ mod unsafe_impl_invalid_comment {
     unsafe impl Interference for () {}
 }
 
+unsafe trait ImplInFn {}
+
+fn impl_in_fn() {
+    // error
+    unsafe impl ImplInFn for () {}
+
+    // SAFETY: ok
+    unsafe impl ImplInFn for (i32) {}
+}
+
+unsafe trait CrateRoot {}
+
+// error
+unsafe impl CrateRoot for () {}
+
+// SAFETY: ok
+unsafe impl CrateRoot for (i32) {}
+
 fn main() {}
diff --git a/tests/ui/undocumented_unsafe_blocks.stderr b/tests/ui/undocumented_unsafe_blocks.stderr
index 3a1f647b06d..b79949e9d06 100644
--- a/tests/ui/undocumented_unsafe_blocks.stderr
+++ b/tests/ui/undocumented_unsafe_blocks.stderr
@@ -164,7 +164,7 @@ LL |         unsafe impl B for (u32) {}
    = help: consider adding a safety comment on the preceding line
 
 error: unsafe impl missing a safety comment
-  --> $DIR/undocumented_unsafe_blocks.rs:368:13
+  --> $DIR/undocumented_unsafe_blocks.rs:369:13
    |
 LL |             unsafe impl T for $t {}
    |             ^^^^^^^^^^^^^^^^^^^^^^^
@@ -176,7 +176,47 @@ LL |     no_safety_comment!(());
    = note: this error originates in the macro `no_safety_comment` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: unsafe impl missing a safety comment
-  --> $DIR/undocumented_unsafe_blocks.rs:427:5
+  --> $DIR/undocumented_unsafe_blocks.rs:394:13
+   |
+LL |             unsafe impl T for $t {}
+   |             ^^^^^^^^^^^^^^^^^^^^^^^
+...
+LL |     no_safety_comment!(());
+   |     ---------------------- in this macro invocation
+   |
+   = help: consider adding a safety comment on the preceding line
+   = note: this error originates in the macro `no_safety_comment` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: unsafe impl missing a safety comment
+  --> $DIR/undocumented_unsafe_blocks.rs:402:5
+   |
+LL |     unsafe impl T for (i32) {}
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider adding a safety comment on the preceding line
+
+error: unsafe impl missing a safety comment
+  --> $DIR/undocumented_unsafe_blocks.rs:394:13
+   |
+LL |             unsafe impl T for $t {}
+   |             ^^^^^^^^^^^^^^^^^^^^^^^
+...
+LL |     no_safety_comment!(u32);
+   |     ----------------------- in this macro invocation
+   |
+   = help: consider adding a safety comment on the preceding line
+   = note: this error originates in the macro `no_safety_comment` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: unsafe impl missing a safety comment
+  --> $DIR/undocumented_unsafe_blocks.rs:408:5
+   |
+LL |     unsafe impl T for (bool) {}
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider adding a safety comment on the preceding line
+
+error: unsafe impl missing a safety comment
+  --> $DIR/undocumented_unsafe_blocks.rs:454:5
    |
 LL |     unsafe impl NoComment for () {}
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -184,7 +224,7 @@ LL |     unsafe impl NoComment for () {}
    = help: consider adding a safety comment on the preceding line
 
 error: unsafe impl missing a safety comment
-  --> $DIR/undocumented_unsafe_blocks.rs:431:19
+  --> $DIR/undocumented_unsafe_blocks.rs:458:19
    |
 LL |     /* SAFETY: */ unsafe impl InlineComment for () {}
    |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -192,7 +232,7 @@ LL |     /* SAFETY: */ unsafe impl InlineComment for () {}
    = help: consider adding a safety comment on the preceding line
 
 error: unsafe impl missing a safety comment
-  --> $DIR/undocumented_unsafe_blocks.rs:435:5
+  --> $DIR/undocumented_unsafe_blocks.rs:462:5
    |
 LL |     unsafe impl TrailingComment for () {} // SAFETY:
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -200,12 +240,28 @@ LL |     unsafe impl TrailingComment for () {} // SAFETY:
    = help: consider adding a safety comment on the preceding line
 
 error: unsafe impl missing a safety comment
-  --> $DIR/undocumented_unsafe_blocks.rs:440:5
+  --> $DIR/undocumented_unsafe_blocks.rs:467:5
    |
 LL |     unsafe impl Interference for () {}
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider adding a safety comment on the preceding line
 
-error: aborting due to 25 previous errors
+error: unsafe impl missing a safety comment
+  --> $DIR/undocumented_unsafe_blocks.rs:474:5
+   |
+LL |     unsafe impl ImplInFn for () {}
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider adding a safety comment on the preceding line
+
+error: unsafe impl missing a safety comment
+  --> $DIR/undocumented_unsafe_blocks.rs:483:1
+   |
+LL | unsafe impl CrateRoot for () {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider adding a safety comment on the preceding line
+
+error: aborting due to 31 previous errors