Skip to content

Commit a001246

Browse files
RadbugletRadbuglet
committed
Improve lint diagnostics
1 parent 67afb0c commit a001246

File tree

3 files changed

+174
-37
lines changed

3 files changed

+174
-37
lines changed

‎clippy_lints/src/danger_not_accepted.rs

Lines changed: 113 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,79 @@
1-
use clippy_utils::diagnostics::span_lint_and_help;
1+
use std::fmt;
2+
3+
use clippy_utils::diagnostics::span_lint_and_then;
24
use clippy_utils::get_attr;
35
use rustc_ast::{ast, token, tokenstream};
4-
use rustc_data_structures::fx::{FxHashMap, StdEntry};
6+
use rustc_data_structures::fx::{FxHashMap, FxHashSet, StdEntry};
57
use rustc_hir::*;
68
use rustc_lint::{LateContext, LateLintPass, LintContext};
79
use rustc_session::{declare_tool_lint, impl_lint_pass};
8-
use rustc_span::Symbol;
10+
use rustc_span::{Span, Symbol};
911

10-
// TODO: Ensure that our attributes are being used properly
11-
// TODO: Improve lint messages
12+
// TODO: Safety override
13+
// TODO: Config
1214

1315
declare_clippy_lint! {
1416
/// ### What it does
1517
///
18+
/// Checks for uses of functions, inherent methods, and trait methods which have been marked as
19+
/// dangerous with the `#[clippy::dangerous(...)]` attribute.
20+
///
21+
/// Each `#[clippy::dangerous(reason_1, reason_2, ...)]` attribute specifies a list of dangers
22+
/// that the user must accept using the `#[clippy::accept_danger(reason_1, reason_2, ...)]`
23+
/// attribute before using the dangerous item.
24+
///
1625
/// ### Why is this bad?
1726
///
27+
/// Some functionality in a project may be dangerous to use without giving it the appropriate
28+
/// caution, even if its misuse does not cause undefined behavior—for example, the method could
29+
/// be the source of tricky logic bugs. Other functionality may be dangerous in some contexts
30+
/// but not others. This lint helps ensure that users do not unknowingly call into these
31+
/// dangerous functions while still allowing users who know what they're doing to call these
32+
/// functions without issue.
33+
///
1834
/// ### Example
1935
/// ```rust
20-
/// // example code where clippy issues a warning
36+
/// #[clippy::dangerous(use_of_lib_1_dangerous_module)]
37+
/// mod dangerous_module {
38+
/// # fn break_the_program() {}
39+
/// #[clippy::dangerous(may_break_program)]
40+
/// pub fn do_something_innocuous_looking() {
41+
/// break_the_program();
42+
/// }
43+
/// }
44+
///
45+
/// use unsuspecting_module {
46+
/// fn do_something() {
47+
/// // This method call causes clippy to issue a warning
48+
/// dangerous_module::do_something_innocuous_looking();
49+
/// }
50+
/// }
2151
/// ```
2252
/// Use instead:
2353
/// ```rust
24-
/// // example code which does not raise clippy warning
54+
/// #[clippy::dangerous(use_of_lib_1_dangerous_module)]
55+
/// mod dangerous_module {
56+
/// # fn break_the_program() {}
57+
/// #[clippy::dangerous(may_break_program)]
58+
/// pub fn do_something_innocuous_looking() {
59+
/// break_the_program();
60+
/// }
61+
/// }
62+
///
63+
/// // This entire module can use functions with the danger `use_of_lib_1_dangerous_module`.
64+
/// #[clippy::accept_danger(use_of_lib_1_dangerous_module)]
65+
/// use unsuspecting_module {
66+
/// fn do_something() {
67+
/// // Only this statement can call functions with the danger `may_break_program`.
68+
/// #[clippy::accept_danger(may_break_program)]
69+
/// dangerous_module::do_something_innocuous_looking();
70+
/// }
71+
/// }
2572
/// ```
2673
#[clippy::version = "1.74.0"]
2774
pub DANGER_NOT_ACCEPTED,
2875
nursery,
29-
"default lint description"
76+
"checks for use of functions marked as dangerous"
3077
}
3178

