diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index b37f2a27f905..87c3a89f1f8f 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -7,8 +7,8 @@ use clippy_utils::is_lint_allowed; use clippy_utils::source::walk_span_to_context; use clippy_utils::visitors::{Descend, for_each_expr}; use hir::HirId; -use rustc_hir as hir; -use rustc_hir::{Block, BlockCheckMode, Impl, ItemKind, Node, UnsafeSource}; +use rustc_errors::Applicability; +use rustc_hir::{self as hir, Block, BlockCheckMode, FnSig, Impl, ItemKind, Node, UnsafeSource}; use rustc_lexer::{FrontmatterAllowed, TokenKind, tokenize}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::impl_lint_pass; @@ -113,7 +113,7 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks { && !block.span.in_external_macro(cx.tcx.sess.source_map()) && !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, block.hir_id) && !is_unsafe_from_proc_macro(cx, block.span) - && !block_has_safety_comment(cx, block.span) + && !block_has_safety_comment(cx, block.span, self.accept_comment_above_attributes) && !block_parents_have_safety_comment( self.accept_comment_above_statement, self.accept_comment_above_attributes, @@ -143,7 +143,7 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks { if let Some(tail) = block.expr && !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, tail.hir_id) && !tail.span.in_external_macro(cx.tcx.sess.source_map()) - && let HasSafetyComment::Yes(pos) = + && let HasSafetyComment::Yes(pos, _) = stmt_has_safety_comment(cx, tail.span, tail.hir_id, self.accept_comment_above_attributes) && let Some(help_span) = expr_has_unnecessary_safety_comment(cx, tail, pos) { @@ -168,7 +168,7 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks { }; if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, stmt.hir_id) && !stmt.span.in_external_macro(cx.tcx.sess.source_map()) - && let HasSafetyComment::Yes(pos) = + && let HasSafetyComment::Yes(pos, _) = stmt_has_safety_comment(cx, stmt.span, stmt.hir_id, self.accept_comment_above_attributes) && let Some(help_span) = expr_has_unnecessary_safety_comment(cx, expr, pos) { @@ -191,8 +191,12 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks { 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 help_span = Span::new( + pos, + pos + BytePos(u32::try_from("SAFETY:".len()).unwrap()), + SyntaxContext::root(), + None, + ); let span = if source_map.is_multiline(item.span) { source_map.span_until_char(item.span, '\n') } else { @@ -201,16 +205,16 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks { (span, help_span) }; - let item_has_safety_comment = item_has_safety_comment(cx, item); + let item_has_safety_comment = item_has_safety_comment(cx, item, self.accept_comment_above_attributes); match item_has_safety_comment { - HasSafetyComment::Yes(pos) => check_has_safety_comment(cx, item, mk_spans(pos)), + HasSafetyComment::Yes(pos, is_doc) => check_has_safety_comment(cx, item, mk_spans(pos), is_doc), HasSafetyComment::No => check_has_no_safety_comment(cx, item), HasSafetyComment::Maybe => {}, } } } -fn check_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>, (span, help_span): (Span, Span)) { +fn check_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>, (span, help_span): (Span, Span), is_doc: bool) { match &item.kind { ItemKind::Impl(Impl { of_trait: Some(of_trait), @@ -252,6 +256,40 @@ fn check_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>, (span, h } } }, + // Unsafe functions with a SAFETY comment are suggested to change it to a `# Safety` comment + ItemKind::Fn { + sig: FnSig { header, .. }, + .. + } if header.is_unsafe() => { + if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, item.hir_id()) { + span_lint_and_then( + cx, + UNNECESSARY_SAFETY_COMMENT, + span, + format!( + "{} has unnecessary safety comment", + cx.tcx.def_descr(item.owner_id.to_def_id()), + ), + |diag| { + if is_doc { + // If it's already within a doc comment, we try to suggest the change + + diag.span_suggestion( + help_span, + "consider changing it to a `# Safety` section", + "# Safety", + Applicability::MachineApplicable, + ); + } else { + diag.span_help( + help_span, + "consider changing the `SAFETY` comment for a `# Safety` doc 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. _ => { @@ -272,6 +310,7 @@ fn check_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>, (span, h }, } } + fn check_has_no_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) { if let ItemKind::Impl(Impl { of_trait: Some(of_trait), @@ -407,21 +446,21 @@ fn block_parents_have_safety_comment( cx: &LateContext<'_>, id: HirId, ) -> bool { - let (span, hir_id) = match cx.tcx.parent_hir_node(id) { - Node::Expr(expr) if let Some(inner) = find_unsafe_block_parent_in_expr(cx, expr) => inner, + let span = match cx.tcx.parent_hir_node(id) { + Node::Expr(expr) if let Some((span, _)) = find_unsafe_block_parent_in_expr(cx, expr) => span, Node::Stmt(hir::Stmt { kind: - hir::StmtKind::Let(hir::LetStmt { span, hir_id, .. }) - | hir::StmtKind::Expr(hir::Expr { span, hir_id, .. }) - | hir::StmtKind::Semi(hir::Expr { span, hir_id, .. }), + hir::StmtKind::Let(hir::LetStmt { span, .. }) + | hir::StmtKind::Expr(hir::Expr { span, .. }) + | hir::StmtKind::Semi(hir::Expr { span, .. }), .. }) - | Node::LetStmt(hir::LetStmt { span, hir_id, .. }) => (*span, *hir_id), + | Node::LetStmt(hir::LetStmt { span, .. }) => *span, - node if let Some((span, hir_id)) = span_and_hid_of_item_alike_node(&node) + node if let Some((span, _)) = span_and_hid_of_item_alike_node(&node) && is_const_or_static(&node) => { - (span, hir_id) + span }, _ => return false, @@ -429,24 +468,7 @@ fn block_parents_have_safety_comment( // if unsafe block is part of a let/const/static statement, // and accept_comment_above_statement is set to true // we accept the safety comment in the line the precedes this statement. - accept_comment_above_statement - && span_with_attrs_has_safety_comment(cx, span, hir_id, accept_comment_above_attributes) -} - -/// Extends `span` to also include its attributes, then checks if that span has a safety comment. -fn span_with_attrs_has_safety_comment( - cx: &LateContext<'_>, - span: Span, - hir_id: HirId, - accept_comment_above_attributes: bool, -) -> bool { - let span = if accept_comment_above_attributes { - include_attrs_in_span(cx, hir_id, span) - } else { - span - }; - - span_has_safety_comment(cx, span) + accept_comment_above_statement && span_has_safety_comment(cx, span, accept_comment_above_attributes) } /// Checks if an expression is "branchy", e.g. loop, match/if/etc. @@ -458,7 +480,7 @@ fn is_branchy(expr: &hir::Expr<'_>) -> bool { } /// Checks if the lines immediately preceding the block contain a safety comment. -fn block_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool { +fn block_has_safety_comment(cx: &LateContext<'_>, span: Span, accept_comment_above_attributes: bool) -> bool { // This intentionally ignores text before the start of a function so something like: // ``` // // SAFETY: reason @@ -468,29 +490,30 @@ fn block_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool { // attributes and doc comments. matches!( - span_from_macro_expansion_has_safety_comment(cx, span), - HasSafetyComment::Yes(_) - ) || span_has_safety_comment(cx, span) -} - -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| { - if attr.is_doc_comment() { - return acc; - } - acc.to(attr.span()) - })) + span_from_macro_expansion_has_safety_comment(cx, span, accept_comment_above_attributes), + HasSafetyComment::Yes(_, _) + ) || span_has_safety_comment(cx, span, accept_comment_above_attributes) } +#[derive(Debug)] enum HasSafetyComment { - Yes(BytePos), + Yes(BytePos, bool), No, Maybe, } +enum SafetyComment { + Present { pos: BytePos, is_doc: bool }, + Absent, +} + /// Checks if the lines immediately preceding the item contain a safety comment. -fn item_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) -> HasSafetyComment { - match span_from_macro_expansion_has_safety_comment(cx, item.span) { +fn item_has_safety_comment( + cx: &LateContext<'_>, + item: &hir::Item<'_>, + accept_comment_above_attributes: bool, +) -> HasSafetyComment { + match span_from_macro_expansion_has_safety_comment(cx, item.span, accept_comment_above_attributes) { HasSafetyComment::Maybe => (), has_safety_comment => return has_safety_comment, } @@ -540,9 +563,10 @@ fn item_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) -> HasSaf &unsafe_line.sf.lines() [(comment_start_line.line + usize::from(!include_first_line_of_file))..=unsafe_line.line], unsafe_line.sf.start_pos, + accept_comment_above_attributes, ) { - Some(b) => HasSafetyComment::Yes(b), - None => HasSafetyComment::No, + SafetyComment::Present { pos, is_doc } => HasSafetyComment::Yes(pos, is_doc), + SafetyComment::Absent => HasSafetyComment::No, } }; } @@ -556,7 +580,7 @@ fn stmt_has_safety_comment( hir_id: HirId, accept_comment_above_attributes: bool, ) -> HasSafetyComment { - match span_from_macro_expansion_has_safety_comment(cx, span) { + match span_from_macro_expansion_has_safety_comment(cx, span, accept_comment_above_attributes) { HasSafetyComment::Maybe => (), has_safety_comment => return has_safety_comment, } @@ -570,13 +594,6 @@ fn stmt_has_safety_comment( _ => return HasSafetyComment::Maybe, }; - // if span_with_attrs_has_safety_comment(cx, span, hir_id, accept_comment_above_attrib - // } - let mut span = span; - if accept_comment_above_attributes { - span = include_attrs_in_span(cx, hir_id, span); - } - let source_map = cx.sess().source_map(); if let Some(comment_start) = comment_start && let Ok(unsafe_line) = source_map.lookup_line(span.lo()) @@ -591,9 +608,10 @@ fn stmt_has_safety_comment( src, &unsafe_line.sf.lines()[comment_start_line.line + 1..=unsafe_line.line], unsafe_line.sf.start_pos, + accept_comment_above_attributes, ) { - Some(b) => HasSafetyComment::Yes(b), - None => HasSafetyComment::No, + SafetyComment::Present { pos, is_doc } => HasSafetyComment::Yes(pos, is_doc), + SafetyComment::Absent => HasSafetyComment::No, } }; } @@ -647,7 +665,11 @@ fn comment_start_before_item_in_mod( }) } -fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span) -> HasSafetyComment { +fn span_from_macro_expansion_has_safety_comment( + cx: &LateContext<'_>, + span: Span, + accept_comment_above_attributes: bool, +) -> HasSafetyComment { let source_map = cx.sess().source_map(); let ctxt = span.ctxt(); if ctxt == SyntaxContext::root() { @@ -667,9 +689,10 @@ fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span src, &unsafe_line.sf.lines()[macro_line.line + 1..=unsafe_line.line], unsafe_line.sf.start_pos, + accept_comment_above_attributes, ) { - Some(b) => HasSafetyComment::Yes(b), - None => HasSafetyComment::No, + SafetyComment::Present { pos, is_doc } => HasSafetyComment::Yes(pos, is_doc), + SafetyComment::Absent => HasSafetyComment::No, } } else { HasSafetyComment::No @@ -713,7 +736,7 @@ fn get_body_search_span(cx: &LateContext<'_>) -> Option { None } -fn span_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool { +fn span_has_safety_comment(cx: &LateContext<'_>, span: Span, accept_comment_above_attributes: bool) -> bool { let source_map = cx.sess().source_map(); let ctxt = span.ctxt(); if ctxt.is_root() @@ -729,12 +752,15 @@ fn span_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool { // fn foo() { some_stuff; unsafe { stuff }; other_stuff; } // ^-------------^ body_line.line < unsafe_line.line - && text_has_safety_comment( - src, - &unsafe_line.sf.lines()[body_line.line + 1..=unsafe_line.line], - unsafe_line.sf.start_pos, + && matches!( + text_has_safety_comment( + src, + &unsafe_line.sf.lines()[body_line.line + 1..=unsafe_line.line], + unsafe_line.sf.start_pos, + accept_comment_above_attributes, + ), + SafetyComment::Present { .. } ) - .is_some() } else { // Problem getting source text. Pretend a comment was found. true @@ -745,7 +771,15 @@ fn span_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: &[RelativeBytePos], start_pos: BytePos) -> Option { +/// +/// If `accept_comment_above_attributes` is true, it will ignore attributes inbetween blocks of +/// comments +fn text_has_safety_comment( + src: &str, + line_starts: &[RelativeBytePos], + start_pos: BytePos, + accept_comment_above_attributes: bool, +) -> SafetyComment { let mut lines = line_starts .array_windows::<2>() .rev() @@ -756,9 +790,12 @@ fn text_has_safety_comment(src: &str, line_starts: &[RelativeBytePos], start_pos let trimmed = text.trim_start(); Some((start + (text.len() - trimmed.len()), trimmed)) }) - .filter(|(_, text)| !text.is_empty()); + .filter(|(_, text)| !(text.is_empty() || (accept_comment_above_attributes && is_attribute(text)))); + + let Some((line_start, line)) = lines.next() else { + return SafetyComment::Absent; + }; - let (line_start, line) = lines.next()?; let mut in_codeblock = false; // Check for a sequence of line comments. if line.starts_with("//") { @@ -771,12 +808,17 @@ fn text_has_safety_comment(src: &str, line_starts: &[RelativeBytePos], start_pos in_codeblock = !in_codeblock; } - if line.to_ascii_uppercase().contains("SAFETY:") && !in_codeblock { - return Some(start_pos + BytePos(u32::try_from(line_start).unwrap())); + if !in_codeblock && let Some(safety_pos) = line.to_ascii_uppercase().find("SAFETY:") { + return SafetyComment::Present { + pos: start_pos + + BytePos(u32::try_from(line_start).unwrap()) + + BytePos(u32::try_from(safety_pos).unwrap()), + is_doc: line.starts_with("///"), + }; } match lines.next() { Some((s, x)) if x.starts_with("//") => (line, line_start) = (x, s), - _ => return None, + _ => return SafetyComment::Absent, } } } @@ -787,19 +829,30 @@ fn text_has_safety_comment(src: &str, line_starts: &[RelativeBytePos], start_pos if line.starts_with("/*") { let src = &src[line_start..line_starts.last().unwrap().to_usize()]; let mut tokens = tokenize(src, FrontmatterAllowed::No); - return (src[..tokens.next().unwrap().len as usize] - .to_ascii_uppercase() - .contains("SAFETY:") - && tokens.all(|t| t.kind == TokenKind::Whitespace)) - .then_some(start_pos + BytePos(u32::try_from(line_start).unwrap())); + let a = tokens.next(); + if let Some(safety_pos) = src[..a.unwrap().len as usize].to_ascii_uppercase().find("SAFETY:") + && tokens.all(|t| t.kind == TokenKind::Whitespace) + { + return SafetyComment::Present { + pos: start_pos + + BytePos(u32::try_from(line_start).unwrap()) + + BytePos(u32::try_from(safety_pos).unwrap()), + is_doc: line.starts_with("/**"), + }; + } + return SafetyComment::Absent; } match lines.next() { Some(x) => (line_start, line) = x, - None => return None, + None => return SafetyComment::Absent, } } } +fn is_attribute(text: &str) -> bool { + (text.starts_with("#[") || text.starts_with("#![")) && text.trim_end().ends_with(']') +} + 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())), diff --git a/tests/ui-cargo/undocumented_unsafe_blocks/fail/Cargo.stderr b/tests/ui-cargo/undocumented_unsafe_blocks/fail/Cargo.stderr index 59a7146ac90f..bfe2486c8502 100644 --- a/tests/ui-cargo/undocumented_unsafe_blocks/fail/Cargo.stderr +++ b/tests/ui-cargo/undocumented_unsafe_blocks/fail/Cargo.stderr @@ -5,10 +5,10 @@ error: module has unnecessary safety comment | ^^^^^^^^ | help: consider removing the safety comment - --> src/main.rs:1:1 + --> src/main.rs:1:4 | 1 | // SAFETY: ... - | ^^^^^^^^^^^^^^ + | ^^^^^^^ = note: requested on the command line with `-D clippy::unnecessary-safety-comment` error: module has unnecessary safety comment @@ -18,9 +18,9 @@ error: module has unnecessary safety comment | ^^^^^^^^ | help: consider removing the safety comment - --> src/main.rs:4:1 + --> src/main.rs:4:4 | 4 | // SAFETY: ... - | ^^^^^^^^^^^^^^ + | ^^^^^^^ error: could not compile `undocumented_unsafe_blocks` (bin "undocumented_unsafe_blocks") due to 2 previous errors 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 bfc14be5421f..0d84ce87028e 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 @@ -247,10 +247,10 @@ LL | const BIG_NUMBER: i32 = 1000000; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: consider removing the safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:507:5 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:507:8 | LL | // SAFETY: - | ^^^^^^^^^^ + | ^^^^^^^ = note: `-D clippy::unnecessary-safety-comment` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::unnecessary_safety_comment)]` @@ -289,10 +289,10 @@ LL | | }; | |______^ | help: consider removing the safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:542:5 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:542:8 | LL | // SAFETY: this is more than one level away, so it should warn - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe block missing a safety comment --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:545:12 @@ -342,17 +342,137 @@ LL | const NO_SAFETY_IN_IMPL: i32 = unsafe { 1 }; | = help: consider adding a safety comment on the preceding line +error: constant has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:701:5 + | +LL | const UNIX_EPOCH_JULIAN_DAY: i32 = + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider removing the safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:699:8 + | +LL | // SAFETY: fail ONLY if `accept-comment-above-attribute = false` + | ^^^^^^^ + error: statement has unnecessary safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:719:5 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:721:5 | LL | _ = bar(); | ^^^^^^^^^^ | help: consider removing the safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:718:5 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:720:8 | LL | // SAFETY: unnecessary_safety_comment triggers here - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: module has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:741:5 + | +LL | mod x {} + | ^^^^^^^^ + | +help: consider removing the safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:740:8 + | +LL | // SAFETY: ... + | ^^^^^^^ + +error: module has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:746:5 + | +LL | mod y {} + | ^^^^^^^^ + | +help: consider removing the safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:744:8 + | +LL | // SAFETY: ... + | ^^^^^^^ + +error: module has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:751:5 + | +LL | mod z {} + | ^^^^^^^^ + | +help: consider removing the safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:750:8 + | +LL | // SAFETY: ... + | ^^^^^^^ + +error: module has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:759:5 + | +LL | mod y {} + | ^^^^^^^^ + | +help: consider removing the safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:757:8 + | +LL | // SAFETY: ... + | ^^^^^^^ + +error: statement has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:774:9 + | +LL | let x = 34; + | ^^^^^^^^^^^ + | +help: consider removing the safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:772:12 + | +LL | // SAFETY: ... + | ^^^^^^^^^^^ + +error: function has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:781:5 + | +LL | unsafe fn unsafe_comment() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider changing the `SAFETY` comment for a `# Safety` doc comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:780:8 + | +LL | // SAFETY: Bla + | ^^^^^^^ + +error: function has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:787:5 + | +LL | unsafe fn unsafe_block_comment() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider changing the `SAFETY` comment for a `# Safety` doc comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:785:8 + | +LL | SAFETY: Bla + | ^^^^^^^ + +error: function has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:791:5 + | +LL | fn safe_comment() {} + | ^^^^^^^^^^^^^^^^^^^^ + | +help: consider removing the safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:790:8 + | +LL | // SAFETY: Bla + | ^^^^^^^ + +error: function has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:795:5 + | +LL | fn safe_doc_comment() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider removing the safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:794:9 + | +LL | /// SAFETY: Bla + | ^^^^^^^ -error: aborting due to 40 previous errors +error: aborting due to 50 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 cebfc48a884f..91ed10625cc6 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 @@ -247,10 +247,10 @@ LL | const BIG_NUMBER: i32 = 1000000; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: consider removing the safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:507:5 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:507:8 | LL | // SAFETY: - | ^^^^^^^^^^ + | ^^^^^^^ = note: `-D clippy::unnecessary-safety-comment` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::unnecessary_safety_comment)]` @@ -297,10 +297,10 @@ LL | | }; | |______^ | help: consider removing the safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:542:5 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:542:8 | LL | // SAFETY: this is more than one level away, so it should warn - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe block missing a safety comment --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:545:12 @@ -439,24 +439,104 @@ LL | unsafe { Date::__from_ordinal_date_unchecked(1970, 1) }.into_julian = help: consider adding a safety comment on the preceding line error: statement has unnecessary safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:719:5 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:721:5 | LL | _ = bar(); | ^^^^^^^^^^ | help: consider removing the safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:718:5 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:720:8 | LL | // SAFETY: unnecessary_safety_comment triggers here - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsafe block missing a safety comment - --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:733:12 + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:735:12 | LL | return unsafe { h() }; | ^^^^^^^^^^^^^^ | = help: consider adding a safety comment on the preceding line -error: aborting due to 53 previous errors +error: module has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:741:5 + | +LL | mod x {} + | ^^^^^^^^ + | +help: consider removing the safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:740:8 + | +LL | // SAFETY: ... + | ^^^^^^^ + +error: module has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:751:5 + | +LL | mod z {} + | ^^^^^^^^ + | +help: consider removing the safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:750:8 + | +LL | // SAFETY: ... + | ^^^^^^^ + +error: unsafe block missing a safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:766:9 + | +LL | unsafe {} + | ^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: function has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:781:5 + | +LL | unsafe fn unsafe_comment() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider changing the `SAFETY` comment for a `# Safety` doc comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:780:8 + | +LL | // SAFETY: Bla + | ^^^^^^^ + +error: function has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:787:5 + | +LL | unsafe fn unsafe_block_comment() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider changing the `SAFETY` comment for a `# Safety` doc comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:785:8 + | +LL | SAFETY: Bla + | ^^^^^^^ + +error: function has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:791:5 + | +LL | fn safe_comment() {} + | ^^^^^^^^^^^^^^^^^^^^ + | +help: consider removing the safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:790:8 + | +LL | // SAFETY: Bla + | ^^^^^^^ + +error: function has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:795:5 + | +LL | fn safe_doc_comment() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider removing the safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs:794:9 + | +LL | /// SAFETY: Bla + | ^^^^^^^ + +error: aborting due to 60 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 a2d7c1b6c796..db9e81cf10a1 100644 --- a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks.rs @@ -701,6 +701,8 @@ mod issue_11709_regression { 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 + // This shouldn't be linted, Issue #15755 + //~[default]^^^^ unnecessary_safety_comment } fn issue_13039() { @@ -734,4 +736,64 @@ fn rfl_issue15034() -> i32 { //~[disabled]^ ERROR: unsafe block missing a safety comment } +mod issue_14555 { + // SAFETY: ... + mod x {} + //~^ unnecessary_safety_comment + + // SAFETY: ... + #[doc(hidden)] + mod y {} + //~[default]^ unnecessary_safety_comment + + #[doc(hidden)] + // SAFETY: ... + mod z {} + //~^ unnecessary_safety_comment +} + +mod issue_15754 { + #[must_use] + // SAFETY: ... + #[doc(hidden)] + mod y {} + //~[default]^ unnecessary_safety_comment + + fn foo() { + #[doc(hidden)] + // SAFETY: unnecessary_safety_comment should not trigger here + #[allow(unsafe_code)] + unsafe {} + //~[disabled]^ undocumented_unsafe_blocks + } + + fn bar() { + #[doc(hidden)] + // SAFETY: ... + #[allow(clippy::unnecessary_cast)] + let x = 34; + //~[default]^ unnecessary_safety_comment + } +} + +mod unsafe_fns { + // SAFETY: Bla + unsafe fn unsafe_comment() {} + //~^ unnecessary_safety_comment + + /* + SAFETY: Bla + */ + unsafe fn unsafe_block_comment() {} + //~^ unnecessary_safety_comment + + // SAFETY: Bla + fn safe_comment() {} + //~^ unnecessary_safety_comment + + /// SAFETY: Bla + fn safe_doc_comment() {} + //~^ unnecessary_safety_comment +} + fn main() {} diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.default.fixed b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.default.fixed new file mode 100644 index 000000000000..cc8d5028b727 --- /dev/null +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.default.fixed @@ -0,0 +1,20 @@ +//@aux-build:../../ui/auxiliary/proc_macro_unsafe.rs +//@revisions: default disabled +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/undocumented_unsafe_blocks/default +//@[disabled] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/undocumented_unsafe_blocks/disabled + +#![warn(clippy::undocumented_unsafe_blocks, clippy::unnecessary_safety_comment)] + +mod unsafe_fns { + /// # Safety Bla + unsafe fn unsafe_doc_comment() {} + //~^ unnecessary_safety_comment + + /** + * # Safety Bla + */ + unsafe fn unsafe_block_doc_comment() {} + //~^ unnecessary_safety_comment +} + +fn main() {} diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.default.stderr b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.default.stderr new file mode 100644 index 000000000000..95e47dc7ed63 --- /dev/null +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.default.stderr @@ -0,0 +1,22 @@ +error: function has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.rs:10:5 + | +LL | /// SAFETY: Bla + | ------- help: consider changing it to a `# Safety` section: `# Safety` +LL | unsafe fn unsafe_doc_comment() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::unnecessary-safety-comment` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unnecessary_safety_comment)]` + +error: function has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.rs:16:5 + | +LL | * SAFETY: Bla + | ------- help: consider changing it to a `# Safety` section: `# Safety` +LL | */ +LL | unsafe fn unsafe_block_doc_comment() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.disabled.fixed b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.disabled.fixed new file mode 100644 index 000000000000..cc8d5028b727 --- /dev/null +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.disabled.fixed @@ -0,0 +1,20 @@ +//@aux-build:../../ui/auxiliary/proc_macro_unsafe.rs +//@revisions: default disabled +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/undocumented_unsafe_blocks/default +//@[disabled] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/undocumented_unsafe_blocks/disabled + +#![warn(clippy::undocumented_unsafe_blocks, clippy::unnecessary_safety_comment)] + +mod unsafe_fns { + /// # Safety Bla + unsafe fn unsafe_doc_comment() {} + //~^ unnecessary_safety_comment + + /** + * # Safety Bla + */ + unsafe fn unsafe_block_doc_comment() {} + //~^ unnecessary_safety_comment +} + +fn main() {} diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.disabled.stderr b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.disabled.stderr new file mode 100644 index 000000000000..95e47dc7ed63 --- /dev/null +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.disabled.stderr @@ -0,0 +1,22 @@ +error: function has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.rs:10:5 + | +LL | /// SAFETY: Bla + | ------- help: consider changing it to a `# Safety` section: `# Safety` +LL | unsafe fn unsafe_doc_comment() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::unnecessary-safety-comment` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unnecessary_safety_comment)]` + +error: function has unnecessary safety comment + --> tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.rs:16:5 + | +LL | * SAFETY: Bla + | ------- help: consider changing it to a `# Safety` section: `# Safety` +LL | */ +LL | unsafe fn unsafe_block_doc_comment() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.rs b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.rs new file mode 100644 index 000000000000..14b91126caa6 --- /dev/null +++ b/tests/ui-toml/undocumented_unsafe_blocks/undocumented_unsafe_blocks_fixable.rs @@ -0,0 +1,20 @@ +//@aux-build:../../ui/auxiliary/proc_macro_unsafe.rs +//@revisions: default disabled +//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/undocumented_unsafe_blocks/default +//@[disabled] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/undocumented_unsafe_blocks/disabled + +#![warn(clippy::undocumented_unsafe_blocks, clippy::unnecessary_safety_comment)] + +mod unsafe_fns { + /// SAFETY: Bla + unsafe fn unsafe_doc_comment() {} + //~^ unnecessary_safety_comment + + /** + * SAFETY: Bla + */ + unsafe fn unsafe_block_doc_comment() {} + //~^ unnecessary_safety_comment +} + +fn main() {} diff --git a/tests/ui/crashes/ice-15684.rs b/tests/ui/crashes/ice-15684.rs new file mode 100644 index 000000000000..12f36042a0fe --- /dev/null +++ b/tests/ui/crashes/ice-15684.rs @@ -0,0 +1,10 @@ +#![warn(clippy::unnecessary_safety_comment)] + +fn foo() -> i32 { + // SAFETY: fail ONLY if `accept-comment-above-attribute = false` + #[must_use] + return 33; + //~^ unnecessary_safety_comment +} + +fn main() {} diff --git a/tests/ui/crashes/ice-15684.stderr b/tests/ui/crashes/ice-15684.stderr new file mode 100644 index 000000000000..0d4eb624a2b1 --- /dev/null +++ b/tests/ui/crashes/ice-15684.stderr @@ -0,0 +1,16 @@ +error: statement has unnecessary safety comment + --> tests/ui/crashes/ice-15684.rs:6:5 + | +LL | return 33; + | ^^^^^^^^^^ + | +help: consider removing the safety comment + --> tests/ui/crashes/ice-15684.rs:4:8 + | +LL | // SAFETY: fail ONLY if `accept-comment-above-attribute = false` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `-D clippy::unnecessary-safety-comment` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unnecessary_safety_comment)]` + +error: aborting due to 1 previous error + diff --git a/tests/ui/unnecessary_safety_comment.stderr b/tests/ui/unnecessary_safety_comment.stderr index 732e6767c178..6ad94f986434 100644 --- a/tests/ui/unnecessary_safety_comment.stderr +++ b/tests/ui/unnecessary_safety_comment.stderr @@ -5,10 +5,10 @@ LL | const CONST: u32 = 0; | ^^^^^^^^^^^^^^^^^^^^^ | help: consider removing the safety comment - --> tests/ui/unnecessary_safety_comment.rs:5:5 + --> tests/ui/unnecessary_safety_comment.rs:5:8 | LL | // SAFETY: - | ^^^^^^^^^^ + | ^^^^^^^ = note: `-D clippy::unnecessary-safety-comment` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::unnecessary_safety_comment)]` @@ -19,10 +19,10 @@ LL | static STATIC: u32 = 0; | ^^^^^^^^^^^^^^^^^^^^^^^ | help: consider removing the safety comment - --> tests/ui/unnecessary_safety_comment.rs:9:5 + --> tests/ui/unnecessary_safety_comment.rs:9:8 | LL | // SAFETY: - | ^^^^^^^^^^ + | ^^^^^^^ error: struct has unnecessary safety comment --> tests/ui/unnecessary_safety_comment.rs:14:5 @@ -31,10 +31,10 @@ LL | struct Struct; | ^^^^^^^^^^^^^^ | help: consider removing the safety comment - --> tests/ui/unnecessary_safety_comment.rs:13:5 + --> tests/ui/unnecessary_safety_comment.rs:13:8 | LL | // SAFETY: - | ^^^^^^^^^^ + | ^^^^^^^ error: enum has unnecessary safety comment --> tests/ui/unnecessary_safety_comment.rs:18:5 @@ -43,10 +43,10 @@ LL | enum Enum {} | ^^^^^^^^^^^^ | help: consider removing the safety comment - --> tests/ui/unnecessary_safety_comment.rs:17:5 + --> tests/ui/unnecessary_safety_comment.rs:17:8 | LL | // SAFETY: - | ^^^^^^^^^^ + | ^^^^^^^ error: module has unnecessary safety comment --> tests/ui/unnecessary_safety_comment.rs:22:5 @@ -55,10 +55,10 @@ LL | mod module {} | ^^^^^^^^^^^^^ | help: consider removing the safety comment - --> tests/ui/unnecessary_safety_comment.rs:21:5 + --> tests/ui/unnecessary_safety_comment.rs:21:8 | LL | // SAFETY: - | ^^^^^^^^^^ + | ^^^^^^^ error: impl has unnecessary safety comment --> tests/ui/unnecessary_safety_comment.rs:42:13 @@ -70,10 +70,10 @@ LL | with_safety_comment!(i32); | ------------------------- in this macro invocation | help: consider removing the safety comment - --> tests/ui/unnecessary_safety_comment.rs:41:13 + --> tests/ui/unnecessary_safety_comment.rs:41:16 | 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 @@ -83,10 +83,10 @@ LL | 24 | ^^ | help: consider removing the safety comment - --> tests/ui/unnecessary_safety_comment.rs:59:5 + --> tests/ui/unnecessary_safety_comment.rs:59:8 | LL | // SAFETY: unnecessary - | ^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^ error: statement has unnecessary safety comment --> tests/ui/unnecessary_safety_comment.rs:52:5 @@ -95,10 +95,10 @@ LL | let num = 42; | ^^^^^^^^^^^^^ | help: consider removing the safety comment - --> tests/ui/unnecessary_safety_comment.rs:51:5 + --> tests/ui/unnecessary_safety_comment.rs:51:8 | LL | // SAFETY: unnecessary - | ^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^ error: statement has unnecessary safety comment --> tests/ui/unnecessary_safety_comment.rs:56:5 @@ -107,10 +107,10 @@ LL | if num > 24 {} | ^^^^^^^^^^^^^^ | help: consider removing the safety comment - --> tests/ui/unnecessary_safety_comment.rs:55:5 + --> tests/ui/unnecessary_safety_comment.rs:55:8 | LL | // SAFETY: unnecessary - | ^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^ error: aborting due to 9 previous errors