Skip to content

Lint unnecessary safety comments #9851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4451,6 +4451,7 @@ Released 2018-09-13
[`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed
[`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation
[`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings
[`unnecessary_safety_comment`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_comment
[`unnecessary_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_doc
[`unnecessary_self_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_self_imports
[`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::types::TYPE_COMPLEXITY_INFO,
crate::types::VEC_BOX_INFO,
crate::undocumented_unsafe_blocks::UNDOCUMENTED_UNSAFE_BLOCKS_INFO,
crate::undocumented_unsafe_blocks::UNNECESSARY_SAFETY_COMMENT_INFO,
crate::unicode::INVISIBLE_CHARACTERS_INFO,
crate::unicode::NON_ASCII_LITERAL_INFO,
crate::unicode::UNICODE_NOT_NFC_INFO,
Expand Down
429 changes: 332 additions & 97 deletions clippy_lints/src/undocumented_unsafe_blocks.rs

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions clippy_utils/src/visitors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,36 +170,36 @@ where
cb: F,
}

struct WithStmtGuarg<'a, F> {
struct WithStmtGuard<'a, F> {
val: &'a mut RetFinder<F>,
prev_in_stmt: bool,
}

impl<F> RetFinder<F> {
fn inside_stmt(&mut self, in_stmt: bool) -> WithStmtGuarg<'_, F> {
fn inside_stmt(&mut self, in_stmt: bool) -> WithStmtGuard<'_, F> {
let prev_in_stmt = std::mem::replace(&mut self.in_stmt, in_stmt);
WithStmtGuarg {
WithStmtGuard {
val: self,
prev_in_stmt,
}
}
}

impl<F> std::ops::Deref for WithStmtGuarg<'_, F> {
impl<F> std::ops::Deref for WithStmtGuard<'_, F> {
type Target = RetFinder<F>;

fn deref(&self) -> &Self::Target {
self.val
}
}

impl<F> std::ops::DerefMut for WithStmtGuarg<'_, F> {
impl<F> std::ops::DerefMut for WithStmtGuard<'_, F> {
fn deref_mut(&mut self) -> &mut Self::Target {
self.val
}
}

impl<F> Drop for WithStmtGuarg<'_, F> {
impl<F> Drop for WithStmtGuard<'_, F> {
fn drop(&mut self) {
self.val.in_stmt = self.prev_in_stmt;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/undocumented_unsafe_blocks.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// aux-build:proc_macro_unsafe.rs

#![warn(clippy::undocumented_unsafe_blocks)]
#![warn(clippy::undocumented_unsafe_blocks, clippy::unnecessary_safety_comment)]
#![allow(clippy::let_unit_value, clippy::missing_safety_doc)]

extern crate proc_macro_unsafe;
Expand Down
33 changes: 32 additions & 1 deletion tests/ui/undocumented_unsafe_blocks.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,19 @@ LL | unsafe impl TrailingComment for () {} // SAFETY:
|
= help: consider adding a safety comment on the preceding line

error: constant item has unnecessary safety comment
--> $DIR/undocumented_unsafe_blocks.rs:471:5
|
LL | const BIG_NUMBER: i32 = 1000000;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: consider removing the safety comment
--> $DIR/undocumented_unsafe_blocks.rs:470:5
|
LL | // SAFETY:
| ^^^^^^^^^^
= note: `-D clippy::unnecessary-safety-comment` implied by `-D warnings`

error: unsafe impl missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:472:5
|
Expand Down Expand Up @@ -271,6 +284,24 @@ LL | unsafe {};
|
= help: consider adding a safety comment on the preceding line

error: statement has unnecessary safety comment
--> $DIR/undocumented_unsafe_blocks.rs:501:5
|
LL | / let _ = {
LL | | if unsafe { true } {
LL | | todo!();
LL | | } else {
... |
LL | | }
LL | | };
| |______^
|
help: consider removing the safety comment
--> $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:502:12
|
Expand All @@ -287,5 +318,5 @@ LL | let bar = unsafe {};
|
= help: consider adding a safety comment on the preceding line

error: aborting due to 34 previous errors
error: aborting due to 36 previous errors

51 changes: 51 additions & 0 deletions tests/ui/unnecessary_safety_comment.rs
Original file line number Diff line number Diff line change
@@ -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() {}
115 changes: 115 additions & 0 deletions tests/ui/unnecessary_safety_comment.stderr
Original file line number Diff line number Diff line change
@@ -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