From f96dd383188e9f24e5b401e664cca97b8bd73825 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 24 Nov 2022 09:47:50 +0100 Subject: [PATCH] Address reviews --- .../src/undocumented_unsafe_blocks.rs | 9 +- tests/ui/undocumented_unsafe_blocks.rs | 44 ------- tests/ui/undocumented_unsafe_blocks.stderr | 116 ++---------------- tests/ui/unnecessary_safety_comment.rs | 51 ++++++++ tests/ui/unnecessary_safety_comment.stderr | 115 +++++++++++++++++ 5 files changed, 178 insertions(+), 157 deletions(-) create mode 100644 tests/ui/unnecessary_safety_comment.rs create mode 100644 tests/ui/unnecessary_safety_comment.stderr diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index d7e48342306..2e1b6d8d4ea 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -89,7 +89,7 @@ declare_clippy_lint! { #[clippy::version = "1.67.0"] pub UNNECESSARY_SAFETY_COMMENT, restriction, - "creating an unsafe block without explaining why it is safe" + "annotating safe code with a safety comment" } declare_lint_pass!(UndocumentedUnsafeBlocks => [UNDOCUMENTED_UNSAFE_BLOCKS, UNNECESSARY_SAFETY_COMMENT]); @@ -138,12 +138,11 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks { } fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &hir::Stmt<'tcx>) { - let expr = match stmt.kind { + let ( hir::StmtKind::Local(&hir::Local { init: Some(expr), .. }) | hir::StmtKind::Expr(expr) - | hir::StmtKind::Semi(expr) => expr, - _ => return, - }; + | hir::StmtKind::Semi(expr) + ) = stmt.kind else { return }; if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, stmt.hir_id) && !in_external_macro(cx.tcx.sess, stmt.span) && let HasSafetyComment::Yes(pos) = stmt_has_safety_comment(cx, stmt.span, stmt.hir_id) diff --git a/tests/ui/undocumented_unsafe_blocks.rs b/tests/ui/undocumented_unsafe_blocks.rs index cb99ce0d421..c05eb447b2e 100644 --- a/tests/ui/undocumented_unsafe_blocks.rs +++ b/tests/ui/undocumented_unsafe_blocks.rs @@ -472,19 +472,6 @@ mod unsafe_impl_invalid_comment { unsafe impl Interference for () {} } -mod unsafe_items_invalid_comment { - // SAFETY: - const CONST: u32 = 0; - // SAFETY: - static STATIC: u32 = 0; - // SAFETY: - struct Struct; - // SAFETY: - enum Enum {} - // SAFETY: - mod module {} -} - unsafe trait ImplInFn {} fn impl_in_fn() { @@ -522,35 +509,4 @@ fn issue_9142() { }; } -mod unnecessary_from_macro { - trait T {} - - macro_rules! no_safety_comment { - ($t:ty) => { - impl T for $t {} - }; - } - - // FIXME: This is not caught - // Safety: unnecessary - no_safety_comment!(()); - - macro_rules! with_safety_comment { - ($t:ty) => { - // Safety: unnecessary - impl T for $t {} - }; - } - - with_safety_comment!(i32); -} - -fn unnecessary_on_stmt_and_expr() -> u32 { - // SAFETY: unnecessary - let num = 42; - - // SAFETY: unnecessary - 24 -} - fn main() {} diff --git a/tests/ui/undocumented_unsafe_blocks.stderr b/tests/ui/undocumented_unsafe_blocks.stderr index 919fd51351c..d1c1bb5ffea 100644 --- a/tests/ui/undocumented_unsafe_blocks.stderr +++ b/tests/ui/undocumented_unsafe_blocks.stderr @@ -260,68 +260,8 @@ LL | unsafe impl Interference for () {} | = help: consider adding a safety comment on the preceding line -error: constant item has unnecessary safety comment - --> $DIR/undocumented_unsafe_blocks.rs:477:5 - | -LL | const CONST: u32 = 0; - | ^^^^^^^^^^^^^^^^^^^^^ - | -help: consider removing the safety comment - --> $DIR/undocumented_unsafe_blocks.rs:476:5 - | -LL | // SAFETY: - | ^^^^^^^^^^ - -error: static item has unnecessary safety comment - --> $DIR/undocumented_unsafe_blocks.rs:479:5 - | -LL | static STATIC: u32 = 0; - | ^^^^^^^^^^^^^^^^^^^^^^^ - | -help: consider removing the safety comment - --> $DIR/undocumented_unsafe_blocks.rs:478:5 - | -LL | // SAFETY: - | ^^^^^^^^^^ - -error: struct has unnecessary safety comment - --> $DIR/undocumented_unsafe_blocks.rs:481:5 - | -LL | struct Struct; - | ^^^^^^^^^^^^^^ - | -help: consider removing the safety comment - --> $DIR/undocumented_unsafe_blocks.rs:480:5 - | -LL | // SAFETY: - | ^^^^^^^^^^ - -error: enum has unnecessary safety comment - --> $DIR/undocumented_unsafe_blocks.rs:483:5 - | -LL | enum Enum {} - | ^^^^^^^^^^^^ - | -help: consider removing the safety comment - --> $DIR/undocumented_unsafe_blocks.rs:482:5 - | -LL | // SAFETY: - | ^^^^^^^^^^ - -error: module has unnecessary safety comment - --> $DIR/undocumented_unsafe_blocks.rs:485:5 - | -LL | mod module {} - | ^^^^^^^^^^^^^ - | -help: consider removing the safety comment - --> $DIR/undocumented_unsafe_blocks.rs:484:5 - | -LL | // SAFETY: - | ^^^^^^^^^^ - error: unsafe impl missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:492:5 + --> $DIR/undocumented_unsafe_blocks.rs:479:5 | LL | unsafe impl ImplInFn for () {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -329,7 +269,7 @@ 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:501:1 + --> $DIR/undocumented_unsafe_blocks.rs:488:1 | LL | unsafe impl CrateRoot for () {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -337,7 +277,7 @@ LL | unsafe impl CrateRoot for () {} = help: consider adding a safety comment on the preceding line error: unsafe block missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:511:9 + --> $DIR/undocumented_unsafe_blocks.rs:498:9 | LL | unsafe {}; | ^^^^^^^^^ @@ -345,7 +285,7 @@ LL | unsafe {}; = help: consider adding a safety comment on the preceding line error: statement has unnecessary safety comment - --> $DIR/undocumented_unsafe_blocks.rs:514:5 + --> $DIR/undocumented_unsafe_blocks.rs:501:5 | LL | / let _ = { LL | | if unsafe { true } { @@ -357,13 +297,13 @@ LL | | }; | |______^ | help: consider removing the safety comment - --> $DIR/undocumented_unsafe_blocks.rs:513:5 + --> $DIR/undocumented_unsafe_blocks.rs:500:5 | LL | // SAFETY: this is more than one level away, so it should warn | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe block missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:515:12 + --> $DIR/undocumented_unsafe_blocks.rs:502:12 | LL | if unsafe { true } { | ^^^^^^^^^^^^^^^ @@ -371,52 +311,12 @@ LL | if unsafe { true } { = help: consider adding a safety comment on the preceding line error: unsafe block missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:518:23 + --> $DIR/undocumented_unsafe_blocks.rs:505:23 | LL | let bar = unsafe {}; | ^^^^^^^^^ | = help: consider adding a safety comment on the preceding line -error: impl has unnecessary safety comment - --> $DIR/undocumented_unsafe_blocks.rs:541:13 - | -LL | impl T for $t {} - | ^^^^^^^^^^^^^^^^ -... -LL | with_safety_comment!(i32); - | ------------------------- in this macro invocation - | -help: consider removing the safety comment - --> $DIR/undocumented_unsafe_blocks.rs:540:13 - | -LL | // Safety: unnecessary - | ^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in the macro `with_safety_comment` (in Nightly builds, run with -Z macro-backtrace for more info) - -error: expression has unnecessary safety comment - --> $DIR/undocumented_unsafe_blocks.rs:553:5 - | -LL | 24 - | ^^ - | -help: consider removing the safety comment - --> $DIR/undocumented_unsafe_blocks.rs:552:5 - | -LL | // SAFETY: unnecessary - | ^^^^^^^^^^^^^^^^^^^^^^ - -error: statement has unnecessary safety comment - --> $DIR/undocumented_unsafe_blocks.rs:550:5 - | -LL | let num = 42; - | ^^^^^^^^^^^^^ - | -help: consider removing the safety comment - --> $DIR/undocumented_unsafe_blocks.rs:549:5 - | -LL | // SAFETY: unnecessary - | ^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 44 previous errors +error: aborting due to 36 previous errors diff --git a/tests/ui/unnecessary_safety_comment.rs b/tests/ui/unnecessary_safety_comment.rs new file mode 100644 index 00000000000..7fefea7051d --- /dev/null +++ b/tests/ui/unnecessary_safety_comment.rs @@ -0,0 +1,51 @@ +#![warn(clippy::undocumented_unsafe_blocks, clippy::unnecessary_safety_comment)] +#![allow(clippy::let_unit_value, clippy::missing_safety_doc)] + +mod unsafe_items_invalid_comment { + // SAFETY: + const CONST: u32 = 0; + // SAFETY: + static STATIC: u32 = 0; + // SAFETY: + struct Struct; + // SAFETY: + enum Enum {} + // SAFETY: + mod module {} +} + +mod unnecessary_from_macro { + trait T {} + + macro_rules! no_safety_comment { + ($t:ty) => { + impl T for $t {} + }; + } + + // FIXME: This is not caught + // Safety: unnecessary + no_safety_comment!(()); + + macro_rules! with_safety_comment { + ($t:ty) => { + // Safety: unnecessary + impl T for $t {} + }; + } + + with_safety_comment!(i32); +} + +fn unnecessary_on_stmt_and_expr() -> u32 { + // SAFETY: unnecessary + let num = 42; + + // SAFETY: unnecessary + if num > 24 {} + + // SAFETY: unnecessary + 24 +} + +fn main() {} diff --git a/tests/ui/unnecessary_safety_comment.stderr b/tests/ui/unnecessary_safety_comment.stderr new file mode 100644 index 00000000000..7b2af67d64c --- /dev/null +++ b/tests/ui/unnecessary_safety_comment.stderr @@ -0,0 +1,115 @@ +error: constant item has unnecessary safety comment + --> $DIR/unnecessary_safety_comment.rs:6:5 + | +LL | const CONST: u32 = 0; + | ^^^^^^^^^^^^^^^^^^^^^ + | +help: consider removing the safety comment + --> $DIR/unnecessary_safety_comment.rs:5:5 + | +LL | // SAFETY: + | ^^^^^^^^^^ + = note: `-D clippy::unnecessary-safety-comment` implied by `-D warnings` + +error: static item has unnecessary safety comment + --> $DIR/unnecessary_safety_comment.rs:8:5 + | +LL | static STATIC: u32 = 0; + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider removing the safety comment + --> $DIR/unnecessary_safety_comment.rs:7:5 + | +LL | // SAFETY: + | ^^^^^^^^^^ + +error: struct has unnecessary safety comment + --> $DIR/unnecessary_safety_comment.rs:10:5 + | +LL | struct Struct; + | ^^^^^^^^^^^^^^ + | +help: consider removing the safety comment + --> $DIR/unnecessary_safety_comment.rs:9:5 + | +LL | // SAFETY: + | ^^^^^^^^^^ + +error: enum has unnecessary safety comment + --> $DIR/unnecessary_safety_comment.rs:12:5 + | +LL | enum Enum {} + | ^^^^^^^^^^^^ + | +help: consider removing the safety comment + --> $DIR/unnecessary_safety_comment.rs:11:5 + | +LL | // SAFETY: + | ^^^^^^^^^^ + +error: module has unnecessary safety comment + --> $DIR/unnecessary_safety_comment.rs:14:5 + | +LL | mod module {} + | ^^^^^^^^^^^^^ + | +help: consider removing the safety comment + --> $DIR/unnecessary_safety_comment.rs:13:5 + | +LL | // SAFETY: + | ^^^^^^^^^^ + +error: impl has unnecessary safety comment + --> $DIR/unnecessary_safety_comment.rs:33:13 + | +LL | impl T for $t {} + | ^^^^^^^^^^^^^^^^ +... +LL | with_safety_comment!(i32); + | ------------------------- in this macro invocation + | +help: consider removing the safety comment + --> $DIR/unnecessary_safety_comment.rs:32:13 + | +LL | // Safety: unnecessary + | ^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in the macro `with_safety_comment` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: expression has unnecessary safety comment + --> $DIR/unnecessary_safety_comment.rs:48:5 + | +LL | 24 + | ^^ + | +help: consider removing the safety comment + --> $DIR/unnecessary_safety_comment.rs:47:5 + | +LL | // SAFETY: unnecessary + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: statement has unnecessary safety comment + --> $DIR/unnecessary_safety_comment.rs:42:5 + | +LL | let num = 42; + | ^^^^^^^^^^^^^ + | +help: consider removing the safety comment + --> $DIR/unnecessary_safety_comment.rs:41:5 + | +LL | // SAFETY: unnecessary + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: statement has unnecessary safety comment + --> $DIR/unnecessary_safety_comment.rs:45:5 + | +LL | if num > 24 {} + | ^^^^^^^^^^^^^^ + | +help: consider removing the safety comment + --> $DIR/unnecessary_safety_comment.rs:44:5 + | +LL | // SAFETY: unnecessary + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 9 previous errors +