From 5d0ca74c753d72bf5b6dcfa532001cdd07aa0365 Mon Sep 17 00:00:00 2001 From: tamaron <tamaron1203@gmail.com> Date: Fri, 29 Apr 2022 18:34:58 +0900 Subject: [PATCH] Resolved conflicts --- .../src/undocumented_unsafe_blocks.rs | 64 ++++++++++- tests/ui/undocumented_unsafe_blocks.rs | 101 +++++++++++++++++- tests/ui/undocumented_unsafe_blocks.stderr | 50 ++++++++- 3 files changed, 212 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index 465d8a914fb..63652dba398 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -2,12 +2,14 @@ use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::is_lint_allowed; use clippy_utils::source::walk_span_to_context; use rustc_data_structures::sync::Lrc; +use rustc_hir as hir; use rustc_hir::{Block, BlockCheckMode, UnsafeSource}; use rustc_lexer::{tokenize, TokenKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::{BytePos, Pos, SyntaxContext}; +use rustc_span::{BytePos, Pos, Span, SyntaxContext}; +use std::rc::Rc; declare_clippy_lint! { /// ### What it does @@ -86,6 +88,66 @@ impl LateLintPass<'_> for UndocumentedUnsafeBlocks { ); } } + + fn check_mod(&mut self, cx: &LateContext<'_>, module: &'_ hir::Mod<'_>, mod_span: Span, hir_id: hir::HirId) { + let source_map = cx.sess().source_map(); + let mut item_and_spans: Vec<(&hir::Item<'_>, Span)> = Vec::new(); // (start, end, item) + + // Collect all items and their spans + for item_id in module.item_ids { + let item = cx.tcx.hir().item(*item_id); + item_and_spans.push((item, item.span)); + } + // Sort items by start position + item_and_spans.sort_by_key(|e| e.1.lo()); + + for (idx, (item, item_span)) in item_and_spans.iter().enumerate() { + if let hir::ItemKind::Impl(imple) = &item.kind + && imple.unsafety == hir::Unsafety::Unsafe + && !item_span.from_expansion() + && !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, hir_id) + { + // Checks if the lines immediately preceding the impl contain a safety comment. + let impl_has_safety_comment = { + let span_before_impl = if idx == 0 { + // mod A { /* comment */ unsafe impl T {} } + // ^--------------------^ + todo!(); + //mod_span.until(module.spans) + } else { + // unsafe impl S {} /* comment */ unsafe impl T {} + // ^-------------^ + item_and_spans[idx - 1].1.between(*item_span) + }; + + if let Ok(start) = source_map.lookup_line(span_before_impl.lo()) + && let Ok(end) = source_map.lookup_line(span_before_impl.hi()) + && let Some(src) = start.sf.src.as_deref() + { + start.line < end.line && text_has_safety_comment( + src, + &start.sf.lines[start.line + 1 ..= end.line], + start.sf.start_pos.to_usize() + ) + } else { + // Problem getting source text. Pretend a comment was found. + true + } + }; + + if !impl_has_safety_comment { + span_lint_and_help( + cx, + UNDOCUMENTED_UNSAFE_BLOCKS, + *item_span, + "unsafe impl missing a safety comment", + None, + "consider adding a safety comment on the preceding line", + ); + } + } + } + } } fn is_unsafe_from_proc_macro(cx: &LateContext<'_>, block: &Block<'_>) -> bool { diff --git a/tests/ui/undocumented_unsafe_blocks.rs b/tests/ui/undocumented_unsafe_blocks.rs index 7be15b0b2dd..3044a92cbd0 100644 --- a/tests/ui/undocumented_unsafe_blocks.rs +++ b/tests/ui/undocumented_unsafe_blocks.rs @@ -1,7 +1,7 @@ // aux-build:proc_macro_unsafe.rs #![warn(clippy::undocumented_unsafe_blocks)] -#![allow(clippy::let_unit_value)] +#![allow(clippy::let_unit_value, clippy::missing_safety_doc)] extern crate proc_macro_unsafe; @@ -334,4 +334,103 @@ pub fn print_binary_tree() { println!("{}", unsafe { String::from_utf8_unchecked(vec![]) }); } +mod unsafe_impl_smoke_test { + unsafe trait A {} + + // error: no safety comment + unsafe impl A for () {} + + // Safety: ok + unsafe impl A for (i32) {} + + mod sub_mod { + // error: also works for the first item + unsafe impl B for (u32) {} + unsafe trait B {} + } + + #[rustfmt::skip] + mod sub_mod2 { + // + // SAFETY: ok + // + + unsafe impl B for (u32) {} + unsafe trait B {} + } +} + +mod unsafe_impl_from_macro { + unsafe trait T {} + + macro_rules! unsafe_impl { + ($t:ty) => { + unsafe impl T for $t {} + }; + } + // ok: from macro expanision + unsafe_impl!(()); + // ok: from macro expansion + unsafe_impl!(i32); +} + +#[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 () {} + + unsafe trait MultiLineBlockComment {} + /* This is a description + * Safety: */ + unsafe impl MultiLineBlockComment for () {} +} + +#[rustfmt::skip] +mod unsafe_impl_invalid_comment { + unsafe trait NoComment {} + + unsafe impl NoComment for () {} + + unsafe trait InlineComment {} + + /* SAFETY: */ unsafe impl InlineComment for () {} + + unsafe trait TrailingComment {} + + unsafe impl TrailingComment for () {} // SAFETY: + + unsafe trait Interference {} + // SAFETY: + const BIG_NUMBER: i32 = 1000000; + unsafe impl Interference for () {} +} + fn main() {} diff --git a/tests/ui/undocumented_unsafe_blocks.stderr b/tests/ui/undocumented_unsafe_blocks.stderr index 87d445bd7b8..80d68a03808 100644 --- a/tests/ui/undocumented_unsafe_blocks.stderr +++ b/tests/ui/undocumented_unsafe_blocks.stderr @@ -147,5 +147,53 @@ LL | println!("{}", unsafe { String::from_utf8_unchecked(vec![]) }); | = help: consider adding a safety comment on the preceding line -error: aborting due to 18 previous errors +error: unsafe impl missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:341:5 + | +LL | unsafe impl A for () {} + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe impl missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:348:9 + | +LL | unsafe impl B for (u32) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe impl missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:420:5 + | +LL | unsafe impl NoComment for () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe impl missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:424:19 + | +LL | /* SAFETY: */ unsafe impl InlineComment for () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe impl missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:428:5 + | +LL | unsafe impl TrailingComment for () {} // SAFETY: + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: unsafe impl missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:433:5 + | +LL | unsafe impl Interference for () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider adding a safety comment on the preceding line + +error: aborting due to 24 previous errors