From e2e6a02445980cd7eb213e87ad22e3bb9b074907 Mon Sep 17 00:00:00 2001 From: Renato Lochetti Date: Mon, 5 Jun 2023 08:55:39 +0100 Subject: [PATCH] Addressing reviewer comments --- book/src/lint_configuration.md | 2 +- .../src/undocumented_unsafe_blocks.rs | 9 ++++++++- clippy_lints/src/utils/conf.rs | 2 +- .../undocumented_unsafe_blocks.rs | 12 +++++++++++ tests/ui/undocumented_unsafe_blocks.rs | 12 +++++++++++ tests/ui/undocumented_unsafe_blocks.stderr | 20 +++++++++++++++++-- 6 files changed, 52 insertions(+), 5 deletions(-) diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index bd44af66d42..78a5f9da461 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -696,7 +696,7 @@ Minimum chars an ident can have, anything below or equal to this will be linted. ## `accept-comment-above-statement` -Whether to accept a safety comment to be placed above the statement containing the `usafe` block +Whether to accept a safety comment to be placed above the statement containing the `unsafe` block **Default Value:** `false` (`bool`) diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index bc739dca0b9..1676c767a34 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -580,7 +580,14 @@ fn get_body_search_span(cx: &LateContext<'_>) -> Option { for (_, node) in map.parent_iter(body.hir_id) { match node { Node::Expr(e) => span = e.span, - Node::Block(_) | Node::Arm(_) | Node::Stmt(_) | Node::Local(_) => (), + Node::Block(_) + | Node::Arm(_) + | Node::Stmt(_) + | Node::Local(_) + | Node::Item(hir::Item { + kind: hir::ItemKind::Const(..) | ItemKind::Static(..), + .. + }) => (), _ => break, } } diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index fccddfbdb8b..e8c8d478ffc 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -540,7 +540,7 @@ pub fn get_configuration_metadata() -> Vec { (min_ident_chars_threshold: u64 = 1), /// Lint: UNDOCUMENTED_UNSAFE_BLOCKS. /// - /// Whether to accept a safety comment to be placed above the statement containing the `usafe` block + /// Whether to accept a safety comment to be placed above the statement containing the `unsafe` block (accept_comment_above_statement: bool = false), } 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 ef208f2c390..e1ba1b2f67e 100644 --- a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs @@ -5,8 +5,20 @@ fn main() { // Safety: A safety comment let _some_variable_with_a_very_long_name_to_break_the_line = unsafe { a_function_with_a_very_long_name_to_break_the_line() }; + + // Safety: Another safety comment + const _SOME_CONST_WITH_A_VERY_LONG_NAME_TO_BREAK_THE_LINE: u32 = + unsafe { a_const_function_with_a_very_long_name_to_break_the_line() }; + + // Safety: Yet another safety comment + static _SOME_STATIC_WITH_A_VERY_LONG_NAME_TO_BREAK_THE_LINE: u32 = + unsafe { a_const_function_with_a_very_long_name_to_break_the_line() }; } pub unsafe fn a_function_with_a_very_long_name_to_break_the_line() -> u32 { 1 } + +pub const unsafe fn a_const_function_with_a_very_long_name_to_break_the_line() -> u32 { + 2 +} diff --git a/tests/ui/undocumented_unsafe_blocks.rs b/tests/ui/undocumented_unsafe_blocks.rs index fa86352554b..f4e7f1943ae 100644 --- a/tests/ui/undocumented_unsafe_blocks.rs +++ b/tests/ui/undocumented_unsafe_blocks.rs @@ -513,10 +513,22 @@ pub unsafe fn a_function_with_a_very_long_name_to_break_the_line() -> u32 { 1 } +pub const unsafe fn a_const_function_with_a_very_long_name_to_break_the_line() -> u32 { + 2 +} + fn issue_10832() { // Safety: A safety comment. But it will warn anyways let _some_variable_with_a_very_long_name_to_break_the_line = unsafe { a_function_with_a_very_long_name_to_break_the_line() }; + + // Safety: Another safety comment. But it will warn anyways + const _SOME_CONST_WITH_A_VERY_LONG_NAME_TO_BREAK_THE_LINE: u32 = + unsafe { a_const_function_with_a_very_long_name_to_break_the_line() }; + + // Safety: Yet another safety comment. But it will warn anyways + static _SOME_STATIC_WITH_A_VERY_LONG_NAME_TO_BREAK_THE_LINE: u32 = + unsafe { a_const_function_with_a_very_long_name_to_break_the_line() }; } fn main() {} diff --git a/tests/ui/undocumented_unsafe_blocks.stderr b/tests/ui/undocumented_unsafe_blocks.stderr index 84a8e681f67..ee1d3aa285a 100644 --- a/tests/ui/undocumented_unsafe_blocks.stderr +++ b/tests/ui/undocumented_unsafe_blocks.stderr @@ -319,12 +319,28 @@ LL | let bar = unsafe {}; = help: consider adding a safety comment on the preceding line error: unsafe block missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:519:9 + --> $DIR/undocumented_unsafe_blocks.rs:523:9 | LL | unsafe { a_function_with_a_very_long_name_to_break_the_line() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: consider adding a safety comment on the preceding line -error: aborting due to 37 previous errors +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:527:9 + | +LL | unsafe { a_const_function_with_a_very_long_name_to_break_the_line() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:531:9 + | +LL | unsafe { a_const_function_with_a_very_long_name_to_break_the_line() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: aborting due to 39 previous errors