Skip to content

Commit 760115c

Browse files
committed
Implement Visitor, support macro definitions and calls
1 parent 1ffaac1 commit 760115c

File tree

3 files changed

+116
-104
lines changed

3 files changed

+116
-104
lines changed

clippy_lints/src/undocumented_unsafe_blocks.rs

Lines changed: 27 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::source::{snippet, indent_of, reindent_multiline};
33
use rustc_errors::Applicability;
4-
use rustc_hir::{Block, BlockCheckMode, Expr, ExprKind, HirId, Local, MatchSource, UnsafeSource, YieldSource};
4+
use rustc_hir::{Block, BlockCheckMode, Expr, ExprKind, HirId, Local, UnsafeSource};
55
use rustc_lexer::TokenKind;
6+
use rustc_middle::hir::map::Map;
67
use rustc_lint::{LateContext, LateLintPass};
78
use rustc_middle::lint::in_external_macro;
89
use rustc_middle::ty::TyCtxt;
910
use rustc_session::{impl_lint_pass, declare_tool_lint};
1011
use rustc_span::{BytePos, Span};
1112
use std::borrow::Cow;
12-
use clippy_utils::is_lint_allowed;
13+
use clippy_utils::{is_lint_allowed, in_macro};
14+
use rustc_hir::intravisit::{NestedVisitorMap, Visitor, walk_expr};
1315

1416
declare_clippy_lint! {
1517
/// ### What it does
@@ -70,13 +72,13 @@ impl LateLintPass<'_> for UndocumentedUnsafeBlocks {
7072

7173
let result = block_has_safety_comment(cx.tcx, enclosing_scope_hir_id, span);
7274

73-
if result == Some(true) || result.is_none() {
75+
if result.unwrap_or(true) {
7476
self.local_checked = true;
7577
return;
7678
}
7779
}
7880

79-
err(cx, span);
81+
lint(cx, span);
8082
}
8183
}
8284
}
@@ -86,12 +88,12 @@ impl LateLintPass<'_> for UndocumentedUnsafeBlocks {
8688
if !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, local.hir_id);
8789
if !in_external_macro(cx.tcx.sess, local.span);
8890
if let Some(init) = local.init;
89-
if let Some((block, element_count)) = find_block_in_expr(&init.kind);
90-
if let BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) = block.rules;
91-
if cx.tcx.hir().get_enclosing_scope(block.hir_id).is_some();
9291
then {
93-
self.local_level = self.local_level.saturating_add(element_count);
94-
self.local_span = Some(local.span);
92+
self.visit_expr(init);
93+
94+
if self.local_level > 0 {
95+
self.local_span = Some(local.span);
96+
}
9597
}
9698
}
9799
}
@@ -106,94 +108,22 @@ impl LateLintPass<'_> for UndocumentedUnsafeBlocks {
106108
}
107109
}
108110

