Auto merge of #9851 - Veykril:unnecessary-safety-comment, r=giraffate

Lint unnecessary safety comments

changelog: [`unnecessary_safety_comment`]: Add unnecessary safety comment lint

Addresses https://github.com/rust-lang/rust-clippy/issues/7954

This does not necessarily catch all occurences, as doing so would require checking all expressions in the entire source which seems rather expensive. Instead what the lint does is it checks items, statements and the tail expression of blocks for safety comments, then checks if those comments are necessary or not, then linting for the unnecessary ones.

I kept the tests in one file to check that the lints do not clash with each other.

CHANGELOG.md

@@ -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

clippy_lints/src/declared_lints.rs

@@ -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,

clippy_lints/src/undocumented_unsafe_blocks.rs

@@ -1,6 +1,10 @@
+use std::ops::ControlFlow;
+
 use clippy_utils::diagnostics::span_lint_and_help;
 use clippy_utils::source::walk_span_to_context;
+use clippy_utils::visitors::{for_each_expr_with_closures, Descend};
 use clippy_utils::{get_parent_node, is_lint_allowed};
+use hir::HirId;
 use rustc_data_structures::sync::Lrc;
 use rustc_hir as hir;
 use rustc_hir::{Block, BlockCheckMode, ItemKind, Node, UnsafeSource};
@@ -59,11 +63,39 @@ declare_clippy_lint! {
     restriction,
     "creating an unsafe block without explaining why it is safe"
 }
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `// SAFETY: ` comments on safe code.
+    ///
+    /// ### Why is this bad?
+    /// Safe code has no safety requirements, so there is no need to
+    /// describe safety invariants.
+    ///
+    /// ### Example
+    /// ```rust
+    /// use std::ptr::NonNull;
+    /// let a = &mut 42;
+    ///
+    /// // SAFETY: references are guaranteed to be non-null.
+    /// let ptr = NonNull::new(a).unwrap();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// use std::ptr::NonNull;
+    /// let a = &mut 42;
+    ///
+    /// let ptr = NonNull::new(a).unwrap();
+    /// ```
+    #[clippy::version = "1.67.0"]
+    pub UNNECESSARY_SAFETY_COMMENT,
+    restriction,
+    "annotating safe code with a safety comment"
+}
 
-declare_lint_pass!(UndocumentedUnsafeBlocks => [UNDOCUMENTED_UNSAFE_BLOCKS]);
+declare_lint_pass!(UndocumentedUnsafeBlocks => [UNDOCUMENTED_UNSAFE_BLOCKS, UNNECESSARY_SAFETY_COMMENT]);
 
-impl LateLintPass<'_> for UndocumentedUnsafeBlocks {
+impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks {
-    fn check_block(&mut self, cx: &LateContext<'_>, block: &'_ Block<'_>) {
+    fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) {
         if block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided)
             && !in_external_macro(cx.tcx.sess, block.span)
             && !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, block.hir_id)
@@ -87,15 +119,69 @@ impl LateLintPass<'_> for UndocumentedUnsafeBlocks {
                 "consider adding a safety comment on the preceding line",
             );
         }
