diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index f8be71704c09..ee8d7a24e358 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -1,15 +1,17 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::{snippet, indent_of, reindent_multiline}; use rustc_errors::Applicability; -use rustc_hir::{Block, BlockCheckMode, Expr, ExprKind, HirId, Local, MatchSource, UnsafeSource, YieldSource}; +use rustc_hir::{Block, BlockCheckMode, Expr, ExprKind, HirId, Local, UnsafeSource}; use rustc_lexer::TokenKind; +use rustc_middle::hir::map::Map; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::TyCtxt; use rustc_session::{impl_lint_pass, declare_tool_lint}; use rustc_span::{BytePos, Span}; use std::borrow::Cow; -use clippy_utils::is_lint_allowed; +use clippy_utils::{is_lint_allowed, in_macro}; +use rustc_hir::intravisit::{NestedVisitorMap, Visitor, walk_expr}; declare_clippy_lint! { /// ### What it does @@ -70,13 +72,13 @@ impl LateLintPass<'_> for UndocumentedUnsafeBlocks { let result = block_has_safety_comment(cx.tcx, enclosing_scope_hir_id, span); - if result == Some(true) || result.is_none() { + if result.unwrap_or(true) { self.local_checked = true; return; } } - err(cx, span); + lint(cx, span); } } } @@ -86,12 +88,12 @@ impl LateLintPass<'_> for UndocumentedUnsafeBlocks { if !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, local.hir_id); if !in_external_macro(cx.tcx.sess, local.span); if let Some(init) = local.init; - if let Some((block, element_count)) = find_block_in_expr(&init.kind); - if let BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) = block.rules; - if cx.tcx.hir().get_enclosing_scope(block.hir_id).is_some(); then { - self.local_level = self.local_level.saturating_add(element_count); - self.local_span = Some(local.span); + self.visit_expr(init); + + if self.local_level > 0 { + self.local_span = Some(local.span); + } } } } @@ -106,94 +108,22 @@ impl LateLintPass<'_> for UndocumentedUnsafeBlocks { } } -fn find_block_in_expr(expr_kind: &ExprKind<'hir>) -> Option<(&'tcx Block<'hir>, u32)> { - match expr_kind { - ExprKind::Array(elements) | ExprKind::Tup(elements) => { - let unsafe_blocks = elements - .iter() - .filter_map(|e| match e { - Expr { - kind: - ExprKind::Block( - block - @ - Block { - rules: BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided), - .. - }, - _, - ), - .. - } => Some(block), - _ => None, - }) - .collect::>(); +impl<'hir> Visitor<'hir> for UndocumentedUnsafeBlocks { + type Map = Map<'hir>; - if let Some(block) = unsafe_blocks.get(0) { - return Some((block, unsafe_blocks.len().try_into().unwrap())); - } + fn nested_visit_map(&mut self) -> NestedVisitorMap { + NestedVisitorMap::None + } - None + fn visit_expr(&mut self, ex: &'v Expr<'v>) { + match ex.kind { + ExprKind::Block(_, _) => self.local_level = self.local_level.saturating_add(1), + _ => walk_expr(self, ex) } - ExprKind::Box(Expr { - kind: ExprKind::Block(block, _), - .. - }) - | ExprKind::Unary( - _, - Expr { - kind: ExprKind::Block(block, _), - .. - }, - ) - | ExprKind::Match( - Expr { - kind: ExprKind::Block(block, _), - .. - }, - _, - MatchSource::Normal, - ) - | ExprKind::Loop(block, _, _, _) - | ExprKind::Block(block, _) - | ExprKind::AddrOf( - _, - _, - Expr { - kind: ExprKind::Block(block, _), - .. - }, - ) - | ExprKind::Break( - _, - Some(Expr { - kind: ExprKind::Block(block, _), - .. - }), - ) - | ExprKind::Ret(Some(Expr { - kind: ExprKind::Block(block, _), - .. - })) - | ExprKind::Repeat( - Expr { - kind: ExprKind::Block(block, _), - .. - }, - _, - ) - | ExprKind::Yield( - Expr { - kind: ExprKind::Block(block, _), - .. - }, - YieldSource::Yield, - ) => Some((block, 1)), - _ => None, } } -fn err(cx: &LateContext<'_>, span: Span) { +fn lint(cx: &LateContext<'_>, span: Span) { let block_indent = indent_of(cx, span); let suggestion = format!("// Safety: ...\n{}", snippet(cx, span, "..")); @@ -217,7 +147,12 @@ fn block_has_safety_comment( let source_map = tcx.sess.source_map(); let enclosing_scope_span = map.opt_span(enclosing_hir_id)?; - let between_span = enclosing_scope_span.to(block_span); + + let between_span = if in_macro(block_span) { + enclosing_scope_span.with_hi(block_span.hi()) + } else { + enclosing_scope_span.to(block_span) + }; let file_name = source_map.span_to_filename(between_span); let source_file = source_map.get_source_file(&file_name)?; diff --git a/tests/ui/undocumented_unsafe_blocks.rs b/tests/ui/undocumented_unsafe_blocks.rs index f887843aba81..52577323a583 100644 --- a/tests/ui/undocumented_unsafe_blocks.rs +++ b/tests/ui/undocumented_unsafe_blocks.rs @@ -169,6 +169,30 @@ fn comment_repeat() { let _ = [unsafe {}; 5]; } +fn comment_macro_call() { + macro_rules! t { + ($b:expr) => { + $b + }; + } + + t!( + // Safety: + unsafe {} + ); +} + +fn comment_macro_def() { + macro_rules! t { + () => { + // Safety: + unsafe {} + }; + } + + t!(); +} + fn non_ascii_comment() { // ॐ᧻໒ Safety: ௵∰ unsafe {}; @@ -180,6 +204,11 @@ fn local_commented_block() { unsafe {}; } +fn local_nest() { + // Safety: + let _ = [(42, unsafe {}, unsafe {}), (52, unsafe {}, unsafe {})]; +} + // Invalid comments fn no_comment() { @@ -217,6 +246,26 @@ fn local_no_comment() { let _ = unsafe {}; } +fn no_comment_macro_call() { + macro_rules! t { + ($b:expr) => { + $b + }; + } + + t!(unsafe {}); +} + +fn no_comment_macro_def() { + macro_rules! t { + () => { + unsafe {} + }; + } + + t!(); +} + fn trailing_comment() { unsafe {} // Safety: } diff --git a/tests/ui/undocumented_unsafe_blocks.stderr b/tests/ui/undocumented_unsafe_blocks.stderr index 7baab11034a8..3f88ae4cec4d 100644 --- a/tests/ui/undocumented_unsafe_blocks.stderr +++ b/tests/ui/undocumented_unsafe_blocks.stderr @@ -1,5 +1,5 @@ error: unsafe block missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:186:5 + --> $DIR/undocumented_unsafe_blocks.rs:215:5 | LL | unsafe {} | ^^^^^^^^^ @@ -12,7 +12,7 @@ LL + unsafe {} | error: unsafe block missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:190:5 + --> $DIR/undocumented_unsafe_blocks.rs:219:5 | LL | let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -24,7 +24,7 @@ LL + let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }]; | error: unsafe block missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:194:5 + --> $DIR/undocumented_unsafe_blocks.rs:223:5 | LL | let _ = (42, unsafe {}, "test", unsafe {}); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -36,7 +36,7 @@ LL + let _ = (42, unsafe {}, "test", unsafe {}); | error: unsafe block missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:198:5 + --> $DIR/undocumented_unsafe_blocks.rs:227:5 | LL | let _ = *unsafe { &42 }; | ^^^^^^^^^^^^^^^^^^^^^^^^ @@ -48,7 +48,7 @@ LL + let _ = *unsafe { &42 }; | error: unsafe block missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:203:5 + --> $DIR/undocumented_unsafe_blocks.rs:232:5 | LL | / let _ = match unsafe {} { LL | | _ => {}, @@ -64,7 +64,7 @@ LL + }; | error: unsafe block missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:209:5 + --> $DIR/undocumented_unsafe_blocks.rs:238:5 | LL | let _ = &unsafe {}; | ^^^^^^^^^^^^^^^^^^^ @@ -76,7 +76,7 @@ LL + let _ = &unsafe {}; | error: unsafe block missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:213:5 + --> $DIR/undocumented_unsafe_blocks.rs:242:5 | LL | let _ = [unsafe {}; 5]; | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -88,7 +88,7 @@ LL + let _ = [unsafe {}; 5]; | error: unsafe block missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:217:5 + --> $DIR/undocumented_unsafe_blocks.rs:246:5 | LL | let _ = unsafe {}; | ^^^^^^^^^^^^^^^^^^ @@ -100,7 +100,35 @@ LL + let _ = unsafe {}; | error: unsafe block missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:221:5 + --> $DIR/undocumented_unsafe_blocks.rs:256:8 + | +LL | t!(unsafe {}); + | ^^^^^^^^^ + | +help: consider adding a safety comment + | +LL ~ t!(// Safety: ... +LL ~ unsafe {}); + | + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:262:13 + | +LL | unsafe {} + | ^^^^^^^^^ +... +LL | t!(); + | ----- in this macro invocation + | + = note: this error originates in the macro `t` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider adding a safety comment + | +LL ~ // Safety: ... +LL + unsafe {} + | + +error: unsafe block missing a safety comment + --> $DIR/undocumented_unsafe_blocks.rs:270:5 | LL | unsafe {} // Safety: | ^^^^^^^^^ @@ -112,7 +140,7 @@ LL ~ unsafe {} // Safety: | error: unsafe block missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:225:5 + --> $DIR/undocumented_unsafe_blocks.rs:274:5 | LL | / unsafe { LL | | // Safety: @@ -128,7 +156,7 @@ LL + } | error: unsafe block missing a safety comment - --> $DIR/undocumented_unsafe_blocks.rs:235:5 + --> $DIR/undocumented_unsafe_blocks.rs:284:5 | LL | unsafe {}; | ^^^^^^^^^ @@ -139,5 +167,5 @@ LL ~ // Safety: ... LL ~ unsafe {}; | -error: aborting due to 11 previous errors +error: aborting due to 13 previous errors