Auto merge of #10986 - Centri3:undocumented_unsafe_blocks, r=Manishearth

Allow safety comment above attributes

Closes #8679

changelog: Enhancement: [`undocumented_safety_block`]: Added `accept-comment-above-attributes` configuration.
This commit is contained in:
bors 2023-06-20 05:04:46 +00:00
commit 8fd021f504
9 changed files with 162 additions and 48 deletions

View File

@ -5385,4 +5385,5 @@ Released 2018-09-13
[`allowed-idents-below-min-chars`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allowed-idents-below-min-chars
[`min-ident-chars-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#min-ident-chars-threshold
[`accept-comment-above-statement`]: https://doc.rust-lang.org/clippy/lint_configuration.html#accept-comment-above-statement
[`accept-comment-above-attributes`]: https://doc.rust-lang.org/clippy/lint_configuration.html#accept-comment-above-attributes
<!-- end autogenerated links to configuration documentation -->

View File

@ -706,3 +706,13 @@ Whether to accept a safety comment to be placed above the statement containing t
* [`undocumented_unsafe_blocks`](https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks)
## `accept-comment-above-attributes`
Whether to accept a safety comment to be placed above the attributes for the `unsafe` block
**Default Value:** `false` (`bool`)
---
**Affected lints:**
* [`undocumented_unsafe_blocks`](https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks)

View File

@ -930,9 +930,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
))
});
let accept_comment_above_statement = conf.accept_comment_above_statement;
let accept_comment_above_attributes = conf.accept_comment_above_attributes;
store.register_late_pass(move |_| {
Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks::new(
accept_comment_above_statement,
accept_comment_above_attributes,
))
});
let allow_mixed_uninlined = conf.allow_mixed_uninlined_format_args;

View File

@ -95,12 +95,14 @@
#[derive(Copy, Clone)]
pub struct UndocumentedUnsafeBlocks {
accept_comment_above_statement: bool,
accept_comment_above_attributes: bool,
}
impl UndocumentedUnsafeBlocks {
pub fn new(accept_comment_above_statement: bool) -> Self {
pub fn new(accept_comment_above_statement: bool, accept_comment_above_attributes: bool) -> Self {
Self {
accept_comment_above_statement,
accept_comment_above_attributes,
}
}
}
@ -114,7 +116,12 @@ fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) {
&& !is_lint_allowed(cx, UNDOCUMENTED_UNSAFE_BLOCKS, block.hir_id)
&& !is_unsafe_from_proc_macro(cx, block.span)
&& !block_has_safety_comment(cx, block.span)
&& !block_parents_have_safety_comment(self.accept_comment_above_statement, cx, block.hir_id)
&& !block_parents_have_safety_comment(
self.accept_comment_above_statement,
self.accept_comment_above_attributes,
cx,
block.hir_id,
)
{
let source_map = cx.tcx.sess.source_map();
let span = if source_map.is_multiline(block.span) {
@ -328,6 +335,7 @@ fn is_unsafe_from_proc_macro(cx: &LateContext<'_>, span: Span) -> bool {
// has a safety comment
fn block_parents_have_safety_comment(
accept_comment_above_statement: bool,
accept_comment_above_attributes: bool,
cx: &LateContext<'_>,
id: hir::HirId,
) -> bool {
@ -343,33 +351,77 @@ fn block_parents_have_safety_comment(
}),
) = get_parent_node(cx.tcx, expr.hir_id)
{
let hir_id = match get_parent_node(cx.tcx, expr.hir_id) {
Some(Node::Local(hir::Local { hir_id, .. })) => *hir_id,
Some(Node::Item(hir::Item { owner_id, .. })) => {
cx.tcx.hir().local_def_id_to_hir_id(owner_id.def_id)
},
_ => unreachable!(),
};
// if unsafe block is part of a let/const/static statement,
// and accept_comment_above_statement is set to true
// we accept the safety comment in the line the precedes this statement.
accept_comment_above_statement && span_in_body_has_safety_comment(cx, *span)
accept_comment_above_statement
&& span_with_attrs_in_body_has_safety_comment(
cx,
*span,
hir_id,
accept_comment_above_attributes,
)
} else {
!is_branchy(expr) && span_in_body_has_safety_comment(cx, expr.span)
!is_branchy(expr)
&& span_with_attrs_in_body_has_safety_comment(
cx,
expr.span,
expr.hir_id,
accept_comment_above_attributes,
)
}
},
Node::Stmt(hir::Stmt {
kind:
hir::StmtKind::Local(hir::Local { span, .. })
| hir::StmtKind::Expr(hir::Expr { span, .. })
| hir::StmtKind::Semi(hir::Expr { span, .. }),
hir::StmtKind::Local(hir::Local { span, hir_id, .. })
| hir::StmtKind::Expr(hir::Expr { span, hir_id, .. })
| hir::StmtKind::Semi(hir::Expr { span, hir_id, .. }),
..
})
| Node::Local(hir::Local { span, .. })
| Node::Item(hir::Item {
| Node::Local(hir::Local { span, hir_id, .. }) => {
span_with_attrs_in_body_has_safety_comment(cx, *span, *hir_id, accept_comment_above_attributes)
},
Node::Item(hir::Item {
kind: hir::ItemKind::Const(..) | ItemKind::Static(..),
span,
owner_id,
..
}) => span_in_body_has_safety_comment(cx, *span),
}) => span_with_attrs_in_body_has_safety_comment(
cx,
*span,
cx.tcx.hir().local_def_id_to_hir_id(owner_id.def_id),
accept_comment_above_attributes,
),
_ => false,
};
}
false
}
/// Extends `span` to also include its attributes, then checks if that span has a safety comment.
fn span_with_attrs_in_body_has_safety_comment(
cx: &LateContext<'_>,
span: Span,
hir_id: HirId,
accept_comment_above_attributes: bool,
) -> bool {
let span = if accept_comment_above_attributes {
include_attrs_in_span(cx, hir_id, span)
} else {
span
};
span_in_body_has_safety_comment(cx, span)
}
/// Checks if an expression is "branchy", e.g. loop, match/if/etc.
fn is_branchy(expr: &hir::Expr<'_>) -> bool {
matches!(
@ -394,6 +446,15 @@ fn block_has_safety_comment(cx: &LateContext<'_>, span: Span) -> bool {
) || span_in_body_has_safety_comment(cx, span)
}
fn include_attrs_in_span(cx: &LateContext<'_>, hir_id: HirId, span: Span) -> Span {
span.to(cx
.tcx
.hir()
.attrs(hir_id)
.iter()
.fold(span, |acc, attr| acc.to(attr.span)))
}
enum HasSafetyComment {
Yes(BytePos),
No,

View File

@ -542,6 +542,10 @@ pub fn get_configuration_metadata() -> Vec<ClippyConfiguration> {
///
/// Whether to accept a safety comment to be placed above the statement containing the `unsafe` block
(accept_comment_above_statement: bool = false),
/// Lint: UNDOCUMENTED_UNSAFE_BLOCKS.
///
/// Whether to accept a safety comment to be placed above the attributes for the `unsafe` block
(accept_comment_above_attributes: bool = false),
}
/// Search for the configuration file.

View File

@ -1,4 +1,5 @@
error: error reading Clippy's configuration file: unknown field `foobar`, expected one of
accept-comment-above-attributes
accept-comment-above-statement
allow-dbg-in-tests
allow-expect-in-tests
@ -66,6 +67,7 @@ LL | foobar = 42
| ^^^^^^
error: error reading Clippy's configuration file: unknown field `barfoo`, expected one of
accept-comment-above-attributes
accept-comment-above-statement
allow-dbg-in-tests
allow-expect-in-tests

View File

@ -1 +1,2 @@
accept-comment-above-statement = true
accept-comment-above-attributes = true

View File

@ -1,7 +1,8 @@
//@aux-build:proc_macro_unsafe.rs
#![warn(clippy::undocumented_unsafe_blocks, clippy::unnecessary_safety_comment)]
#![allow(clippy::let_unit_value, clippy::missing_safety_doc)]
#![allow(deref_nullptr, clippy::let_unit_value, clippy::missing_safety_doc)]
#![feature(lint_reasons)]
extern crate proc_macro_unsafe;
@ -531,4 +532,36 @@ fn issue_10832() {
unsafe { a_const_function_with_a_very_long_name_to_break_the_line() };
}
fn issue_8679<T: Copy>() {
// SAFETY:
#[allow(unsafe_code)]
unsafe {}
// SAFETY:
#[expect(unsafe_code, reason = "totally safe")]
unsafe {
*std::ptr::null::<T>()
};
// Safety: A safety comment
#[allow(unsafe_code)]
let _some_variable_with_a_very_long_name_to_break_the_line =
unsafe { a_function_with_a_very_long_name_to_break_the_line() };
// Safety: Another safety comment
#[allow(unsafe_code)]
const _SOME_CONST_WITH_A_VERY_LONG_NAME_TO_BREAK_THE_LINE: u32 =
unsafe { a_const_function_with_a_very_long_name_to_break_the_line() };
// Safety: Yet another safety comment
#[allow(unsafe_code)]
static _SOME_STATIC_WITH_A_VERY_LONG_NAME_TO_BREAK_THE_LINE: u32 =
unsafe { a_const_function_with_a_very_long_name_to_break_the_line() };
// SAFETY:
#[allow(unsafe_code)]
// This also works I guess
unsafe {}
}
fn main() {}

View File

@ -1,5 +1,5 @@
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:262:19
--> $DIR/undocumented_unsafe_blocks.rs:263:19
|
LL | /* Safety: */ unsafe {}
| ^^^^^^^^^
@ -8,7 +8,7 @@ LL | /* Safety: */ unsafe {}
= note: `-D clippy::undocumented-unsafe-blocks` implied by `-D warnings`
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:266:5
--> $DIR/undocumented_unsafe_blocks.rs:267:5
|
LL | unsafe {}
| ^^^^^^^^^
@ -16,7 +16,7 @@ LL | unsafe {}
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:270:14
--> $DIR/undocumented_unsafe_blocks.rs:271:14
|
LL | let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }];
| ^^^^^^^^^^^^^
@ -24,7 +24,7 @@ LL | let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }];
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:270:29
--> $DIR/undocumented_unsafe_blocks.rs:271:29
|
LL | let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }];
| ^^^^^^^^^^^^^
@ -32,7 +32,7 @@ LL | let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }];
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:270:48
--> $DIR/undocumented_unsafe_blocks.rs:271:48
|
LL | let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }];
| ^^^^^^^^^^^^^
@ -40,7 +40,7 @@ LL | let _ = [unsafe { 14 }, unsafe { 15 }, 42, unsafe { 16 }];
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:274:18
--> $DIR/undocumented_unsafe_blocks.rs:275:18
|
LL | let _ = (42, unsafe {}, "test", unsafe {});
| ^^^^^^^^^
@ -48,7 +48,7 @@ LL | let _ = (42, unsafe {}, "test", unsafe {});
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:274:37
--> $DIR/undocumented_unsafe_blocks.rs:275:37
|
LL | let _ = (42, unsafe {}, "test", unsafe {});
| ^^^^^^^^^
@ -56,7 +56,7 @@ LL | let _ = (42, unsafe {}, "test", unsafe {});
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:278:14
--> $DIR/undocumented_unsafe_blocks.rs:279:14
|
LL | let _ = *unsafe { &42 };
| ^^^^^^^^^^^^^^
@ -64,7 +64,7 @@ LL | let _ = *unsafe { &42 };
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:283:19
--> $DIR/undocumented_unsafe_blocks.rs:284:19
|
LL | let _ = match unsafe {} {
| ^^^^^^^^^
@ -72,7 +72,7 @@ LL | let _ = match unsafe {} {
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:289:14
--> $DIR/undocumented_unsafe_blocks.rs:290:14
|
LL | let _ = &unsafe {};
| ^^^^^^^^^
@ -80,7 +80,7 @@ LL | let _ = &unsafe {};
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:293:14
--> $DIR/undocumented_unsafe_blocks.rs:294:14
|
LL | let _ = [unsafe {}; 5];
| ^^^^^^^^^
@ -88,7 +88,7 @@ LL | let _ = [unsafe {}; 5];
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:297:13
--> $DIR/undocumented_unsafe_blocks.rs:298:13
|
LL | let _ = unsafe {};
| ^^^^^^^^^
@ -96,7 +96,7 @@ LL | let _ = unsafe {};
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:307:8
--> $DIR/undocumented_unsafe_blocks.rs:308:8
|
LL | t!(unsafe {});
| ^^^^^^^^^
@ -104,7 +104,7 @@ LL | t!(unsafe {});
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:313:13
--> $DIR/undocumented_unsafe_blocks.rs:314:13
|
LL | unsafe {}
| ^^^^^^^^^
@ -116,7 +116,7 @@ LL | t!();
= note: this error originates in the macro `t` (in Nightly builds, run with -Z macro-backtrace for more info)
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:321:5
--> $DIR/undocumented_unsafe_blocks.rs:322:5
|
LL | unsafe {} // SAFETY:
| ^^^^^^^^^
@ -124,7 +124,7 @@ LL | unsafe {} // SAFETY:
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:325:5
--> $DIR/undocumented_unsafe_blocks.rs:326:5
|
LL | unsafe {
| ^^^^^^^^
@ -132,7 +132,7 @@ LL | unsafe {
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:335:5
--> $DIR/undocumented_unsafe_blocks.rs:336:5
|
LL | unsafe {};
| ^^^^^^^^^
@ -140,7 +140,7 @@ LL | unsafe {};
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:339:20
--> $DIR/undocumented_unsafe_blocks.rs:340:20
|
LL | println!("{}", unsafe { String::from_utf8_unchecked(vec![]) });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -148,7 +148,7 @@ LL | println!("{}", unsafe { String::from_utf8_unchecked(vec![]) });
= help: consider adding a safety comment on the preceding line
error: unsafe impl missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:346:5
--> $DIR/undocumented_unsafe_blocks.rs:347:5
|
LL | unsafe impl A for () {}
| ^^^^^^^^^^^^^^^^^^^^^^^
@ -156,7 +156,7 @@ 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:353:9
--> $DIR/undocumented_unsafe_blocks.rs:354:9
|
LL | unsafe impl B for (u32) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -164,7 +164,7 @@ 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:374:13
--> $DIR/undocumented_unsafe_blocks.rs:375:13
|
LL | unsafe impl T for $t {}
| ^^^^^^^^^^^^^^^^^^^^^^^
@ -176,7 +176,7 @@ LL | no_safety_comment!(());
= note: this error originates in the macro `no_safety_comment` (in Nightly builds, run with -Z macro-backtrace for more info)
error: unsafe impl missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:399:13
--> $DIR/undocumented_unsafe_blocks.rs:400:13
|
LL | unsafe impl T for $t {}
| ^^^^^^^^^^^^^^^^^^^^^^^
@ -188,7 +188,7 @@ LL | no_safety_comment!(());
= note: this error originates in the macro `no_safety_comment` (in Nightly builds, run with -Z macro-backtrace for more info)
error: unsafe impl missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:407:5
--> $DIR/undocumented_unsafe_blocks.rs:408:5
|
LL | unsafe impl T for (i32) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -196,7 +196,7 @@ LL | unsafe impl T for (i32) {}
= help: consider adding a safety comment on the preceding line
error: unsafe impl missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:399:13
--> $DIR/undocumented_unsafe_blocks.rs:400:13
|
LL | unsafe impl T for $t {}
| ^^^^^^^^^^^^^^^^^^^^^^^
@ -208,7 +208,7 @@ LL | no_safety_comment!(u32);
= note: this error originates in the macro `no_safety_comment` (in Nightly builds, run with -Z macro-backtrace for more info)
error: unsafe impl missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:413:5
--> $DIR/undocumented_unsafe_blocks.rs:414:5
|
LL | unsafe impl T for (bool) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -216,7 +216,7 @@ LL | unsafe impl T for (bool) {}
= help: consider adding a safety comment on the preceding line
error: unsafe impl missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:459:5
--> $DIR/undocumented_unsafe_blocks.rs:460:5
|
LL | unsafe impl NoComment for () {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -224,7 +224,7 @@ 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:463:19
--> $DIR/undocumented_unsafe_blocks.rs:464:19
|
LL | /* SAFETY: */ unsafe impl InlineComment for () {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -232,7 +232,7 @@ 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:467:5
--> $DIR/undocumented_unsafe_blocks.rs:468:5
|
LL | unsafe impl TrailingComment for () {} // SAFETY:
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -240,20 +240,20 @@ LL | unsafe impl TrailingComment for () {} // SAFETY:
= help: consider adding a safety comment on the preceding line
error: constant item has unnecessary safety comment
--> $DIR/undocumented_unsafe_blocks.rs:471:5
--> $DIR/undocumented_unsafe_blocks.rs:472:5
|
LL | const BIG_NUMBER: i32 = 1000000;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: consider removing the safety comment
--> $DIR/undocumented_unsafe_blocks.rs:470:5
--> $DIR/undocumented_unsafe_blocks.rs:471:5
|
LL | // SAFETY:
| ^^^^^^^^^^
= note: `-D clippy::unnecessary-safety-comment` implied by `-D warnings`
error: unsafe impl missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:472:5
--> $DIR/undocumented_unsafe_blocks.rs:473:5
|
LL | unsafe impl Interference for () {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -261,7 +261,7 @@ LL | unsafe impl Interference for () {}
= help: consider adding a safety comment on the preceding line
error: unsafe impl missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:479:5
--> $DIR/undocumented_unsafe_blocks.rs:480:5
|
LL | unsafe impl ImplInFn for () {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -269,7 +269,7 @@ LL | unsafe impl ImplInFn for () {}
= help: consider adding a safety comment on the preceding line
error: unsafe impl missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:488:1
--> $DIR/undocumented_unsafe_blocks.rs:489:1
|
LL | unsafe impl CrateRoot for () {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -277,7 +277,7 @@ LL | unsafe impl CrateRoot for () {}
= help: consider adding a safety comment on the preceding line
error: statement has unnecessary safety comment
--> $DIR/undocumented_unsafe_blocks.rs:501:5
--> $DIR/undocumented_unsafe_blocks.rs:502:5
|
LL | / let _ = {
LL | | if unsafe { true } {
@ -289,13 +289,13 @@ LL | | };
| |______^
|
help: consider removing the safety comment
--> $DIR/undocumented_unsafe_blocks.rs:500:5
--> $DIR/undocumented_unsafe_blocks.rs:501:5
|
LL | // SAFETY: this is more than one level away, so it should warn
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:502:12
--> $DIR/undocumented_unsafe_blocks.rs:503:12
|
LL | if unsafe { true } {
| ^^^^^^^^^^^^^^^
@ -303,7 +303,7 @@ LL | if unsafe { true } {
= help: consider adding a safety comment on the preceding line
error: unsafe block missing a safety comment
--> $DIR/undocumented_unsafe_blocks.rs:505:23
--> $DIR/undocumented_unsafe_blocks.rs:506:23
|
LL | let bar = unsafe {};
| ^^^^^^^^^