+
+        if let Some(tail) = block.expr
+            && !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, tail.hir_id)
+            && !in_external_macro(cx.tcx.sess, tail.span)
+            && let HasSafetyComment::Yes(pos) = stmt_has_safety_comment(cx, tail.span, tail.hir_id)
+            && let Some(help_span) = expr_has_unnecessary_safety_comment(cx, tail, pos)
+        {
+            span_lint_and_help(
+                cx,
+                UNNECESSARY_SAFETY_COMMENT,
+                tail.span,
+                "expression has unnecessary safety comment",
+                Some(help_span),
+                "consider removing the safety comment",
+            );
+        }
+    }
+
+    fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &hir::Stmt<'tcx>) {
+        let (
+            hir::StmtKind::Local(&hir::Local { init: Some(expr), .. })
+            | hir::StmtKind::Expr(expr)
+            | 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)
+            && let Some(help_span) = expr_has_unnecessary_safety_comment(cx, expr, pos)
+        {
+            span_lint_and_help(
+                cx,
+                UNNECESSARY_SAFETY_COMMENT,
+                stmt.span,
+                "statement has unnecessary safety comment",
+                Some(help_span),
+                "consider removing the safety comment",
+            );
+        }
     }
 
     fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) {
-        if let hir::ItemKind::Impl(imple) = item.kind
+        if in_external_macro(cx.tcx.sess, item.span) {
-            && imple.unsafety == hir::Unsafety::Unsafe
+            return;
-            && !in_external_macro(cx.tcx.sess, item.span)
+        }
-            && !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, item.hir_id())
+
+        let mk_spans = |pos: BytePos| {
+            let source_map = cx.tcx.sess.source_map();
+            let span = Span::new(pos, pos, SyntaxContext::root(), None);
+            let help_span = source_map.span_extend_to_next_char(span, '\n', true);
+            let span = if source_map.is_multiline(item.span) {
+                source_map.span_until_char(item.span, '\n')
+            } else {
+                item.span
+            };
+            (span, help_span)
+        };
+
+        let item_has_safety_comment = item_has_safety_comment(cx, item);
+        match (&item.kind, item_has_safety_comment) {
+            // lint unsafe impl without safety comment
+            (hir::ItemKind::Impl(impl_), HasSafetyComment::No) if impl_.unsafety == hir::Unsafety::Unsafe => {
+                if !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, item.hir_id())
                     && !is_unsafe_from_proc_macro(cx, item.span)
-            && !item_has_safety_comment(cx, item)
                 {
                     let source_map = cx.tcx.sess.source_map();
                     let span = if source_map.is_multiline(item.span) {
@@ -113,7 +199,93 @@ impl LateLintPass<'_> for UndocumentedUnsafeBlocks {
                         "consider adding a safety comment on the preceding line",
                     );
                 }
+            },
+            // lint safe impl with unnecessary safety comment
+            (hir::ItemKind::Impl(impl_), HasSafetyComment::Yes(pos)) if impl_.unsafety == hir::Unsafety::Normal => {
+                if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, item.hir_id()) {
+                    let (span, help_span) = mk_spans(pos);
+
+                    span_lint_and_help(
+                        cx,
+                        UNNECESSARY_SAFETY_COMMENT,
+                        span,
+                        "impl has unnecessary safety comment",
+                        Some(help_span),
+                        "consider removing the safety comment",
+                    );
                 }