109-
fn find_block_in_expr(expr_kind: &ExprKind<'hir>) -> Option<(&'tcx Block<'hir>, u32)> {
110-
match expr_kind {
111-
ExprKind::Array(elements) | ExprKind::Tup(elements) => {
112-
let unsafe_blocks = elements
113-
.iter()
114-
.filter_map(|e| match e {
115-
Expr {
116-
kind:
117-
ExprKind::Block(
118-
block
119-
@
120-
Block {
121-
rules: BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided),
122-
..
123-
},
124-
_,
125-
),
126-
..
127-
} => Some(block),
128-
_ => None,
129-
})
130-
.collect::<Vec<_>>();
111+
impl<'hir> Visitor<'hir> for UndocumentedUnsafeBlocks {
112+
type Map = Map<'hir>;
131113

132-
if let Some(block) = unsafe_blocks.get(0) {
133-
return Some((block, unsafe_blocks.len().try_into().unwrap()));
134-
}
114+
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
115+
NestedVisitorMap::None
116+
}
135117

136-
None
118+
fn visit_expr(&mut self, ex: &'v Expr<'v>) {
119+
match ex.kind {
120+
ExprKind::Block(_, _) => self.local_level = self.local_level.saturating_add(1),
121+
_ => walk_expr(self, ex)
137122
}
138-
ExprKind::Box(Expr {
139-
kind: ExprKind::Block(block, _),
140-
..
141-
})
142-
| ExprKind::Unary(
143-
_,
144-
Expr {
145-
kind: ExprKind::Block(block, _),
146-
..
147-
},
148-
)
149-
| ExprKind::Match(
150-
Expr {
151-
kind: ExprKind::Block(block, _),
152-
..
153-
},
154-
_,
155-
MatchSource::Normal,
156-
)
157-
| ExprKind::Loop(block, _, _, _)
158-
| ExprKind::Block(block, _)
159-
| ExprKind::AddrOf(
160-
_,
161-
_,
162-
Expr {
163-
kind: ExprKind::Block(block, _),
164-
..
165-
},
166-
)
167-
| ExprKind::Break(
168-
_,
169-
Some(Expr {
170-
kind: ExprKind::Block(block, _),
171-
..
172-
}),
173-
)
174-
| ExprKind::Ret(Some(Expr {
175-
kind: ExprKind::Block(block, _),
176-
..
177-
}))
178-
| ExprKind::Repeat(
179-
Expr {
180-
kind: ExprKind::Block(block, _),
181-
..
182-
},
183-
_,
184-
)
185-
| ExprKind::Yield(
186-
Expr {
187-
kind: ExprKind::Block(block, _),
188-
..
189-
},
190-
YieldSource::Yield,
191-
) => Some((block, 1)),
192-
_ => None,
193123
}
194124
}
195125

196-
fn err(cx: &LateContext<'_>, span: Span) {
126+
fn lint(cx: &LateContext<'_>, span: Span) {
197127
let block_indent = indent_of(cx, span);
198128
let suggestion = format!("// Safety: ...\n{}", snippet(cx, span, ".."));
199129

@@ -217,7 +147,12 @@ fn block_has_safety_comment(
217147
let source_map = tcx.sess.source_map();
218148

219149
let enclosing_scope_span = map.opt_span(enclosing_hir_id)?;
220-
let between_span = enclosing_scope_span.to(block_span);
150+
151+
let between_span = if in_macro(block_span) {
152+
enclosing_scope_span.with_hi(block_span.hi())
153+
} else {
154+
enclosing_scope_span.to(block_span)
155+
};
221156

222157
let file_name = source_map.span_to_filename(between_span);
223158
let source_file = source_map.get_source_file(&file_name)?;

tests/ui/undocumented_unsafe_blocks.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,30 @@ fn comment_repeat() {
169169
let _ = [unsafe {}; 5];
170170
}
171171

172+
fn comment_macro_call() {
173+
macro_rules! t {
174+
($b:expr) => {
175+
$b
176+
};
177+
}
178+
179+
t!(
180+
// Safety:
181+
unsafe {}
182+
);
183+
}
184+
185+
fn comment_macro_def() {
186+
macro_rules! t {
187+
() => {
188+
// Safety:
189+
unsafe {}
190+
};
191+
}
192+
193+
t!();
194+
}
195+
172196
fn non_ascii_comment() {
173197
// ॐ᧻໒ Safety: ௵∰
174198
unsafe {};
@@ -180,6 +204,11 @@ fn local_commented_block() {
180204
unsafe {};
181205
}
182206

207+
fn local_nest() {
208+
// Safety:
209+
let _ = [(42, unsafe {}, unsafe {}), (52, unsafe {}, unsafe {})];
210+
}
211+
183212
// Invalid comments
184213

185214
fn no_comment() {
@@ -217,6 +246,26 @@ fn local_no_comment() {
217246
let _ = unsafe {};
218247
}
219248

249+
fn no_comment_macro_call() {
250+
macro_rules! t {
251+
($b:expr) => {
252+
$b
253+
};
254+
}
255+
256+
t!(unsafe {});
257+
}
258+
259+
fn no_comment_macro_def() {
260+
macro_rules! t {
261+
() => {
262+
unsafe {}
263+
};
264+
}
265+
266+
t!();
267+
}
268+
220269
fn trailing_comment() {
221270
unsafe {} // Safety:
222271
}

tests/ui/undocumented_unsafe_blocks.stderr

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: unsafe block missing a safety comment
2-
--> $DIR/undocumented_unsafe_blocks.rs:186:5
2+
--> $DIR/undocumented_unsafe_blocks.rs:215:5
33
|
44
LL | unsafe {}
55
| ^^^^^^^^^
@@ -12,7 +12,7 @@ LL + unsafe {}
1212
|
1313

1414
error: unsafe block missing a safety comment
15-
--> $DIR/undocumented_unsafe_blocks.rs:190:5
15+
--> $DIR/undocumented_unsafe_blocks.rs:219:5
1616
|
1717
LL | let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }];
1818
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -24,7 +24,7 @@ LL + let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }];
2424
|
2525

2626
error: unsafe block missing a safety comment
27-
--> $DIR/undocumented_unsafe_blocks.rs:194:5
27+
--> $DIR/undocumented_unsafe_blocks.rs:223:5
2828
|
2929
LL | let _ = (42, unsafe {}, "test", unsafe {});
3030
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -36,7 +36,7 @@ LL + let _ = (42, unsafe {}, "test", unsafe {});
3636
|
3737

3838
error: unsafe block missing a safety comment
39-
--> $DIR/undocumented_unsafe_blocks.rs:198:5
39+
--> $DIR/undocumented_unsafe_blocks.rs:227:5
4040
|
4141
LL | let _ = *unsafe { &42 };
4242
| ^^^^^^^^^^^^^^^^^^^^^^^^
@@ -48,7 +48,7 @@ LL + let _ = *unsafe { &42 };
4848
|
4949

5050
error: unsafe block missing a safety comment
51-
--> $DIR/undocumented_unsafe_blocks.rs:203:5
51+
--> $DIR/undocumented_unsafe_blocks.rs:232:5
5252
|
5353
LL | / let _ = match unsafe {} {
5454
LL | | _ => {},
@@ -64,7 +64,7 @@ LL + };
6464
|
6565

6666
error: unsafe block missing a safety comment
67-
--> $DIR/undocumented_unsafe_blocks.rs:209:5
67+
--> $DIR/undocumented_unsafe_blocks.rs:238:5
6868
|
6969
LL | let _ = &unsafe {};
7070
| ^^^^^^^^^^^^^^^^^^^
@@ -76,7 +76,7 @@ LL + let _ = &unsafe {};
7676
|
7777

7878
error: unsafe block missing a safety comment
79-
--> $DIR/undocumented_unsafe_blocks.rs:213:5
79+
--> $DIR/undocumented_unsafe_blocks.rs:242:5
8080
|
8181
LL | let _ = [unsafe {}; 5];
8282
| ^^^^^^^^^^^^^^^^^^^^^^^
@@ -88,7 +88,7 @@ LL + let _ = [unsafe {}; 5];
8888
|
8989

9090
error: unsafe block missing a safety comment
91-
--> $DIR/undocumented_unsafe_blocks.rs:217:5
91+
--> $DIR/undocumented_unsafe_blocks.rs:246:5
9292
|
9393
LL | let _ = unsafe {};
9494
| ^^^^^^^^^^^^^^^^^^
@@ -100,7 +100,35 @@ LL + let _ = unsafe {};
100100
|
101101

102102
error: unsafe block missing a safety comment
103-
--> $DIR/undocumented_unsafe_blocks.rs:221:5
103+
--> $DIR/undocumented_unsafe_blocks.rs:256:8
104+
|
105+
LL | t!(unsafe {});
106+
| ^^^^^^^^^
107+
|
108+
help: consider adding a safety comment
109+
|
110+
LL ~ t!(// Safety: ...
111+
LL ~ unsafe {});
112+
|
113+
114+
error: unsafe block missing a safety comment
115+
--> $DIR/undocumented_unsafe_blocks.rs:262:13
116+
|
117+
LL | unsafe {}
118+
| ^^^^^^^^^
119+
...
120+
LL | t!();
121+
| ----- in this macro invocation
122+
|
123+
= note: this error originates in the macro `t` (in Nightly builds, run with -Z macro-backtrace for more info)
124+
help: consider adding a safety comment
125+
|
126+
LL ~ // Safety: ...
127+
LL + unsafe {}
128+
|
129+
130+
error: unsafe block missing a safety comment
131+
--> $DIR/undocumented_unsafe_blocks.rs:270:5
104132
|
105133
LL | unsafe {} // Safety:
106134
| ^^^^^^^^^
@@ -112,7 +140,7 @@ LL ~ unsafe {} // Safety:
112140
|
113141

114142
error: unsafe block missing a safety comment
115-
--> $DIR/undocumented_unsafe_blocks.rs:225:5
143+
--> $DIR/undocumented_unsafe_blocks.rs:274:5
116144
|
117145
LL | / unsafe {
118146
LL | | // Safety:
@@ -128,7 +156,7 @@ LL + }
128156
|
129157

130158
error: unsafe block missing a safety comment
131-
--> $DIR/undocumented_unsafe_blocks.rs:235:5
159+
--> $DIR/undocumented_unsafe_blocks.rs:284:5
132160
|
133161
LL | unsafe {};
134162
| ^^^^^^^^^
@@ -139,5 +167,5 @@ LL ~ // Safety: ...
139167
LL ~ unsafe {};
140168
|
141169

142-
error: aborting due to 11 previous errors
170+
error: aborting due to 13 previous errors
143171

0 commit comments

Comments
 (0)