From 2c5b3f4c09807f977f3bd0139899444d158cb5c4 Mon Sep 17 00:00:00 2001 From: Robert Vicari Date: Fri, 20 Dec 2024 18:28:40 +0100 Subject: [PATCH] new lint proper_safety_comment --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/safety/mod.rs | 110 ++++++ .../src/safety/proper_safety_comment.rs | 346 ++++++++++++++++++ .../safety/proper_safety_comment/attribute.rs | 32 ++ .../proper_safety_comment/attribute.stderr | 30 ++ .../ui/safety/proper_safety_comment/block.rs | 64 ++++ .../safety/proper_safety_comment/block.stderr | 64 ++++ .../proper_safety_comment/proc_macros.rs | 84 +++++ .../proper_safety_comment/trait_impl.rs | 166 +++++++++ .../proper_safety_comment/trait_impl.stderr | 117 ++++++ .../proper_safety_comment/unsafe_extern.rs | 68 ++++ .../unsafe_extern.stderr | 49 +++ 14 files changed, 1134 insertions(+) create mode 100644 clippy_lints/src/safety/mod.rs create mode 100644 clippy_lints/src/safety/proper_safety_comment.rs create mode 100644 tests/ui/safety/proper_safety_comment/attribute.rs create mode 100644 tests/ui/safety/proper_safety_comment/attribute.stderr create mode 100644 tests/ui/safety/proper_safety_comment/block.rs create mode 100644 tests/ui/safety/proper_safety_comment/block.stderr create mode 100644 tests/ui/safety/proper_safety_comment/proc_macros.rs create mode 100644 tests/ui/safety/proper_safety_comment/trait_impl.rs create mode 100644 tests/ui/safety/proper_safety_comment/trait_impl.stderr create mode 100644 tests/ui/safety/proper_safety_comment/unsafe_extern.rs create mode 100644 tests/ui/safety/proper_safety_comment/unsafe_extern.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index b6033de93501..4277a855c7a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5914,6 +5914,7 @@ Released 2018-09-13 [`print_stdout`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_stdout [`print_with_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_with_newline [`println_empty_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#println_empty_string +[`proper_safety_comment`]: https://rust-lang.github.io/rust-clippy/master/index.html#proper_safety_comment [`ptr_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg [`ptr_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr [`ptr_cast_constness`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_cast_constness diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 7451fb909ef8..eeecb0ffda79 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -660,6 +660,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::returns::LET_AND_RETURN_INFO, crate::returns::NEEDLESS_RETURN_INFO, crate::returns::NEEDLESS_RETURN_WITH_QUESTION_MARK_INFO, + crate::safety::PROPER_SAFETY_COMMENT_INFO, crate::same_name_method::SAME_NAME_METHOD_INFO, crate::self_named_constructors::SELF_NAMED_CONSTRUCTORS_INFO, crate::semicolon_block::SEMICOLON_INSIDE_BLOCK_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d33013ba6638..97396d857fa8 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -326,6 +326,7 @@ mod repeat_vec_with_capacity; mod reserve_after_initialization; mod return_self_not_must_use; mod returns; +mod safety; mod same_name_method; mod self_named_constructors; mod semicolon_block; @@ -967,4 +968,5 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound)); store.register_late_pass(move |_| Box::new(arbitrary_source_item_ordering::ArbitrarySourceItemOrdering::new(conf))); // add lints here, do not remove this comment, it's used in `new_lint` + store.register_early_pass(|| Box::new(safety::Safety)); } diff --git a/clippy_lints/src/safety/mod.rs b/clippy_lints/src/safety/mod.rs new file mode 100644 index 000000000000..46a44cebb7e6 --- /dev/null +++ b/clippy_lints/src/safety/mod.rs @@ -0,0 +1,110 @@ +mod proper_safety_comment; + +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_session::impl_lint_pass; + +declare_clippy_lint! { + /// ### What it does + /// + /// It requires proper safety comments at the barrier of [Unsafety](https://doc.rust-lang.org/reference/unsafety.html). + /// This includes any part of the [code that needs to satisfy extra safety conditions](https://doc.rust-lang.org/reference/unsafe-keyword.html): + /// + /// - unsafe blocks (`unsafe {}`) + /// - unsafe trait implementations (`unsafe impl`) + /// - unsafe external blocks (`unsafe extern`) + /// - unsafe attributes (`#[unsafe(attr)]`) + /// + /// Safety comments are [non-doc line comments](https://doc.rust-lang.org/reference/comments.html) starting with `SAFETY:`: + /// + /// ```no_run + /// // SAFETY: A safety comment + /// // that can cover + /// // multiple lines. + /// ``` + /// + /// Furthermore, it detects unnecessary safety comments for non-critical blocks, trait implementations and attributes. However, there can be false negatives. + /// + /// [Code that defines extra safety conditions](https://doc.rust-lang.org/reference/unsafe-keyword.html) is covered by [`clippy::missing_safety_doc`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc) and [`clippy::unnecessary_safety_doc`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_doc) + /// + /// ### Why restrict this? + /// + /// Breaking the safety barrier should not be done carelessly. + /// Proper documentation should be provided as to why each unsafe operation does not introduce [undefined behavior](https://doc.rust-lang.org/reference/behavior-considered-undefined.html). + /// Thinking about these safety requirements and writing them down can prevent incorrect implementations. + /// On the other hand, unnecessary safety comments are confusing and should not exist. + /// + /// ### Example + /// + /// ```no_run + /// unsafe fn f1() {} + /// fn f2() { + /// unsafe { f1() } + /// } + /// + /// unsafe trait A {} + /// unsafe impl A for (); + /// + /// unsafe extern { + /// pub fn g1(); + /// pub unsafe fn g2(); + /// pub safe fn g3(); + /// } + /// + /// #[unsafe(no_mangle)] + /// fn h(); + /// ``` + /// + /// Use instead: + /// + /// ```no_run + /// unsafe fn f1() {} + /// fn f2() { + /// unsafe { + /// // SAFETY: ... + /// f1() + /// } + /// } + /// + /// unsafe trait A {} + /// // SAFETY: ... + /// unsafe impl A for (); + /// + /// // SAFETY: ... + /// unsafe extern { + /// // SAFETY: ... + /// pub fn g1(); + /// // SAFETY: ... + /// pub unsafe fn g2(); + /// // SAFETY: ... + /// pub safe fn g3(); + /// } + /// + /// // SAFETY: ... + /// #[unsafe(no_mangle)] + /// fn h(); + /// ``` + #[clippy::version = "1.85.0"] + pub PROPER_SAFETY_COMMENT, + restriction, + "requires proper safety comments at the barrier of unsafety" +} + +pub struct Safety; + +impl_lint_pass!(Safety => [ + PROPER_SAFETY_COMMENT, +]); + +impl EarlyLintPass for Safety { + fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &rustc_ast::Attribute) { + proper_safety_comment::check_attribute(cx, attr); + } + + fn check_block(&mut self, cx: &EarlyContext<'_>, block: &rustc_ast::Block) { + proper_safety_comment::check_block(cx, block); + } + + fn check_item(&mut self, cx: &EarlyContext<'_>, item: &rustc_ast::Item) { + proper_safety_comment::check_item(cx, item); + } +} diff --git a/clippy_lints/src/safety/proper_safety_comment.rs b/clippy_lints/src/safety/proper_safety_comment.rs new file mode 100644 index 000000000000..4cd2f8171caa --- /dev/null +++ b/clippy_lints/src/safety/proper_safety_comment.rs @@ -0,0 +1,346 @@ +use rustc_ast::{ + AttrKind, AttrStyle, Attribute, Block, BlockCheckMode, ForeignMod, Impl, Item, ItemKind, Safety, UnsafeSource, +}; +use rustc_lint::{EarlyContext, LintContext}; +use rustc_middle::lint; +use rustc_span::{BytePos, SourceFileAndLine, Span}; + +use super::PROPER_SAFETY_COMMENT; + +/// All safety comments have the format `// SAFETY_COMMENT_LABEL **comment**` +const SAFETY_COMMENT_LABEL: &str = "SAFETY:"; + +pub(super) fn check_attribute(cx: &EarlyContext<'_>, attr: &Attribute) { + if lint::in_external_macro(cx.sess(), attr.span) { + return; + } + + let is_critical = match &attr.kind { + AttrKind::Normal(p) => match p.item.unsafety { + Safety::Unsafe(_) => true, + Safety::Safe(_) | Safety::Default => false, + }, + AttrKind::DocComment(_, _) => false, + }; + + if is_critical { + // check for procedural macro + let expected_tokens = match attr.style { + AttrStyle::Outer => &["#", "", "[", "unsafe", "("], + AttrStyle::Inner => &["#", "!", "[", "unsafe", "("], + }; + if !span_starts_with(cx, attr.span, expected_tokens) { + return; + } + + if span_has_safety_comment(cx, attr.span).is_none() { + clippy_utils::diagnostics::span_lint( + cx, + PROPER_SAFETY_COMMENT, + attr.span, + "missing safety comment on critical attribute", + ); + } + } else { + // TODO what about `#[derive(..)]`? + // // check for procedural macro + // if !span_starts_with(cx, attr.span, &["#", "["]) { + // return; + // } + + if let Some(span) = span_has_safety_comment(cx, attr.span) { + clippy_utils::diagnostics::span_lint( + cx, + PROPER_SAFETY_COMMENT, + span, + "unnecessary safety comment on attribute", + ); + } + } +} + +pub(super) fn check_block(cx: &EarlyContext<'_>, block: &Block) { + if lint::in_external_macro(cx.sess(), block.span) { + return; + } + + let (is_unsafe, is_critical) = match block.rules { + BlockCheckMode::Default => (false, false), + BlockCheckMode::Unsafe(UnsafeSource::UserProvided) => (true, true), + BlockCheckMode::Unsafe(UnsafeSource::CompilerGenerated) => (true, false), + }; + + match block_contains_safety_comment(cx, block.span, is_unsafe) { + Ok(Some(span)) => { + if !(is_critical || is_unsafe) { + clippy_utils::diagnostics::span_lint( + cx, + PROPER_SAFETY_COMMENT, + span, + "unnecessary safety comment inside block", + ); + } + }, + Ok(None) => { + if is_critical && is_unsafe { + clippy_utils::diagnostics::span_lint( + cx, + PROPER_SAFETY_COMMENT, + block.span, + "missing safety comment inside unsafe block", + ); + } + }, + Err(()) => {}, + } +} + +pub(super) fn check_item(cx: &EarlyContext<'_>, item: &Item) { + if lint::in_external_macro(cx.sess(), item.span) { + return; + } + + match &item.kind { + ItemKind::ForeignMod(foreign_mod) => check_foreign_mod(cx, foreign_mod, item.span), + ItemKind::Impl(impl_block) => check_impl(cx, impl_block, item.span), + _ => (), + } +} + +fn check_foreign_mod(cx: &EarlyContext<'_>, foreign_mod: &ForeignMod, span: Span) { + // check for procedural macro + if !span_starts_with(cx, span, &["unsafe", "extern"]) { + //TODO remove this `if`-statement once `unsafe extern` is mandatory + if !span_starts_with(cx, span, &["extern"]) { + return; + } + } + + if span_has_safety_comment(cx, span).is_none() { + clippy_utils::diagnostics::span_lint( + cx, + PROPER_SAFETY_COMMENT, + span, + "missing safety comment on `unsafe extern`-block", + ); + } + + for foreign_mod_item in &foreign_mod.items { + if span_has_safety_comment(cx, foreign_mod_item.span).is_none() { + clippy_utils::diagnostics::span_lint( + cx, + PROPER_SAFETY_COMMENT, + foreign_mod_item.span, + "missing safety comment on item in `unsafe extern`-block", + ); + } + } +} + +fn check_impl(cx: &EarlyContext<'_>, impl_block: &Impl, span: Span) { + match impl_block.safety { + Safety::Unsafe(_) => { + // check for procedural macro + if !span_starts_with(cx, span, &["unsafe", "impl"]) { + return; + } + + if span_has_safety_comment(cx, span).is_none() { + clippy_utils::diagnostics::span_lint( + cx, + PROPER_SAFETY_COMMENT, + span, + "missing safety comment on unsafe impl", + ); + } + }, + Safety::Safe(_) => {}, + Safety::Default => { + // check for procedural macro + if !span_starts_with(cx, span, &["impl"]) { + return; + } + + if let Some(span) = span_has_safety_comment(cx, span) { + clippy_utils::diagnostics::span_lint( + cx, + PROPER_SAFETY_COMMENT, + span, + "unnecessary safety comment on impl", + ); + } + }, + } +} + +fn block_contains_safety_comment(cx: &impl LintContext, span: Span, is_unsafe: bool) -> Result, ()> { + let source_map = cx.sess().source_map(); + + let snippet = source_map.span_to_snippet(span).map_err(|_| ())?; + + let trimmed_snippet = snippet + .trim_start() + .strip_prefix(if is_unsafe { "unsafe" } else { "" }) + .ok_or(())? + .trim_start() + .strip_prefix("{") + .ok_or(())? + .trim_start(); + + if identify_text_type(trimmed_snippet) != TextType::SafetyComment { + return Ok(None); + } + + let safety_comment_start = + span.lo() + BytePos(u32::try_from(snippet.len() - trimmed_snippet.len()).map_err(|_| ())?); + + let SourceFileAndLine { + sf: safety_comment_source_file, + line: safety_comment_start_line, + } = source_map.lookup_line(safety_comment_start).map_err(|_| ())?; + + let mut safety_comment_end_line = safety_comment_start_line; + while let TextType::Comment | TextType::Empty = identify_text_type( + &safety_comment_source_file + .get_line(safety_comment_end_line + 1) + .ok_or(())?, + ) { + safety_comment_end_line += 1; + } + let safety_comment_end_line = safety_comment_end_line; + + let safety_comment = span + .with_lo(safety_comment_start) + .with_hi(safety_comment_source_file.line_bounds(safety_comment_end_line).end); + + Ok(Some(safety_comment)) +} + +#[must_use] +fn span_has_safety_comment(cx: &impl LintContext, span: Span) -> Option { + let source_map = cx.sess().source_map(); + + let SourceFileAndLine { + sf: unsafe_source_file, + line: unsafe_line_number, + } = source_map.lookup_line(span.lo()).ok()?; + + for line_number in (0..unsafe_line_number).rev() { + match identify_text_type(&unsafe_source_file.get_line(line_number)?) { + TextType::SafetyComment => { + let safety_comment = span + .with_lo(unsafe_source_file.line_bounds(line_number).start) + .with_hi(unsafe_source_file.line_bounds(unsafe_line_number - 1).end); + + return Some(safety_comment); + }, + TextType::Comment | TextType::Empty => continue, + TextType::NoComment | TextType::DocComment => break, + } + } + + None +} + +#[must_use] +fn span_starts_with(cx: &impl LintContext, span: Span, tokens: &[&str]) -> bool { + cx.sess() + .source_map() + .span_to_source(span, |src, start, end| { + if let Some(snippet) = src.get(start..end) { + let mut remaining_snippet = snippet; + for &token in tokens { + if let Some(s) = remaining_snippet.strip_prefix(token) { + remaining_snippet = s.trim_start(); + } else { + return Ok(false); + } + } + Ok(true) + } else { + Ok(false) + } + }) + .unwrap_or(false) +} + +#[derive(Debug, PartialEq)] +enum TextType { + SafetyComment, + Comment, + DocComment, + NoComment, + Empty, +} + +#[must_use] +fn identify_text_type(text: &str) -> TextType { + let text_trimmed = text.trim_start(); + + if text_trimmed.starts_with("///") { + TextType::DocComment + } else if text_trimmed.starts_with("//") { + let comment = text_trimmed.strip_prefix("//").unwrap().trim_start(); + + if comment.starts_with(SAFETY_COMMENT_LABEL) { + TextType::SafetyComment + } else { + TextType::Comment + } + } else if !text_trimmed.is_empty() { + TextType::NoComment + } else { + TextType::Empty + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_identify_text_type() { + assert_eq!( + identify_text_type(&format!("//{SAFETY_COMMENT_LABEL}")), + TextType::SafetyComment + ); + assert_eq!( + identify_text_type(&format!(" // {SAFETY_COMMENT_LABEL} ")), + TextType::SafetyComment + ); + assert_eq!( + identify_text_type(&format!(" // {SAFETY_COMMENT_LABEL} ")), + TextType::SafetyComment + ); + assert_eq!(identify_text_type("//"), TextType::Comment); + assert_eq!(identify_text_type(" // "), TextType::Comment); + assert_eq!(identify_text_type(" // "), TextType::Comment); + assert_eq!( + identify_text_type(&format!("///{SAFETY_COMMENT_LABEL}")), + TextType::DocComment + ); + assert_eq!( + identify_text_type(&format!(" /// {SAFETY_COMMENT_LABEL} ")), + TextType::DocComment + ); + assert_eq!( + identify_text_type(&format!(" /// {SAFETY_COMMENT_LABEL} ")), + TextType::DocComment + ); + assert_eq!( + identify_text_type(&format!("/{SAFETY_COMMENT_LABEL}")), + TextType::NoComment + ); + assert_eq!( + identify_text_type(&format!(" / {SAFETY_COMMENT_LABEL} ")), + TextType::NoComment + ); + assert_eq!( + identify_text_type(&format!(" / {SAFETY_COMMENT_LABEL} ")), + TextType::NoComment + ); + assert_eq!(identify_text_type(SAFETY_COMMENT_LABEL), TextType::NoComment); + assert_eq!(identify_text_type(""), TextType::Empty); + assert_eq!(identify_text_type(" \n \n\n "), TextType::Empty); + } +} diff --git a/tests/ui/safety/proper_safety_comment/attribute.rs b/tests/ui/safety/proper_safety_comment/attribute.rs new file mode 100644 index 000000000000..4b56deca74b1 --- /dev/null +++ b/tests/ui/safety/proper_safety_comment/attribute.rs @@ -0,0 +1,32 @@ +#![warn(clippy::proper_safety_comment)] + +//~v ERROR: missing safety comment on critical attribute +#[unsafe(no_mangle)] +pub fn f1() {} + +//~v ERROR: unnecessary safety comment on attribute +// SAFETY: ... +#[warn(clippy::proper_safety_comment)] +#[unsafe(no_mangle)] +//~^ ERROR: missing safety comment on critical attribute +pub fn f2() {} + +#[warn(clippy::proper_safety_comment)] +// SAFETY: ... +#[unsafe(no_mangle)] +pub fn f3() {} + +fn nested() { + //~v ERROR: missing safety comment on critical attribute + #[unsafe(no_mangle)] + pub fn f4() {} +} + +// SAFETY: not detected as unnecessary safety comment due to the procedural macro +#[derive(Debug)] +struct S1; + +#[derive(Debug)] +struct S2; + +fn main() {} diff --git a/tests/ui/safety/proper_safety_comment/attribute.stderr b/tests/ui/safety/proper_safety_comment/attribute.stderr new file mode 100644 index 000000000000..4a449cfc8dbf --- /dev/null +++ b/tests/ui/safety/proper_safety_comment/attribute.stderr @@ -0,0 +1,30 @@ +error: missing safety comment on critical attribute + --> tests/ui/safety/proper_safety_comment/attribute.rs:4:1 + | +LL | #[unsafe(no_mangle)] + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::proper-safety-comment` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::proper_safety_comment)]` + +error: unnecessary safety comment on attribute + --> tests/ui/safety/proper_safety_comment/attribute.rs:8:1 + | +LL | / // SAFETY: ... +LL | | #[warn(clippy::proper_safety_comment)] + | |_^ + +error: missing safety comment on critical attribute + --> tests/ui/safety/proper_safety_comment/attribute.rs:10:1 + | +LL | #[unsafe(no_mangle)] + | ^^^^^^^^^^^^^^^^^^^^ + +error: missing safety comment on critical attribute + --> tests/ui/safety/proper_safety_comment/attribute.rs:21:5 + | +LL | #[unsafe(no_mangle)] + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 4 previous errors + diff --git a/tests/ui/safety/proper_safety_comment/block.rs b/tests/ui/safety/proper_safety_comment/block.rs new file mode 100644 index 000000000000..e73a43192419 --- /dev/null +++ b/tests/ui/safety/proper_safety_comment/block.rs @@ -0,0 +1,64 @@ +#![warn(clippy::proper_safety_comment)] +#![allow(dead_code, unused_unsafe)] + +unsafe fn f1() {} + +unsafe fn f2() { + // SAFETY: + //~^ ERROR: unnecessary safety comment inside block +} + +fn main() { + unsafe { + // SAFETY: + } + + unsafe {} + //~^ ERROR: missing safety comment inside unsafe block + + // SAFETY: + unsafe {} + //~^ ERROR: missing safety comment inside unsafe block + + unsafe { // SAFETY: + } + + unsafe { + + // SAFETY: + } + + //~v ERROR: missing safety comment inside unsafe block + unsafe { + let x = false; + // SAFETY: + } + + { + let x = false; + // SAFETY: + } + + //~v ERROR: unnecessary safety comment inside block + { // SAFETY: + } + + { + // SAFETY: + //~^ ERROR: unnecessary safety comment inside block + } + + { + + // SAFETY: + //~^ ERROR: unnecessary safety comment inside block + } + + println!("{}", unsafe { + // SAFETY: + String::from_utf8_unchecked(vec![]) + }); + + println!("{}", unsafe { String::from_utf8_unchecked(vec![]) }); + //~^ ERROR: missing safety comment inside unsafe block +} diff --git a/tests/ui/safety/proper_safety_comment/block.stderr b/tests/ui/safety/proper_safety_comment/block.stderr new file mode 100644 index 000000000000..3f5879ef7c34 --- /dev/null +++ b/tests/ui/safety/proper_safety_comment/block.stderr @@ -0,0 +1,64 @@ +error: unnecessary safety comment inside block + --> tests/ui/safety/proper_safety_comment/block.rs:7:5 + | +LL | / // SAFETY: +LL | | +LL | | } + | |_^ + | + = note: `-D clippy::proper-safety-comment` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::proper_safety_comment)]` + +error: missing safety comment inside unsafe block + --> tests/ui/safety/proper_safety_comment/block.rs:16:5 + | +LL | unsafe {} + | ^^^^^^^^^ + +error: missing safety comment inside unsafe block + --> tests/ui/safety/proper_safety_comment/block.rs:20:5 + | +LL | unsafe {} + | ^^^^^^^^^ + +error: missing safety comment inside unsafe block + --> tests/ui/safety/proper_safety_comment/block.rs:32:5 + | +LL | / unsafe { +LL | | let x = false; +LL | | // SAFETY: +LL | | } + | |_____^ + +error: unnecessary safety comment inside block + --> tests/ui/safety/proper_safety_comment/block.rs:43:7 + | +LL | { // SAFETY: + | _______^ +LL | | } + | |_^ + +error: unnecessary safety comment inside block + --> tests/ui/safety/proper_safety_comment/block.rs:47:9 + | +LL | / // SAFETY: +LL | | +LL | | } + | |_^ + +error: unnecessary safety comment inside block + --> tests/ui/safety/proper_safety_comment/block.rs:53:9 + | +LL | / // SAFETY: +LL | | +LL | | } + | |_^ + +error: missing safety comment inside unsafe block + --> tests/ui/safety/proper_safety_comment/block.rs:62:20 + | +LL | println!("{}", unsafe { String::from_utf8_unchecked(vec![]) }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 8 previous errors + diff --git a/tests/ui/safety/proper_safety_comment/proc_macros.rs b/tests/ui/safety/proper_safety_comment/proc_macros.rs new file mode 100644 index 000000000000..67ac8e4575d7 --- /dev/null +++ b/tests/ui/safety/proper_safety_comment/proc_macros.rs @@ -0,0 +1,84 @@ +//@aux-build:../../auxiliary/proc_macros.rs + +#![warn(clippy::proper_safety_comment)] +#![allow(clippy::missing_safety_doc, clippy::no_effect)] + +extern crate proc_macros; + +mod proc_macro_attribute { + proc_macros::with_span!( + span + + #[unsafe(no_mangle)] + struct A; + ); + + proc_macros::with_span!( + // SAFETY: + span + + // SAFETY: + #[derive(Debug)] + struct B; + ); +} + +fn proc_macro_block() { + proc_macros::with_span!( + span + + let mut x = unsafe { 0 }; + ); + + proc_macros::with_span!( + span + + { + // SAFETY: + x += 1; + } + ); + + x += 1; +} + +mod proc_macro_extern { + proc_macros::with_span!( + span + + unsafe extern { + pub safe fn f1(); + pub unsafe fn f2(); + pub fn f3(); + } + ); + + proc_macros::with_span!( + span + + extern { + pub fn g(); + } + ); +} + +mod proc_macro_impl { + unsafe trait A {} + trait B {} + + proc_macros::with_span!( + span + + unsafe impl A for () {} + ); + + proc_macros::with_span!( + // SAFETY: + span + + // SAFETY: + impl B for () {} + ); +} + +fn main() {} diff --git a/tests/ui/safety/proper_safety_comment/trait_impl.rs b/tests/ui/safety/proper_safety_comment/trait_impl.rs new file mode 100644 index 000000000000..4ff2245ec435 --- /dev/null +++ b/tests/ui/safety/proper_safety_comment/trait_impl.rs @@ -0,0 +1,166 @@ +#![warn(clippy::proper_safety_comment)] +#![allow(clippy::missing_safety_doc, non_local_definitions)] + +unsafe trait UnsafeTrait {} + +//~v ERROR: missing safety comment on unsafe impl +unsafe impl UnsafeTrait for u8 {} + +// SAFETY: ... +unsafe impl UnsafeTrait for u16 {} + +// SAFETY: ... + +// ... + +unsafe impl UnsafeTrait for u32 {} + +//~v ERROR: missing safety comment on unsafe impl +unsafe impl UnsafeTrait for u64 {} + +fn impl_in_fn() { + //~v ERROR: missing safety comment on unsafe impl + unsafe impl UnsafeTrait for i8 {} + + // SAFETY: ... + unsafe impl UnsafeTrait for i16 {} +} + +mod unsafe_impl { + unsafe trait A {} + trait C {} + + //~v ERROR: missing safety comment on unsafe impl + unsafe impl A for () {} + + // SAFETY: + unsafe impl A for bool {} + + mod mod_1 { + unsafe trait B {} + //~v ERROR: missing safety comment on unsafe impl + unsafe impl B for () {} + } + + mod mod_2 { + unsafe trait B {} + // + // SAFETY: + // + unsafe impl B for () {} + } + + //~v ERROR: unnecessary safety comment on impl + // SAFETY: + impl C for () {} +} + +mod unsafe_impl_from_macro { + unsafe trait T {} + + macro_rules! no_safety_comment { + ($t:ty) => { + unsafe impl T for $t {} + //~^ ERROR: missing safety comment on unsafe impl + }; + } + + no_safety_comment!(()); + + macro_rules! with_safety_comment { + ($t:ty) => { + // SAFETY: + unsafe impl T for $t {} + }; + } + + with_safety_comment!(i32); +} + +mod unsafe_impl_macro_and_not_macro { + unsafe trait T {} + + macro_rules! no_safety_comment { + ($t:ty) => { + unsafe impl T for $t {} + //~^ ERROR: missing safety comment on unsafe impl + }; + } + + no_safety_comment!(()); + + //~v ERROR: missing safety comment on unsafe impl + unsafe impl T for i32 {} + + no_safety_comment!(u32); + + //~v ERROR: missing safety comment on unsafe impl + unsafe impl T for bool {} +} + +#[rustfmt::skip] +mod unsafe_impl_valid_comment { + unsafe trait SaFety {} + // SAFETY: + unsafe impl SaFety for () {} + + unsafe trait MultiLineComment {} + // The following impl is safe + // ... + // SAFETY: reason + unsafe impl MultiLineComment for () {} + + unsafe trait NoAscii {} + // SAFETY: 以下のコードは安全です + unsafe impl NoAscii for () {} + + unsafe trait InlineAndPrecedingComment {} + // SAFETY: + /* comment */ unsafe impl InlineAndPrecedingComment for () {} + + unsafe trait BuriedSafety {} + // Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor + // incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation + // ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in + // reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint + // occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est + // laborum. + // SAFETY: + // Tellus elementum sagittis vitae et leo duis ut diam quam. Sit amet nulla facilisi + // morbi tempus iaculis urna. Amet luctus venenatis lectus magna. At quis risus sed vulputate odio + // ut. Luctus venenatis lectus magna fringilla urna. Tortor id aliquet lectus proin nibh nisl + // condimentum id venenatis. Vulputate dignissim suspendisse in est ante in nibh mauris cursus. + unsafe impl BuriedSafety for () {} +} + +#[rustfmt::skip] +mod unsafe_impl_invalid_comment { + unsafe trait NoComment {} + + //~v ERROR: missing safety comment on unsafe impl + unsafe impl NoComment for () {} + + unsafe trait InlineComment {} + + //~v ERROR: missing safety comment on unsafe impl + /* SAFETY: */ unsafe impl InlineComment for () {} + + unsafe trait TrailingComment {} + + //~v ERROR: missing safety comment on unsafe impl + unsafe impl TrailingComment for () {} // SAFETY: + + unsafe trait Interference {} + // SAFETY: + const BIG_NUMBER: i32 = 1000000; + unsafe impl Interference for () {} + //~^ ERROR: missing safety comment on unsafe impl + + unsafe trait MultiLineBlockComment {} + /* This is a description + * Safety: */ + unsafe impl MultiLineBlockComment for () {} + //~^ ERROR: missing safety comment on unsafe impl +} + +fn main() {} diff --git a/tests/ui/safety/proper_safety_comment/trait_impl.stderr b/tests/ui/safety/proper_safety_comment/trait_impl.stderr new file mode 100644 index 000000000000..a2195ea24ddf --- /dev/null +++ b/tests/ui/safety/proper_safety_comment/trait_impl.stderr @@ -0,0 +1,117 @@ +error: missing safety comment on unsafe impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:7:1 + | +LL | unsafe impl UnsafeTrait for u8 {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::proper-safety-comment` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::proper_safety_comment)]` + +error: missing safety comment on unsafe impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:19:1 + | +LL | unsafe impl UnsafeTrait for u64 {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: missing safety comment on unsafe impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:23:5 + | +LL | unsafe impl UnsafeTrait for i8 {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: missing safety comment on unsafe impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:34:5 + | +LL | unsafe impl A for () {} + | ^^^^^^^^^^^^^^^^^^^^^^^ + +error: missing safety comment on unsafe impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:42:9 + | +LL | unsafe impl B for () {} + | ^^^^^^^^^^^^^^^^^^^^^^^ + +error: unnecessary safety comment on impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:54:1 + | +LL | / // SAFETY: +LL | | impl C for () {} + | |_^ + +error: missing safety comment on unsafe impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:63:13 + | +LL | unsafe impl T for $t {} + | ^^^^^^^^^^^^^^^^^^^^^^^ +... +LL | no_safety_comment!(()); + | ---------------------- in this macro invocation + | + = note: this error originates in the macro `no_safety_comment` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: missing safety comment on unsafe impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:85:13 + | +LL | unsafe impl T for $t {} + | ^^^^^^^^^^^^^^^^^^^^^^^ +... +LL | no_safety_comment!(()); + | ---------------------- in this macro invocation + | + = note: this error originates in the macro `no_safety_comment` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: missing safety comment on unsafe impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:93:5 + | +LL | unsafe impl T for i32 {} + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: missing safety comment on unsafe impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:85:13 + | +LL | unsafe impl T for $t {} + | ^^^^^^^^^^^^^^^^^^^^^^^ +... +LL | no_safety_comment!(u32); + | ----------------------- in this macro invocation + | + = note: this error originates in the macro `no_safety_comment` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: missing safety comment on unsafe impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:98:5 + | +LL | unsafe impl T for bool {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: missing safety comment on unsafe impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:141:5 + | +LL | unsafe impl NoComment for () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: missing safety comment on unsafe impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:146:19 + | +LL | /* SAFETY: */ unsafe impl InlineComment for () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: missing safety comment on unsafe impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:151:5 + | +LL | unsafe impl TrailingComment for () {} // SAFETY: + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: missing safety comment on unsafe impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:156:5 + | +LL | unsafe impl Interference for () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: missing safety comment on unsafe impl + --> tests/ui/safety/proper_safety_comment/trait_impl.rs:162:5 + | +LL | unsafe impl MultiLineBlockComment for () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 16 previous errors + diff --git a/tests/ui/safety/proper_safety_comment/unsafe_extern.rs b/tests/ui/safety/proper_safety_comment/unsafe_extern.rs new file mode 100644 index 000000000000..41f098977125 --- /dev/null +++ b/tests/ui/safety/proper_safety_comment/unsafe_extern.rs @@ -0,0 +1,68 @@ +#![warn(clippy::proper_safety_comment)] + +//~v ERROR: missing safety comment on `unsafe extern`-block +unsafe extern "C" { + //~v ERROR: missing safety comment on item in `unsafe extern`-block + pub safe fn f1(); + + //~v ERROR: missing safety comment on item in `unsafe extern`-block + pub unsafe fn f2(); + + //~v ERROR: missing safety comment on item in `unsafe extern`-block + pub fn f3(); +} + +//~v ERROR: missing safety comment on `unsafe extern`-block +extern "C" { + //~v ERROR: missing safety comment on item in `unsafe extern`-block + pub fn f4(); +} + +// SAFETY: +unsafe extern "C" { + // SAFETY: + pub safe fn g1(); + + // SAFETY: + pub unsafe fn g2(); + + // SAFETY: + pub fn g3(); +} + +// SAFETY: +extern "C" { + // SAFETY: + pub fn g4(); +} + +// SAFETY: + +unsafe extern "C" { + // SAFETY: + + // ... + + pub safe fn h1(); + + // SAFETY: + // ... + + pub unsafe fn h2(); + + // SAFETY: + + pub fn h3(); +} + +// SAFETY: + +// ... + +extern "C" { + // SAFETY: + + pub fn h4(); +} + +fn main() {} diff --git a/tests/ui/safety/proper_safety_comment/unsafe_extern.stderr b/tests/ui/safety/proper_safety_comment/unsafe_extern.stderr new file mode 100644 index 000000000000..43deb239cf53 --- /dev/null +++ b/tests/ui/safety/proper_safety_comment/unsafe_extern.stderr @@ -0,0 +1,49 @@ +error: missing safety comment on `unsafe extern`-block + --> tests/ui/safety/proper_safety_comment/unsafe_extern.rs:4:1 + | +LL | / unsafe extern "C" { +LL | | +LL | | pub safe fn f1(); +... | +LL | | pub fn f3(); +LL | | } + | |_^ + | + = note: `-D clippy::proper-safety-comment` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::proper_safety_comment)]` + +error: missing safety comment on item in `unsafe extern`-block + --> tests/ui/safety/proper_safety_comment/unsafe_extern.rs:6:5 + | +LL | pub safe fn f1(); + | ^^^^^^^^^^^^^^^^^ + +error: missing safety comment on item in `unsafe extern`-block + --> tests/ui/safety/proper_safety_comment/unsafe_extern.rs:9:5 + | +LL | pub unsafe fn f2(); + | ^^^^^^^^^^^^^^^^^^^ + +error: missing safety comment on item in `unsafe extern`-block + --> tests/ui/safety/proper_safety_comment/unsafe_extern.rs:12:5 + | +LL | pub fn f3(); + | ^^^^^^^^^^^^ + +error: missing safety comment on `unsafe extern`-block + --> tests/ui/safety/proper_safety_comment/unsafe_extern.rs:16:1 + | +LL | / extern "C" { +LL | | +LL | | pub fn f4(); +LL | | } + | |_^ + +error: missing safety comment on item in `unsafe extern`-block + --> tests/ui/safety/proper_safety_comment/unsafe_extern.rs:18:5 + | +LL | pub fn f4(); + | ^^^^^^^^^^^^ + +error: aborting due to 6 previous errors +