Skip to content

Commit aaf87c3

Browse files
tamaroningtamaroning
committed
fix
1 parent 8d85889 commit aaf87c3

File tree

3 files changed

+176
-42
lines changed

3 files changed

+176
-42
lines changed

‎clippy_lints/src/undocumented_unsafe_blocks.rs

Lines changed: 67 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -140,55 +140,59 @@ fn block_has_safety_comment(cx: &LateContext<'_>, block: &hir::Block<'_>) -> boo
140140
}
141141

142142
/// Checks if the lines immediately preceding the item contain a safety comment.
143+
#[allow(clippy::collapsible_match)]
143144
fn item_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) -> bool {
144145
if span_from_macro_expansion_has_safety_comment(cx, item.span) || span_in_body_has_safety_comment(cx, item.span) {
145146
return true;
146147
}
147148

148149
if item.span.ctxt() == SyntaxContext::root() {
149150
if let Some(parent_node) = get_parent_node(cx.tcx, item.hir_id()) {
150-
let mut span_before_item = None;
151-
let mut hi = false;
152-
if let Node::Item(parent_item) = parent_node {
153-
if let ItemKind::Mod(parent_mod) = &parent_item.kind {
154-
for (idx, item_id) in parent_mod.item_ids.iter().enumerate() {
155-
if *item_id == item.item_id() {
156-
if idx == 0 {
157-
// mod A { /* comment */ unsafe impl T {} ... }
158-
// ^------------------------------------------^ gets this span
159-
// ^---------------------^ finally checks the text in this range
160-
hi = false;
161-
span_before_item = Some(parent_item.span);
162-
} else {
163-
let prev_item = cx.tcx.hir().item(parent_mod.item_ids[idx - 1]);
164-
// some_item /* comment */ unsafe impl T {}
165-
// ^-------^ gets this span
166-
// ^---------------^ finally checks the text in this range
167-
hi = true;
168-
span_before_item = Some(prev_item.span);
169-
}
170-
break;
151+
let comment_start;
152+
match parent_node {
153+
Node::Crate(parent_mod) => {
154+
comment_start = comment_start_before_impl_in_mod(cx, parent_mod, parent_mod.spans.inner_span, item);
155+
},
156+
Node::Item(parent_item) => {
157+
if let ItemKind::Mod(parent_mod) = &parent_item.kind {
158+
comment_start = comment_start_before_impl_in_mod(cx, parent_mod, parent_item.span, item);
159+
} else {
160+
// Doesn't support impls in this position. Pretend a comment was found.
161+
return true;
162+
}
163+
},
164+
Node::Stmt(stmt) => {
165+
if let Some(stmt_parent) = get_parent_node(cx.tcx, stmt.hir_id) {
166+
match stmt_parent {
167+
Node::Block(block) => {
168+
comment_start = walk_span_to_context(block.span, SyntaxContext::root()).map(Span::lo);
169+
},
170+
_ => {
171+
// Doesn't support impls in this position. Pretend a comment was found.
172+
return true;
173+
},
171174
}
175+
} else {
176+
// Doesn't support impls in this position. Pretend a comment was found.
177+
return true;
172178
}
173-
}
179+
},
180+
_ => {
181+
// Doesn't support impls in this position. Pretend a comment was found.
182+
return true;
183+
},
174184
}
175-
let span_before_item = span_before_item.unwrap();
176185

177186
let source_map = cx.sess().source_map();
178-
if let Some(item_span) = walk_span_to_context(item.span, SyntaxContext::root())
179-
&& let Some(span_before_item) = walk_span_to_context(span_before_item, SyntaxContext::root())
180-
&& let Ok(unsafe_line) = source_map.lookup_line(item_span.lo())
181-
&& let Ok(line_before_unsafe) = source_map.lookup_line(if hi {
182-
span_before_item.hi()
183-
} else {
184-
span_before_item.lo()
185-
})
186-
&& Lrc::ptr_eq(&unsafe_line.sf, &line_before_unsafe.sf)
187+
if let Some(comment_start) = comment_start
188+
&& let Ok(unsafe_line) = source_map.lookup_line(item.span.lo())
189+
&& let Ok(comment_start_line) = source_map.lookup_line(comment_start)
190+
&& Lrc::ptr_eq(&unsafe_line.sf, &comment_start_line.sf)
187191
&& let Some(src) = unsafe_line.sf.src.as_deref()
188192
{
189-
line_before_unsafe.line < unsafe_line.line && text_has_safety_comment(
193+
comment_start_line.line < unsafe_line.line && text_has_safety_comment(
190194
src,
191-
&unsafe_line.sf.lines[line_before_unsafe.line + 1..=unsafe_line.line],
195+
&unsafe_line.sf.lines[comment_start_line.line + 1..=unsafe_line.line],
192196
unsafe_line.sf.start_pos.to_usize(),
193197
)
194198
} else {
@@ -204,6 +208,35 @@ fn item_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) -> bool {
204208
}
205209
}
206210

211+
fn comment_start_before_impl_in_mod(
212+
cx: &LateContext<'_>,
213+
parent_mod: &hir::Mod<'_>,
214+
parent_mod_span: Span,
215+
imple: &hir::Item<'_>,
216+
) -> Option<BytePos> {
217+
parent_mod.item_ids.iter().enumerate().find_map(|(idx, item_id)| {
218+
if *item_id == imple.item_id() {
219+
if idx == 0 {
220+
// mod A { /* comment */ unsafe impl T {} ... }
221+
// ^------------------------------------------^ returns the start of this span
222+
// ^---------------^ finally checks comments in this range
223+
if let Some(sp) = walk_span_to_context(parent_mod_span, SyntaxContext::root()) {
224+
return Some(sp.lo());
225+
}
226+
} else {
227+
// some_item /* comment */ unsafe impl T {}
228+
// ^-------^ returns the end of this span
229+
// ^---------------^ finally checks comments in this range
230+
let prev_item = cx.tcx.hir().item(parent_mod.item_ids[idx - 1]);
231+
if let Some(sp) = walk_span_to_context(prev_item.span, SyntaxContext::root()) {
232+
return Some(sp.hi());
233+
}
234+
}
235+
}
236+
None
237+
})
238+
}
239+
207240
fn span_from_macro_expansion_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
208241
let source_map = cx.sess().source_map();
209242
let ctxt = span.ctxt();

‎tests/ui/undocumented_unsafe_blocks.rs

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ mod unsafe_impl_smoke_test {
344344
unsafe impl A for (i32) {}
345345

346346
mod sub_mod {
347-
// error: also works for the first item
347+
// error:
348348
unsafe impl B for (u32) {}
349349
unsafe trait B {}
350350
}
@@ -363,24 +363,51 @@ mod unsafe_impl_smoke_test {
363363
mod unsafe_impl_from_macro {
364364
unsafe trait T {}
365365

366+
// error
366367
macro_rules! no_safety_comment {
367368
($t:ty) => {
368369
unsafe impl T for $t {}
369370
};
370371
}
371-
// error
372+
373+
// ok
372374
no_safety_comment!(());
373375

376+
// ok
374377
macro_rules! with_safety_comment {
375378
($t:ty) => {
376379
// SAFETY:
377380
unsafe impl T for $t {}
378381
};
379382
}
383+
380384
// ok
381385
with_safety_comment!((i32));
382386
}
383387

388+
mod unsafe_impl_macro_and_not_macro {
389+
unsafe trait T {}
390+
391+
// error
392+
macro_rules! no_safety_comment {
393+
($t:ty) => {
394+
unsafe impl T for $t {}
395+
};
396+
}
397+
398+
// ok
399+
no_safety_comment!(());
400+
401+
// error
402+
unsafe impl T for (i32) {}
403+
404+
// ok
405+
no_safety_comment!(u32);
406+
407+
// error
408+
unsafe impl T for (bool) {}
409+
}
410+
384411
#[rustfmt::skip]
385412
mod unsafe_impl_valid_comment {
386413
unsafe trait SaFety {}
@@ -440,4 +467,22 @@ mod unsafe_impl_invalid_comment {
440467
unsafe impl Interference for () {}
441468
}
442469

470+
unsafe trait ImplInFn {}
471+
472+
fn impl_in_fn() {
473+
// error
474+
unsafe impl ImplInFn for () {}
475+
476+
// SAFETY: ok
477+
unsafe impl ImplInFn for (i32) {}
478+
}
479+
480+
unsafe trait CrateRoot {}
481+
482+
// error
483+
unsafe impl CrateRoot for () {}
484+
485+
// SAFETY: ok
486+
unsafe impl CrateRoot for (i32) {}
487+
443488
fn main() {}

‎tests/ui/undocumented_unsafe_blocks.stderr

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ LL | unsafe impl B for (u32) {}
164164
= help: consider adding a safety comment on the preceding line
165165

166166
error: unsafe impl missing a safety comment
167-
--> $DIR/undocumented_unsafe_blocks.rs:368:13
167+
--> $DIR/undocumented_unsafe_blocks.rs:369:13
168168
|
169169
LL | unsafe impl T for $t {}
170170
| ^^^^^^^^^^^^^^^^^^^^^^^
@@ -176,36 +176,92 @@ LL | no_safety_comment!(());
176176
= note: this error originates in the macro `no_safety_comment` (in Nightly builds, run with -Z macro-backtrace for more info)
177177

178178
error: unsafe impl missing a safety comment
179-
--> $DIR/undocumented_unsafe_blocks.rs:427:5
179+
--> $DIR/undocumented_unsafe_blocks.rs:394:13
180+
|
181+
LL | unsafe impl T for $t {}
182+
| ^^^^^^^^^^^^^^^^^^^^^^^
183+
...
184+
LL | no_safety_comment!(());
185+
| ---------------------- in this macro invocation
186+
|
187+
= help: consider adding a safety comment on the preceding line
188+
= note: this error originates in the macro `no_safety_comment` (in Nightly builds, run with -Z macro-backtrace for more info)
189+
190+
error: unsafe impl missing a safety comment
191+
--> $DIR/undocumented_unsafe_blocks.rs:402:5
192+
|
193+
LL | unsafe impl T for (i32) {}
194+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
195+
|
196+
= help: consider adding a safety comment on the preceding line
197+
198+
error: unsafe impl missing a safety comment
199+
--> $DIR/undocumented_unsafe_blocks.rs:394:13
200+
|
201+
LL | unsafe impl T for $t {}
202+
| ^^^^^^^^^^^^^^^^^^^^^^^
203+
...
204+
LL | no_safety_comment!(u32);
205+
| ----------------------- in this macro invocation
206+
|
207+
= help: consider adding a safety comment on the preceding line
208+
= note: this error originates in the macro `no_safety_comment` (in Nightly builds, run with -Z macro-backtrace for more info)
209+
210+
error: unsafe impl missing a safety comment
211+
--> $DIR/undocumented_unsafe_blocks.rs:408:5
212+
|
213+
LL | unsafe impl T for (bool) {}
214+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
215+
|
216+
= help: consider adding a safety comment on the preceding line
217+
218+
error: unsafe impl missing a safety comment
219+
--> $DIR/undocumented_unsafe_blocks.rs:454:5
180220
|
181221
LL | unsafe impl NoComment for () {}
182222
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
183223
|
184224
= help: consider adding a safety comment on the preceding line
185225

186226
error: unsafe impl missing a safety comment
187-
--> $DIR/undocumented_unsafe_blocks.rs:431:19
227+
--> $DIR/undocumented_unsafe_blocks.rs:458:19
188228
|
189229
LL | /* SAFETY: */ unsafe impl InlineComment for () {}
190230
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
191231
|
192232
= help: consider adding a safety comment on the preceding line
193233

194234
error: unsafe impl missing a safety comment
195-
--> $DIR/undocumented_unsafe_blocks.rs:435:5
235+
--> $DIR/undocumented_unsafe_blocks.rs:462:5
196236
|
197237
LL | unsafe impl TrailingComment for () {} // SAFETY:
198238
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
199239
|
200240
= help: consider adding a safety comment on the preceding line
201241

202242
error: unsafe impl missing a safety comment
203-
--> $DIR/undocumented_unsafe_blocks.rs:440:5
243+
--> $DIR/undocumented_unsafe_blocks.rs:467:5
204244
|
205245
LL | unsafe impl Interference for () {}
206246
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
207247
|
208248
= help: consider adding a safety comment on the preceding line
209249

210-
error: aborting due to 25 previous errors
250+
error: unsafe impl missing a safety comment
251+
--> $DIR/undocumented_unsafe_blocks.rs:474:5
252+
|
253+
LL | unsafe impl ImplInFn for () {}
254+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
255+
|
256+
= help: consider adding a safety comment on the preceding line
257+
258+
error: unsafe impl missing a safety comment
259+
--> $DIR/undocumented_unsafe_blocks.rs:483:1
260+
|
261+
LL | unsafe impl CrateRoot for () {}
262+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
263+
|
264+
= help: consider adding a safety comment on the preceding line
265+
266+
error: aborting due to 31 previous errors
211267

0 commit comments

Comments
 (0)