3279
#[derive(Default)]
@@ -40,7 +87,7 @@ impl LateLintPass<'_> for DangerNotAccepted {
4087
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) {
4188
// If we're calling a method...
4289
if let ExprKind::MethodCall(_path, _, _self_arg, ..) = &expr.kind
43-
&& let Some(fn_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
90+
&& let Some(fn_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
4491
// And that function is dangerous to us...
4592
&& let Some(dangers) = self.get_unaccepted_dangers(cx, fn_id)
4693
{
@@ -67,7 +114,7 @@ impl LateLintPass<'_> for DangerNotAccepted {
67114
for attr_name in ["accept_danger", "dangerous"] {
68115
for attr in get_attr(cx.sess(), attrs, attr_name) {
69116
let dangers = parse_dangers_attr(cx, attr);
70-
for danger in dangers {
117+
for (_span, danger) in dangers {
71118
*self.accepted_dangers.entry(danger).or_default() += 1;
72119
}
73120
}
@@ -79,7 +126,7 @@ impl LateLintPass<'_> for DangerNotAccepted {
79126
for attr_name in ["accept_danger", "dangerous"] {
80127
for attr in get_attr(cx.sess(), attrs, attr_name) {
81128
let dangers = parse_dangers_attr(cx, attr);
82-
for danger in dangers {
129+
for (_span, danger) in dangers {
83130
match self.accepted_dangers.entry(danger) {
84131
StdEntry::Occupied(mut entry) => {
85132
*entry.get_mut() -= 1;
@@ -96,7 +143,7 @@ impl LateLintPass<'_> for DangerNotAccepted {
96143
}
97144

98145
impl DangerNotAccepted {
99-
fn get_unaccepted_dangers(&self, cx: &LateContext<'_>, item_id: def_id::DefId) -> Option<Vec<Symbol>> {
146+
fn get_unaccepted_dangers(&self, cx: &LateContext<'_>, item_id: def_id::DefId) -> Option<Vec<(Span, Symbol)>> {
100147
let mut unaccepted_dangers = Vec::new();
101148
let mut item_iter = Some(item_id);
102149

@@ -111,7 +158,7 @@ impl DangerNotAccepted {
111158

112159
for attr in get_attr(cx.sess(), cx.tcx.get_attrs_unchecked(item_id), "dangerous") {
113160
for danger in parse_dangers_attr(cx, attr) {
114-
if self.accepted_dangers.contains_key(&danger) {
161+
if self.accepted_dangers.contains_key(&danger.1) {
115162
continue;
116163
}
117164

@@ -124,18 +171,7 @@ impl DangerNotAccepted {
124171
}
125172
}
126173

127-
fn emit_dangerous_call_lint(cx: &LateContext<'_>, expr: &'_ Expr<'_>, dangers: &[Symbol]) {
128-
span_lint_and_help(
129-
cx,
130-
DANGER_NOT_ACCEPTED,
131-
expr.span,
132-
"Called a method marked with `#[clippy::dangerous]` without blessing the calling module with `#![clippy::accept_danger]`.",
133-
None,
134-
format!("This method poses the following unaccepted dangers: {dangers:?}").as_str(),
135-
);
136-
}
137-
138-
fn parse_dangers_attr(cx: &LateContext<'_>, attr: &ast::Attribute) -> Vec<Symbol> {
174+
fn parse_dangers_attr(cx: &LateContext<'_>, attr: &ast::Attribute) -> Vec<(Span, Symbol)> {
139175
const EXPECTATION: &str = "Expected a delimited attribute with a list of danger identifiers.";
140176

141177
let span = attr.span;
@@ -173,7 +209,7 @@ fn parse_dangers_attr(cx: &LateContext<'_>, attr: &ast::Attribute) -> Vec<Symbol
173209
return Vec::new();
174210
};
175211

176-
dangers.push(sym.name);
212+
dangers.push((sym.span, sym.name));
177213

178214
match stream.next() {
179215
Some(tokenstream::TokenTree::Token(sym, _)) if sym.kind == token::TokenKind::Comma => {
@@ -191,3 +227,54 @@ fn parse_dangers_attr(cx: &LateContext<'_>, attr: &ast::Attribute) -> Vec<Symbol
191227

192228
dangers
193229
}
230+
231+
fn emit_dangerous_call_lint(cx: &LateContext<'_>, expr: &'_ Expr<'_>, unaccepted_dangers: &[(Span, Symbol)]) {
232+
// Define formatting helpers
233+
struct FmtInline<F>(F);
234+
235+
impl<F> fmt::Display for FmtInline<F>
236+
where
237+
F: Fn(&mut fmt::Formatter<'_>) -> fmt::Result,
238+
{
239+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
240+
self.0(f)
241+
}
242+
}
243+
244+
fn fmt_inline<F>(f: F) -> FmtInline<F>
245+
where
246+
F: Fn(&mut fmt::Formatter<'_>) -> fmt::Result,
247+
{
248+
FmtInline(f)
249+
}
250+
251+
// Collect all unique dangers
252+
let unique_dangers = unaccepted_dangers.iter().map(|(_, sym)| sym).collect::<FxHashSet<_>>();
253+
254+
// Create a lint
255+
span_lint_and_then(
256+
cx,
257+
DANGER_NOT_ACCEPTED,
258+
expr.span,
259+
&format!(
260+
"Called a method marked with `#[clippy::dangerous(...)]` without blessing the calling \
261+
module with `#![clippy::accept_danger({})]`.",
262+
fmt_inline(|f| {
263+
let mut is_subsequent = false;
264+
for danger in &unique_dangers {
265+
if is_subsequent {
266+
f.write_str(", ")?;
267+
}
268+
is_subsequent = true;
269+
f.write_str(danger.as_str())?;
270+
}
271+
Ok(())
272+
}),
273+
),
274+
|diag| {
275+
for (danger_span, danger_name) in unaccepted_dangers {
276+
diag.span_note(*danger_span, format!("Danger `{danger_name}` declared here."));
277+
}
278+
},
279+
);
280+
}

‎tests/ui/danger_not_accepted.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ fn main() {
2222

2323
#[clippy::accept_danger(may_deadlock, may_delete_system)]
2424
Maz.faz2();
25+
26+
waz::woo2();
27+
28+
#[clippy::accept_danger(may_deadlock)]
29+
waz::woo2();
2530
}
2631

2732
fn wee() {}
@@ -32,6 +37,9 @@ struct Maz;
3237
mod waz {
3338
pub fn woo() {}
3439

40+
#[clippy::dangerous(may_deadlock)]
41+
pub fn woo2() {}
42+
3543
impl super::Maz {
3644
#[clippy::dangerous(may_delete_system)]
3745
pub fn faz(&self) {}

‎tests/ui/danger_not_accepted.stderr

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,86 @@
1-
error: Called a method marked with `#[clippy::dangerous]` without blessing the calling module with `#![clippy::accept_danger]`.
1+
error: Called a method marked with `#[clippy::dangerous(...)]` without blessing the calling module with `#![clippy::accept_danger(may_deadlock)]`.
22
--> $DIR/danger_not_accepted.rs:5:5
33
|
44
LL | waz::woo();
55
| ^^^^^^^^
66
|
7-
= help: This method poses the following unaccepted dangers: ["may_deadlock"]
7+
note: Danger `may_deadlock` declared here.
8+
--> $DIR/danger_not_accepted.rs:36:21
9+
|
10+
LL | #[clippy::dangerous(may_deadlock)]
11+
| ^^^^^^^^^^^^
812
= note: `-D clippy::danger-not-accepted` implied by `-D warnings`
913
= help: to override `-D warnings` add `#[allow(clippy::danger_not_accepted)]`
1014

11-
error: Called a method marked with `#[clippy::dangerous]` without blessing the calling module with `#![clippy::accept_danger]`.
15+
error: Called a method marked with `#[clippy::dangerous(...)]` without blessing the calling module with `#![clippy::accept_danger(may_delete_system, may_deadlock)]`.
1216
--> $DIR/danger_not_accepted.rs:10:5
1317
|
1418
LL | Maz.faz();
1519
| ^^^^^^^^^
1620
|
17-
= help: This method poses the following unaccepted dangers: ["may_delete_system", "may_deadlock"]
21+
note: Danger `may_delete_system` declared here.
22+
--> $DIR/danger_not_accepted.rs:44:29
23+
|
24+
LL | #[clippy::dangerous(may_delete_system)]
25+
| ^^^^^^^^^^^^^^^^^
26+
note: Danger `may_deadlock` declared here.
27+
--> $DIR/danger_not_accepted.rs:36:21
28+
|
29+
LL | #[clippy::dangerous(may_deadlock)]
30+
| ^^^^^^^^^^^^
1831

19-
error: Called a method marked with `#[clippy::dangerous]` without blessing the calling module with `#![clippy::accept_danger]`.
32+
error: Called a method marked with `#[clippy::dangerous(...)]` without blessing the calling module with `#![clippy::accept_danger(may_delete_system)]`.
2033
--> $DIR/danger_not_accepted.rs:13:5
2134
|
2235
LL | Maz.faz();
2336
| ^^^^^^^^^
2437
|
25-
= help: This method poses the following unaccepted dangers: ["may_delete_system"]
38+
note: Danger `may_delete_system` declared here.
39+
--> $DIR/danger_not_accepted.rs:44:29
40+
|
41+
LL | #[clippy::dangerous(may_delete_system)]
42+
| ^^^^^^^^^^^^^^^^^
2643

27-
error: Called a method marked with `#[clippy::dangerous]` without blessing the calling module with `#![clippy::accept_danger]`.
44+
error: Called a method marked with `#[clippy::dangerous(...)]` without blessing the calling module with `#![clippy::accept_danger(may_delete_system)]`.
2845
--> $DIR/danger_not_accepted.rs:18:5
2946
|
3047
LL | Maz.faz2();
3148
| ^^^^^^^^^^
3249
|
33-
= help: This method poses the following unaccepted dangers: ["may_delete_system"]
50+
note: Danger `may_delete_system` declared here.
51+
--> $DIR/danger_not_accepted.rs:54:25
52+
|
53+
LL | #[clippy::dangerous(may_delete_system)]
54+
| ^^^^^^^^^^^^^^^^^
3455

35-
error: Called a method marked with `#[clippy::dangerous]` without blessing the calling module with `#![clippy::accept_danger]`.
56+
error: Called a method marked with `#[clippy::dangerous(...)]` without blessing the calling module with `#![clippy::accept_danger(may_delete_system)]`.
3657
--> $DIR/danger_not_accepted.rs:21:5
3758
|
3859
LL | Maz.faz2();
3960
| ^^^^^^^^^^
4061
|
41-
= help: This method poses the following unaccepted dangers: ["may_delete_system"]
62+
note: Danger `may_delete_system` declared here.
63+
--> $DIR/danger_not_accepted.rs:54:25
64+
|
65+
LL | #[clippy::dangerous(may_delete_system)]
66+
| ^^^^^^^^^^^^^^^^^
67+
68+
error: Called a method marked with `#[clippy::dangerous(...)]` without blessing the calling module with `#![clippy::accept_danger(may_deadlock)]`.
69+
--> $DIR/danger_not_accepted.rs:26:5
70+
|
71+
LL | waz::woo2();
72+
| ^^^^^^^^^
73+
|
74+
note: Danger `may_deadlock` declared here.
75+
--> $DIR/danger_not_accepted.rs:40:25
76+
|
77+
LL | #[clippy::dangerous(may_deadlock)]
78+
| ^^^^^^^^^^^^
79+
note: Danger `may_deadlock` declared here.
80+
--> $DIR/danger_not_accepted.rs:36:21
81+
|
82+
LL | #[clippy::dangerous(may_deadlock)]
83+
| ^^^^^^^^^^^^
4284

43-
error: aborting due to 5 previous errors
85+
error: aborting due to 6 previous errors
4486

0 commit comments

Comments
 (0)