Skip to content

Commit

Permalink
Implement Visitor, support macro definitions and calls
Browse files Browse the repository at this point in the history
  • Loading branch information
@Serial-ATA
Serial-ATA committed Oct 5, 2021
1 parent 1ffaac1 commit 760115c
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 104 deletions.
119 changes: 27 additions & 92 deletions clippy_lints/src/undocumented_unsafe_blocks.rs
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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);
}
}
}
Expand All @@ -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);
}
}
}
}
Expand All @@ -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::<Vec<_>>();
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<Self::Map> {
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, ".."));

Expand All @@ -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)?;
Expand Down
49 changes: 49 additions & 0 deletions tests/ui/undocumented_unsafe_blocks.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {};
Expand All @@ -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() {
Expand Down Expand Up @@ -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:
}
Expand Down
52 changes: 40 additions & 12 deletions tests/ui/undocumented_unsafe_blocks.stderr
View file
Original file line number Diff line number Diff line change
@@ -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 {}
| ^^^^^^^^^
Expand All @@ -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 }];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -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 {});
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -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 };
| ^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -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 | | _ => {},
Expand All @@ -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 {};
| ^^^^^^^^^^^^^^^^^^^
Expand All @@ -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];
| ^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -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 {};
| ^^^^^^^^^^^^^^^^^^
Expand All @@ -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:
| ^^^^^^^^^
Expand All @@ -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:
Expand All @@ -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 {};
| ^^^^^^^^^
Expand All @@ -139,5 +167,5 @@ LL ~ // Safety: ...
LL ~ unsafe {};
|

error: aborting due to 11 previous errors
error: aborting due to 13 previous errors

0 comments on commit 760115c

Please sign in to comment.