+            },
+            (hir::ItemKind::Impl(_), _) => {},
+            // const and static items only need a safety comment if their body is an unsafe block, lint otherwise
+            (&hir::ItemKind::Const(.., body) | &hir::ItemKind::Static(.., body), HasSafetyComment::Yes(pos)) => {
+                if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, body.hir_id) {
+                    let body = cx.tcx.hir().body(body);
+                    if !matches!(
+                        body.value.kind, hir::ExprKind::Block(block, _)
+                        if block.rules == BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided)
+                    ) {
+                        let (span, help_span) = mk_spans(pos);
+
+                        span_lint_and_help(
+                            cx,
+                            UNNECESSARY_SAFETY_COMMENT,
+                            span,
+                            &format!("{} has unnecessary safety comment", item.kind.descr()),
+                            Some(help_span),
+                            "consider removing the safety comment",
+                        );
+                    }
+                }
+            },
+            // Aside from unsafe impls and consts/statics with an unsafe block, items in general
+            // do not have safety invariants that need to be documented, so lint those.
+            (_, HasSafetyComment::Yes(pos)) => {
+                if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, item.hir_id()) {
+                    let (span, help_span) = mk_spans(pos);
+
+                    span_lint_and_help(
+                        cx,
+                        UNNECESSARY_SAFETY_COMMENT,
+                        span,
+                        &format!("{} has unnecessary safety comment", item.kind.descr()),
+                        Some(help_span),
+                        "consider removing the safety comment",
+                    );
+                }
+            },
+            _ => (),
+        }
+    }
+}
+
+fn expr_has_unnecessary_safety_comment<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx hir::Expr<'tcx>,
+    comment_pos: BytePos,
+) -> Option<Span> {
+    // this should roughly be the reverse of `block_parents_have_safety_comment`
+    if for_each_expr_with_closures(cx, expr, |expr| match expr.kind {
+        hir::ExprKind::Block(
+            Block {
+                rules: BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided),
+                ..
+            },
+            _,
+        ) => ControlFlow::Break(()),
+        // statements will be handled by check_stmt itself again
+        hir::ExprKind::Block(..) => ControlFlow::Continue(Descend::No),
+        _ => ControlFlow::Continue(Descend::Yes),
+    })
+    .is_some()
+    {
+        return None;
+    }
+
+    let source_map = cx.tcx.sess.source_map();
+    let span = Span::new(comment_pos, comment_pos, SyntaxContext::root(), None);
+    let help_span = source_map.span_extend_to_next_char(span, '\n', true);
+
+    Some(help_span)
 }
 
 fn is_unsafe_from_proc_macro(cx: &LateContext<'_>, span: Span) -> bool {
@@ -170,47 +342,53 @@ fn block_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
     // won't work. This is to avoid dealing with where such a comment should be place relative to
     // attributes and doc comments.
 
-    span_from_macro_expansion_has_safety_comment(cx, span) || span_in_body_has_safety_comment(cx, span)
+    matches!(
+        span_from_macro_expansion_has_safety_comment(cx, span),
+        HasSafetyComment::Yes(_)
+    ) || span_in_body_has_safety_comment(cx, span)
+}
+
+enum HasSafetyComment {
+    Yes(BytePos),
+    No,
+    Maybe,
 }
 
 /// 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 {
