From d395646a60f91ff40baebc9f0cd032cb182ea916 Mon Sep 17 00:00:00 2001 From: J-ZhengLi Date: Wed, 10 Jul 2024 11:34:24 +0800 Subject: [PATCH 1/4] fix [`undocumented_unsafe_blocks`] FP with trait/impl items --- .../src/undocumented_unsafe_blocks.rs | 82 +++++++++++-------- .../undocumented_unsafe_blocks.default.stderr | 42 +++++++++- ...undocumented_unsafe_blocks.disabled.stderr | 42 +++++++++- .../undocumented_unsafe_blocks.rs | 41 ++++++++++ 4 files changed, 171 insertions(+), 36 deletions(-) diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index 746bf018bcc3..917e7919c124 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -350,12 +350,13 @@ fn block_parents_have_safety_comment( let (span, hir_id) = match cx.tcx.parent_hir_node(id) { Node::Expr(expr) => match cx.tcx.parent_hir_node(expr.hir_id) { Node::LetStmt(hir::LetStmt { span, hir_id, .. }) => (*span, *hir_id), - Node::Item(hir::Item { - kind: ItemKind::Const(..) | ItemKind::Static(..), - span, - owner_id, - .. - }) => (*span, cx.tcx.local_def_id_to_hir_id(owner_id.def_id)), + + node if let Some((span, hir_id)) = span_and_hid_of_item_alike_node(&node) + && is_const_or_static(&node) => + { + (span, hir_id) + }, + _ => { if is_branchy(expr) { return false; @@ -371,12 +372,13 @@ fn block_parents_have_safety_comment( .. }) | Node::LetStmt(hir::LetStmt { span, hir_id, .. }) => (*span, *hir_id), - Node::Item(hir::Item { - kind: ItemKind::Const(..) | ItemKind::Static(..), - span, - owner_id, - .. - }) => (*span, cx.tcx.local_def_id_to_hir_id(owner_id.def_id)), + + node if let Some((span, hir_id)) = span_and_hid_of_item_alike_node(&node) + && is_const_or_static(&node) => + { + (span, hir_id) + }, + _ => return false, }; // if unsafe block is part of a let/const/static statement, @@ -604,31 +606,18 @@ fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span fn get_body_search_span(cx: &LateContext<'_>) -> Option { let body = cx.enclosing_body?; - let mut span = cx.tcx.hir_body(body).value.span; - let mut maybe_global_var = false; - for (_, node) in cx.tcx.hir_parent_iter(body.hir_id) { - match node { - Node::Expr(e) => span = e.span, - Node::Block(_) | Node::Arm(_) | Node::Stmt(_) | Node::LetStmt(_) => (), - Node::Item(hir::Item { - kind: ItemKind::Const(..) | ItemKind::Static(..), - .. - }) => maybe_global_var = true, - Node::Item(hir::Item { - kind: ItemKind::Mod(_), - span: item_span, - .. - }) => { - span = *item_span; - break; + for (_, parent_node) in cx.tcx.hir_parent_iter(body.hir_id) { + match parent_node { + Node::Crate(mod_) => return Some(mod_.spans.inner_span), + node if let Some((span, _)) = span_and_hid_of_item_alike_node(&node) + && !is_const_or_static(&node) => + { + return Some(span); }, - Node::Crate(mod_) if maybe_global_var => { - span = mod_.spans.inner_span; - }, - _ => break, + _ => {}, } } - Some(span) + None } fn span_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool { @@ -717,3 +706,28 @@ fn text_has_safety_comment(src: &str, line_starts: &[RelativeBytePos], start_pos } } } + +fn span_and_hid_of_item_alike_node(node: &Node<'_>) -> Option<(Span, HirId)> { + match node { + Node::Item(item) => Some((item.span, item.owner_id.into())), + Node::TraitItem(ti) => Some((ti.span, ti.owner_id.into())), + Node::ImplItem(ii) => Some((ii.span, ii.owner_id.into())), + _ => None, + } +} + +fn is_const_or_static(node: &Node<'_>) -> bool { + matches!( + node, + Node::Item(hir::Item { + kind: ItemKind::Const(..) | ItemKind::Static(..), + .. + }) | Node::ImplItem(hir::ImplItem { + kind: hir::ImplItemKind::Const(..), + .. + }) | Node::TraitItem(hir::TraitItem { + kind: hir::TraitItemKind::Const(..), + .. + }) + ) +} diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.default.stderr b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.default.stderr index 32ed78151d23..d3585322bb92 100644 --- a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.default.stderr +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.default.stderr @@ -310,5 +310,45 @@ LL | let bar = unsafe {}; | = help: consider adding a safety comment on the preceding line -error: aborting due to 35 previous errors +error: unsafe block missing a safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:592:52 + | +LL | const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 }; + | ^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe block missing a safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:601:41 + | +LL | const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 }; + | ^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe block missing a safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:611:42 + | +LL | const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 }; + | ^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe block missing a safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:616:40 + | +LL | const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 }; + | ^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe block missing a safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:627:9 + | +LL | unsafe { here_is_another_variable_with_long_name }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: aborting due to 40 previous errors diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr index 83a6986addf2..9ded7fb407a2 100644 --- a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr @@ -390,5 +390,45 @@ LL | unsafe {} | = help: consider adding a safety comment on the preceding line -error: aborting due to 45 previous errors +error: unsafe block missing a safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:592:52 + | +LL | const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 }; + | ^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe block missing a safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:601:41 + | +LL | const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 }; + | ^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe block missing a safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:611:42 + | +LL | const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 }; + | ^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe block missing a safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:616:40 + | +LL | const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 }; + | ^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe block missing a safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:627:9 + | +LL | unsafe { here_is_another_variable_with_long_name }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: aborting due to 50 previous errors diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs index 6a3fda3df5c3..582f31f3bba2 100644 --- a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs @@ -632,4 +632,45 @@ mod issue_11246 { // Safety: Another safety comment const FOO: () = unsafe {}; +// trait items and impl items +mod issue_11709 { + trait MyTrait { + const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 }; + //~^ ERROR: unsafe block missing a safety comment + + // SAFETY: safe + const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 1 }; + + // SAFETY: unrelated + unsafe fn unsafe_fn() {} + + const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 }; + //~^ ERROR: unsafe block missing a safety comment + } + + struct UnsafeStruct; + + impl MyTrait for UnsafeStruct { + // SAFETY: safe in this impl + const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 2 }; + + const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 }; + //~^ ERROR: unsafe block missing a safety comment + } + + impl UnsafeStruct { + const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 }; + //~^ ERROR: unsafe block missing a safety comment + } +} + +fn issue_13024() { + let mut just_a_simple_variable_with_a_very_long_name_that_has_seventy_eight_characters = 0; + let here_is_another_variable_with_long_name = 100; + + // SAFETY: good + just_a_simple_variable_with_a_very_long_name_that_has_seventy_eight_characters = + unsafe { here_is_another_variable_with_long_name }; +} + fn main() {} From 91d20cbb82f041a31f747788567f884023cc322b Mon Sep 17 00:00:00 2001 From: yanglsh Date: Sat, 28 Dec 2024 00:19:20 -0700 Subject: [PATCH 2/4] Use inner span in `undocumented_unsafe_blocks` --- .../src/undocumented_unsafe_blocks.rs | 19 ++++++++++++++++++- .../undocumented_unsafe_blocks.default.stderr | 10 +++++----- ...undocumented_unsafe_blocks.disabled.stderr | 10 +++++----- .../undocumented_unsafe_blocks.rs | 1 + 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index 917e7919c124..cfb7246f2d24 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -606,15 +606,32 @@ fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span fn get_body_search_span(cx: &LateContext<'_>) -> Option { let body = cx.enclosing_body?; + let mut maybe_mod_item = None; + for (_, parent_node) in cx.tcx.hir_parent_iter(body.hir_id) { match parent_node { Node::Crate(mod_) => return Some(mod_.spans.inner_span), + Node::Item(hir::Item { + kind: ItemKind::Mod(mod_), + 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)) + .or(Some(*span)); + }, node if let Some((span, _)) = span_and_hid_of_item_alike_node(&node) && !is_const_or_static(&node) => { return Some(span); }, - _ => {}, + Node::Item(item) => { + maybe_mod_item = Some(*item); + }, + _ => { + maybe_mod_item = None; + }, } } None diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.default.stderr b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.default.stderr index d3585322bb92..1773babbfcfc 100644 --- a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.default.stderr +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.default.stderr @@ -311,7 +311,7 @@ LL | let bar = unsafe {}; = help: consider adding a safety comment on the preceding line error: unsafe block missing a safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:592:52 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:638:52 | LL | const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 }; | ^^^^^^^^^^^^ @@ -319,7 +319,7 @@ LL | const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 }; = help: consider adding a safety comment on the preceding line error: unsafe block missing a safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:601:41 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:647:41 | LL | const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 }; | ^^^^^^^^^^^^ @@ -327,7 +327,7 @@ LL | const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 }; = help: consider adding a safety comment on the preceding line error: unsafe block missing a safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:611:42 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:657:42 | LL | const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 }; | ^^^^^^^^^^^^ @@ -335,7 +335,7 @@ LL | const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 }; = help: consider adding a safety comment on the preceding line error: unsafe block missing a safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:616:40 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:662:40 | LL | const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 }; | ^^^^^^^^^^^^ @@ -343,7 +343,7 @@ LL | const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 }; = help: consider adding a safety comment on the preceding line error: unsafe block missing a safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:627:9 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:673:9 | LL | unsafe { here_is_another_variable_with_long_name }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr index 9ded7fb407a2..494da37a5fb3 100644 --- a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr @@ -391,7 +391,7 @@ LL | unsafe {} = help: consider adding a safety comment on the preceding line error: unsafe block missing a safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:592:52 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:638:52 | LL | const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 }; | ^^^^^^^^^^^^ @@ -399,7 +399,7 @@ LL | const NO_SAFETY_IN_TRAIT_BUT_IN_IMPL: u8 = unsafe { 0 }; = help: consider adding a safety comment on the preceding line error: unsafe block missing a safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:601:41 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:647:41 | LL | const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 }; | ^^^^^^^^^^^^ @@ -407,7 +407,7 @@ LL | const NO_SAFETY_IN_TRAIT: i32 = unsafe { 1 }; = help: consider adding a safety comment on the preceding line error: unsafe block missing a safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:611:42 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:657:42 | LL | const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 }; | ^^^^^^^^^^^^ @@ -415,7 +415,7 @@ LL | const HAS_SAFETY_IN_TRAIT: i32 = unsafe { 3 }; = help: consider adding a safety comment on the preceding line error: unsafe block missing a safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:616:40 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:662:40 | LL | const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 }; | ^^^^^^^^^^^^ @@ -423,7 +423,7 @@ LL | const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 }; = help: consider adding a safety comment on the preceding line error: unsafe block missing a safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:627:9 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:673:9 | LL | unsafe { here_is_another_variable_with_long_name }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs index 582f31f3bba2..e6002c2d22ef 100644 --- a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs @@ -671,6 +671,7 @@ fn issue_13024() { // SAFETY: good just_a_simple_variable_with_a_very_long_name_that_has_seventy_eight_characters = unsafe { here_is_another_variable_with_long_name }; + //~^ undocumented_unsafe_blocks } fn main() {} From 3899488e5363f438c5373656ba70cbbe77b438a5 Mon Sep 17 00:00:00 2001 From: yanglsh Date: Tue, 18 Feb 2025 00:08:33 +0800 Subject: [PATCH 3/4] fix `undocumented_unsafe_blocks` FP in long assignment --- clippy_lints/src/undocumented_unsafe_blocks.rs | 8 ++++++-- .../undocumented_unsafe_blocks.default.stderr | 10 +--------- .../undocumented_unsafe_blocks.rs | 4 ++-- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index cfb7246f2d24..fcb3e4e6ab9e 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -349,8 +349,12 @@ fn block_parents_have_safety_comment( ) -> bool { let (span, hir_id) = match cx.tcx.parent_hir_node(id) { Node::Expr(expr) => match cx.tcx.parent_hir_node(expr.hir_id) { - Node::LetStmt(hir::LetStmt { span, hir_id, .. }) => (*span, *hir_id), - + Node::LetStmt(hir::LetStmt { span, hir_id, .. }) + | Node::Expr(hir::Expr { + hir_id, + kind: hir::ExprKind::Assign(_, _, span), + .. + }) => (*span, *hir_id), node if let Some((span, hir_id)) = span_and_hid_of_item_alike_node(&node) && is_const_or_static(&node) => { diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.default.stderr b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.default.stderr index 1773babbfcfc..4357c455e50e 100644 --- a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.default.stderr +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.default.stderr @@ -342,13 +342,5 @@ LL | const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 }; | = help: consider adding a safety comment on the preceding line -error: unsafe block missing a safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:673:9 - | -LL | unsafe { here_is_another_variable_with_long_name }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: consider adding a safety comment on the preceding line - -error: aborting due to 40 previous errors +error: aborting due to 39 previous errors diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs index e6002c2d22ef..f3546101c96b 100644 --- a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs @@ -668,10 +668,10 @@ fn issue_13024() { let mut just_a_simple_variable_with_a_very_long_name_that_has_seventy_eight_characters = 0; let here_is_another_variable_with_long_name = 100; - // SAFETY: good + // SAFETY: fail ONLY if `accept-comment-above-statement = false` just_a_simple_variable_with_a_very_long_name_that_has_seventy_eight_characters = unsafe { here_is_another_variable_with_long_name }; - //~^ undocumented_unsafe_blocks + //~[disabled]^ undocumented_unsafe_blocks } fn main() {} From d027ca95de8ebe3795d3a94304f654635a893c54 Mon Sep 17 00:00:00 2001 From: yanglsh Date: Tue, 18 Feb 2025 03:23:42 +0800 Subject: [PATCH 4/4] Add regression test for #11709 --- .../src/undocumented_unsafe_blocks.rs | 60 +++++++++++-------- ...undocumented_unsafe_blocks.disabled.stderr | 10 +++- .../undocumented_unsafe_blocks.rs | 29 +++++++++ 3 files changed, 72 insertions(+), 27 deletions(-) diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index fcb3e4e6ab9e..b824eb88abf9 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -339,6 +339,33 @@ fn is_unsafe_from_proc_macro(cx: &LateContext<'_>, span: Span) -> bool { .is_none_or(|src| !src.starts_with("unsafe")) } +fn find_unsafe_block_parent_in_expr<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx hir::Expr<'tcx>, +) -> Option<(Span, HirId)> { + match cx.tcx.parent_hir_node(expr.hir_id) { + Node::LetStmt(hir::LetStmt { span, hir_id, .. }) + | Node::Expr(hir::Expr { + hir_id, + kind: hir::ExprKind::Assign(_, _, span), + .. + }) => Some((*span, *hir_id)), + Node::Expr(expr) => find_unsafe_block_parent_in_expr(cx, expr), + node if let Some((span, hir_id)) = span_and_hid_of_item_alike_node(&node) + && is_const_or_static(&node) => + { + Some((span, hir_id)) + }, + + _ => { + if is_branchy(expr) { + return None; + } + Some((expr.span, expr.hir_id)) + }, + } +} + // Checks if any parent {expression, statement, block, local, const, static} // has a safety comment fn block_parents_have_safety_comment( @@ -348,26 +375,7 @@ fn block_parents_have_safety_comment( id: HirId, ) -> bool { let (span, hir_id) = match cx.tcx.parent_hir_node(id) { - Node::Expr(expr) => match cx.tcx.parent_hir_node(expr.hir_id) { - Node::LetStmt(hir::LetStmt { span, hir_id, .. }) - | Node::Expr(hir::Expr { - hir_id, - kind: hir::ExprKind::Assign(_, _, span), - .. - }) => (*span, *hir_id), - node if let Some((span, hir_id)) = span_and_hid_of_item_alike_node(&node) - && is_const_or_static(&node) => - { - (span, hir_id) - }, - - _ => { - if is_branchy(expr) { - return false; - } - (expr.span, expr.hir_id) - }, - }, + Node::Expr(expr) if let Some(inner) = find_unsafe_block_parent_in_expr(cx, expr) => inner, Node::Stmt(hir::Stmt { kind: hir::StmtKind::Let(hir::LetStmt { span, hir_id, .. }) @@ -433,12 +441,12 @@ fn block_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool { } fn include_attrs_in_span(cx: &LateContext<'_>, hir_id: HirId, span: Span) -> Span { - span.to(cx - .tcx - .hir() - .attrs(hir_id) - .iter() - .fold(span, |acc, attr| acc.to(attr.span()))) + span.to(cx.tcx.hir().attrs(hir_id).iter().fold(span, |acc, attr| { + if attr.is_doc_comment() { + return acc; + } + acc.to(attr.span()) + })) } enum HasSafetyComment { diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr index 494da37a5fb3..eb61e1e6da08 100644 --- a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.disabled.stderr @@ -430,5 +430,13 @@ LL | unsafe { here_is_another_variable_with_long_name }; | = help: consider adding a safety comment on the preceding line -error: aborting due to 50 previous errors +error: unsafe block missing a safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:702:9 + | +LL | unsafe { Date::__from_ordinal_date_unchecked(1970, 1) }.into_julian_day_just_make_this_line_longer(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: aborting due to 51 previous errors diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs index f3546101c96b..2492a3c2f562 100644 --- a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs @@ -674,4 +674,33 @@ fn issue_13024() { //~[disabled]^ undocumented_unsafe_blocks } +// https://docs.rs/time/0.3.36/src/time/offset_date_time.rs.html#35 +mod issue_11709_regression { + use std::num::NonZeroI32; + + struct Date { + value: NonZeroI32, + } + + impl Date { + const unsafe fn __from_ordinal_date_unchecked(year: i32, ordinal: u16) -> Self { + Self { + // Safety: The caller must guarantee that `ordinal` is not zero. + value: unsafe { NonZeroI32::new_unchecked((year << 9) | ordinal as i32) }, + } + } + + const fn into_julian_day_just_make_this_line_longer(self) -> i32 { + 42 + } + } + + /// The Julian day of the Unix epoch. + // SAFETY: fail ONLY if `accept-comment-above-attribute = false` + #[allow(unsafe_code)] + const UNIX_EPOCH_JULIAN_DAY: i32 = + unsafe { Date::__from_ordinal_date_unchecked(1970, 1) }.into_julian_day_just_make_this_line_longer(); + //~[disabled]^ undocumented_unsafe_blocks +} + fn main() {}