+fn item_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) -> HasSafetyComment {
-    if span_from_macro_expansion_has_safety_comment(cx, item.span) {
+    match span_from_macro_expansion_has_safety_comment(cx, item.span) {
-        return true;
+        HasSafetyComment::Maybe => (),
+        has_safety_comment => return has_safety_comment,
     }
 
-    if item.span.ctxt() == SyntaxContext::root() {
+    if item.span.ctxt() != SyntaxContext::root() {
+        return HasSafetyComment::No;
+    }
     if let Some(parent_node) = get_parent_node(cx.tcx, item.hir_id()) {
         let comment_start = match parent_node {
             Node::Crate(parent_mod) => {
-                    comment_start_before_impl_in_mod(cx, parent_mod, parent_mod.spans.inner_span, item)
+                comment_start_before_item_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_before_impl_in_mod(cx, parent_mod, parent_item.span, item)
+                    comment_start_before_item_in_mod(cx, parent_mod, parent_item.span, item)
                 } else {
                     // Doesn't support impls in this position. Pretend a comment was found.
-                        return true;
+                    return HasSafetyComment::Maybe;
                 }
             },
             Node::Stmt(stmt) => {
-                    if let Some(stmt_parent) = get_parent_node(cx.tcx, stmt.hir_id) {
+                if let Some(Node::Block(block)) = get_parent_node(cx.tcx, stmt.hir_id) {
-                        match stmt_parent {
+                    walk_span_to_context(block.span, SyntaxContext::root()).map(Span::lo)
-                            Node::Block(block) => 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 {
                     // Problem getting the parent node. Pretend a comment was found.
-                        return true;
+                    return HasSafetyComment::Maybe;
                 }
             },
             _ => {
                 // Doesn't support impls in this position. Pretend a comment was found.
-                    return true;
+                return HasSafetyComment::Maybe;
             },
         };
 
@@ -221,34 +399,77 @@ fn item_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) -> bool {
             && Lrc::ptr_eq(&unsafe_line.sf, &comment_start_line.sf)
             && let Some(src) = unsafe_line.sf.src.as_deref()
         {
-                unsafe_line.sf.lines(|lines| {
+            return unsafe_line.sf.lines(|lines| {
-                    comment_start_line.line < unsafe_line.line && text_has_safety_comment(
+                if comment_start_line.line >= unsafe_line.line {
+                    HasSafetyComment::No
+                } else {
+                    match text_has_safety_comment(
                         src,
                         &lines[comment_start_line.line + 1..=unsafe_line.line],
                         unsafe_line.sf.start_pos.to_usize(),
-                    )
+                    ) {
-                })
+                        Some(b) => HasSafetyComment::Yes(b),
-            } else {
+                        None => HasSafetyComment::No,
-                // Problem getting source text. Pretend a comment was found.
-                true
                     }
-        } else {
-            // No parent node. Pretend a comment was found.
-            true
                 }
-    } else {
+            });
-        false
         }
     }
+    HasSafetyComment::Maybe
+}
 
-fn comment_start_before_impl_in_mod(
+/// Checks if the lines immediately preceding the item contain a safety comment.
+#[allow(clippy::collapsible_match)]
+fn stmt_has_safety_comment(cx: &LateContext<'_>, span: Span, hir_id: HirId) -> HasSafetyComment {
+    match span_from_macro_expansion_has_safety_comment(cx, span) {
+        HasSafetyComment::Maybe => (),
+        has_safety_comment => return has_safety_comment,
+    }
+
+    if span.ctxt() != SyntaxContext::root() {
+        return HasSafetyComment::No;
+    }
+
+    if let Some(parent_node) = get_parent_node(cx.tcx, hir_id) {
+        let comment_start = match parent_node {
+            Node::Block(block) => walk_span_to_context(block.span, SyntaxContext::root()).map(Span::lo),
+            _ => return HasSafetyComment::Maybe,
+        };
+
+        let source_map = cx.sess().source_map();
+        if let Some(comment_start) = comment_start
+            && let Ok(unsafe_line) = source_map.lookup_line(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()
+        {
+            return unsafe_line.sf.lines(|lines| {
+                if comment_start_line.line >= unsafe_line.line {
+                    HasSafetyComment::No
+                } else {
+                    match text_has_safety_comment(
+                        src,
+                        &lines[comment_start_line.line + 1..=unsafe_line.line],
+                        unsafe_line.sf.start_pos.to_usize(),
+                    ) {
+                        Some(b) => HasSafetyComment::Yes(b),
+                        None => HasSafetyComment::No,
+                    }
+                }
+            });
+        }
+    }
+    HasSafetyComment::Maybe
+}
+
+fn comment_start_before_item_in_mod(
     cx: &LateContext<'_>,
     parent_mod: &hir::Mod<'_>,
     parent_mod_span: Span,
-    imple: &hir::Item<'_>,
+    item: &hir::Item<'_>,
 ) -> Option<BytePos> {
     parent_mod.item_ids.iter().enumerate().find_map(|(idx, item_id)| {
-        if *item_id == imple.item_id() {
+        if *item_id == item.item_id() {
             if idx == 0 {
                 // mod A { /* comment */ unsafe impl T {} ... }
                 // ^------------------------------------------^ returns the start of this span
@@ -270,11 +491,11 @@ fn comment_start_before_impl_in_mod(
     })
 }
 
-fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
+fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span) -> HasSafetyComment {
     let source_map = cx.sess().source_map();
     let ctxt = span.ctxt();
     if ctxt == SyntaxContext::root() {
-        false
+        HasSafetyComment::Maybe
     } else {
         // From a macro expansion. Get the text from the start of the macro declaration to start of the
         // unsafe block.
@@ -286,15 +507,22 @@ fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span
             && let Some(src) = unsafe_line.sf.src.as_deref()
         {
             unsafe_line.sf.lines(|lines| {
-                macro_line.line < unsafe_line.line && text_has_safety_comment(
+                if macro_line.line < unsafe_line.line {
+                    match text_has_safety_comment(
                         src,
                         &lines[macro_line.line + 1..=unsafe_line.line],
                         unsafe_line.sf.start_pos.to_usize(),
-                )
+                    ) {
+                        Some(b) => HasSafetyComment::Yes(b),
+                        None => HasSafetyComment::No,
+                    }
+                } else {
+                    HasSafetyComment::No
+                }
             })
         } else {
             // Problem getting source text. Pretend a comment was found.
-            true
+            HasSafetyComment::Maybe
         }
     }
 }
@@ -333,7 +561,7 @@ fn span_in_body_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
                     src,
                     &lines[body_line.line + 1..=unsafe_line.line],
                     unsafe_line.sf.start_pos.to_usize(),
-                )
+                ).is_some()
             })
         } else {
             // Problem getting source text. Pretend a comment was found.
@@ -345,30 +573,34 @@ fn span_in_body_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
 }
 
 /// Checks if the given text has a safety comment for the immediately proceeding line.
-fn text_has_safety_comment(src: &str, line_starts: &[BytePos], offset: usize) -> bool {
+fn text_has_safety_comment(src: &str, line_starts: &[BytePos], offset: usize) -> Option<BytePos> {
     let mut lines = line_starts
         .array_windows::<2>()
         .rev()
         .map_while(|[start, end]| {
             let start = start.to_usize() - offset;
             let end = end.to_usize() - offset;
-            src.get(start..end).map(|text| (start, text.trim_start()))
+            let text = src.get(start..end)?;
+            let trimmed = text.trim_start();
+            Some((start + (text.len() - trimmed.len()), trimmed))
         })
         .filter(|(_, text)| !text.is_empty());
 
     let Some((line_start, line)) = lines.next() else {
-        return false;
+        return None;
     };
     // Check for a sequence of line comments.
     if line.starts_with("//") {
-        let mut line = line;
+        let (mut line, mut line_start) = (line, line_start);
         loop {
             if line.to_ascii_uppercase().contains("SAFETY:") {
-                return true;
+                return Some(BytePos(
+                    u32::try_from(line_start).unwrap() + u32::try_from(offset).unwrap(),
+                ));
             }
             match lines.next() {
-                Some((_, x)) if x.starts_with("//") => line = x,
+                Some((s, x)) if x.starts_with("//") => (line, line_start) = (x, s),
-                _ => return false,
+                _ => return None,
             }
         }
     }
@@ -377,16 +609,19 @@ fn text_has_safety_comment(src: &str, line_starts: &[BytePos], offset: usize) ->
     let (mut line_start, mut line) = (line_start, line);
     loop {
         if line.starts_with("/*") {
-            let src = src[line_start..line_starts.last().unwrap().to_usize() - offset].trim_start();
+            let src = &src[line_start..line_starts.last().unwrap().to_usize() - offset];
             let mut tokens = tokenize(src);
-            return src[..tokens.next().unwrap().len as usize]
+            return (src[..tokens.next().unwrap().len as usize]
                 .to_ascii_uppercase()
                 .contains("SAFETY:")
-                && tokens.all(|t| t.kind == TokenKind::Whitespace);
+                && tokens.all(|t| t.kind == TokenKind::Whitespace))
+            .then_some(BytePos(
+                u32::try_from(line_start).unwrap() + u32::try_from(offset).unwrap(),
+            ));
         }
         match lines.next() {
             Some(x) => (line_start, line) = x,
-            None => return false,
+            None => return None,
         }
     }
 }

clippy_utils/src/visitors.rs

@@ -170,22 +170,22 @@ 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 {
@@ -193,13 +193,13 @@ where
         }
     }
 
-    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;
         }

tests/ui/undocumented_unsafe_blocks.rs

@@ -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;

tests/ui/undocumented_unsafe_blocks.stderr

@@ -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
    |
@@ -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
    |
@@ -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
 

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() {}

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
+