Skip to content

Commit f159a3e

Browse files
committed
Support replacements in disallowed_methods
1 parent 10e2941 commit f159a3e

File tree

8 files changed

+87
-31
lines changed

8 files changed

+87
-31
lines changed

clippy_config/src/types.rs

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use clippy_utils::def_path_def_ids;
2+
use rustc_errors::{Applicability, Diag};
23
use rustc_hir::def_id::DefIdMap;
34
use rustc_middle::ty::TyCtxt;
5+
use rustc_span::Span;
46
use serde::de::{self, Deserializer, Visitor};
57
use serde::{Deserialize, Serialize, ser};
68
use std::collections::HashMap;
@@ -16,7 +18,11 @@ pub struct Rename {
1618
#[serde(untagged)]
1719
pub enum DisallowedPath {
1820
Simple(String),
19-
WithReason { path: String, reason: Option<String> },
21+
WithReason {
22+
path: String,
23+
reason: Option<String>,
24+
replacement: Option<String>,
25+
},
2026
}
2127

2228
impl DisallowedPath {
@@ -26,23 +32,47 @@ impl DisallowedPath {
2632
path
2733
}
2834

35+
pub fn diag_amendment(&self, span: Span) -> impl FnOnce(&mut Diag<'_, ()>) + use<'_> {
36+
move |diag| {
37+
if let Some(replacement) = self.replacement() {
38+
diag.span_suggestion(
39+
span,
40+
self.reason().map_or_else(|| String::from("use"), ToOwned::to_owned),
41+
replacement,
42+
Applicability::MachineApplicable,
43+
);
44+
} else if let Some(reason) = self.reason() {
45+
diag.note(reason.to_owned());
46+
}
47+
}
48+
}
49+
2950
pub fn reason(&self) -> Option<&str> {
3051
match &self {
3152
Self::WithReason { reason, .. } => reason.as_deref(),
3253
Self::Simple(_) => None,
3354
}
3455
}
56+
57+
fn replacement(&self) -> Option<&str> {
58+
match &self {
59+
Self::WithReason { replacement, .. } => replacement.as_deref(),
60+
Self::Simple(_) => None,
61+
}
62+
}
3563
}
3664

3765
/// Creates a map of disallowed items to the reason they were disallowed.
3866
pub fn create_disallowed_map(
3967
tcx: TyCtxt<'_>,
4068
disallowed: &'static [DisallowedPath],
41-
) -> DefIdMap<(&'static str, Option<&'static str>)> {
69+
) -> DefIdMap<(&'static str, &'static DisallowedPath)> {
4270
disallowed
4371
.iter()
44-
.map(|x| (x.path(), x.path().split("::").collect::<Vec<_>>(), x.reason()))
45-
.flat_map(|(name, path, reason)| def_path_def_ids(tcx, &path).map(move |id| (id, (name, reason))))
72+
.map(|x| (x.path(), x.path().split("::").collect::<Vec<_>>(), x))
73+
.flat_map(|(name, path, disallowed_path)| {
74+
def_path_def_ids(tcx, &path).map(move |id| (id, (name, disallowed_path)))
75+
})
4676
.collect()
4777
}
4878

clippy_lints/src/await_holding_invalid.rs

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_config::Conf;
2-
use clippy_config::types::create_disallowed_map;
2+
use clippy_config::types::{DisallowedPath, create_disallowed_map};
33
use clippy_utils::diagnostics::span_lint_and_then;
44
use clippy_utils::{match_def_path, paths};
55
use rustc_hir as hir;
@@ -174,7 +174,7 @@ declare_clippy_lint! {
174174
impl_lint_pass!(AwaitHolding => [AWAIT_HOLDING_LOCK, AWAIT_HOLDING_REFCELL_REF, AWAIT_HOLDING_INVALID_TYPE]);
175175

176176
pub struct AwaitHolding {
177-
def_ids: DefIdMap<(&'static str, Option<&'static str>)>,
177+
def_ids: DefIdMap<(&'static str, &'static DisallowedPath)>,
178178
}
179179

180180
impl AwaitHolding {
@@ -247,25 +247,21 @@ impl AwaitHolding {
247247
);
248248
},
249249
);
250-
} else if let Some(&(path, reason)) = self.def_ids.get(&adt.did()) {
251-
emit_invalid_type(cx, ty_cause.source_info.span, path, reason);
250+
} else if let Some(&(path, disallowed_path)) = self.def_ids.get(&adt.did()) {
251+
emit_invalid_type(cx, ty_cause.source_info.span, path, disallowed_path);
252252
}
253253
}
254254
}
255255
}
256256
}
257257

258-
fn emit_invalid_type(cx: &LateContext<'_>, span: Span, path: &'static str, reason: Option<&'static str>) {
258+
fn emit_invalid_type(cx: &LateContext<'_>, span: Span, path: &'static str, disallowed_path: &'static DisallowedPath) {
259259
span_lint_and_then(
260260
cx,
261261
AWAIT_HOLDING_INVALID_TYPE,
262262
span,
263263
format!("holding a disallowed type across an await point `{path}`"),
264-
|diag| {
265-
if let Some(reason) = reason {
266-
diag.note(reason);
267-
}
268-
},
264+
disallowed_path.diag_amendment(span),
269265
);
270266
}
271267

clippy_lints/src/disallowed_macros.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
use clippy_config::Conf;
2-
use clippy_config::types::create_disallowed_map;
2+
use clippy_config::types::{DisallowedPath, create_disallowed_map};
33
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
44
use clippy_utils::macros::macro_backtrace;
55
use rustc_data_structures::fx::FxHashSet;
6-
use rustc_errors::Diag;
76
use rustc_hir::def_id::DefIdMap;
87
use rustc_hir::{
98
Expr, ExprKind, ForeignItem, HirId, ImplItem, Item, ItemKind, OwnerId, Pat, Path, Stmt, TraitItem, Ty,
@@ -60,7 +59,7 @@ declare_clippy_lint! {
6059
}
6160

6261
pub struct DisallowedMacros {
63-
disallowed: DefIdMap<(&'static str, Option<&'static str>)>,
62+
disallowed: DefIdMap<(&'static str, &'static DisallowedPath)>,
6463
seen: FxHashSet<ExpnId>,
6564
// Track the most recently seen node that can have a `derive` attribute.
6665
// Needed to use the correct lint level.
@@ -91,13 +90,9 @@ impl DisallowedMacros {
9190
return;
9291
}
9392

94-
if let Some(&(path, reason)) = self.disallowed.get(&mac.def_id) {
93+
if let Some(&(path, disallowed_path)) = self.disallowed.get(&mac.def_id) {
9594
let msg = format!("use of a disallowed macro `{path}`");
96-
let add_note = |diag: &mut Diag<'_, _>| {
97-
if let Some(reason) = reason {
98-
diag.note(reason);
99-
}
100-
};
95+
let add_note = disallowed_path.diag_amendment(mac.span);
10196
if matches!(mac.kind, MacroKind::Derive)
10297
&& let Some(derive_src) = derive_src
10398
{

clippy_lints/src/disallowed_methods.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_config::Conf;
2-
use clippy_config::types::create_disallowed_map;
2+
use clippy_config::types::{DisallowedPath, create_disallowed_map};
33
use clippy_utils::diagnostics::span_lint_and_then;
44
use rustc_hir::def::{CtorKind, DefKind, Res};
55
use rustc_hir::def_id::DefIdMap;
@@ -31,6 +31,8 @@ declare_clippy_lint! {
3131
/// # When using an inline table, can add a `reason` for why the method
3232
/// # is disallowed.
3333
/// { path = "std::vec::Vec::leak", reason = "no leaking memory" },
34+
/// # Can also add a `replacement` that will be offered as a suggestion.
35+
/// { path = "std::sync::Mutex::new", reason = "prefer faster & simpler non-poisonable mutex", replacement = "parking_lot::Mutex::new" },
3436
/// ]
3537
/// ```
3638
///
@@ -58,7 +60,7 @@ declare_clippy_lint! {
5860
}
5961

6062
pub struct DisallowedMethods {
61-
disallowed: DefIdMap<(&'static str, Option<&'static str>)>,
63+
disallowed: DefIdMap<(&'static str, &'static DisallowedPath)>,
6264
}
6365

6466
impl DisallowedMethods {
@@ -85,17 +87,13 @@ impl<'tcx> LateLintPass<'tcx> for DisallowedMethods {
8587
},
8688
_ => return,
8789
};
88-
if let Some(&(path, reason)) = self.disallowed.get(&id) {
90+
if let Some(&(path, disallowed_path)) = self.disallowed.get(&id) {
8991
span_lint_and_then(
9092
cx,
9193
DISALLOWED_METHODS,
9294
span,
9395
format!("use of a disallowed method `{path}`"),
94-
|diag| {
95-
if let Some(reason) = reason {
96-
diag.note(reason);
97-
}
98-
},
96+
disallowed_path.diag_amendment(span),
9997
);
10098
}
10199
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
disallowed-methods = [
2+
{ path = "replaceable_disallowed_methods::bad", replacement = "good" },
3+
{ path = "replaceable_disallowed_methods::questionable", replacement = "good", reason = "a better function exists" },
4+
]
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
fn bad() {}
2+
fn questionable() {}
3+
fn good() {}
4+
5+
fn main() {
6+
good();
7+
good();
8+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
fn bad() {}
2+
fn questionable() {}
3+
fn good() {}
4+
5+
fn main() {
6+
bad();
7+
questionable();
8+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
error: use of a disallowed method `replaceable_disallowed_methods::bad`
2+
--> tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.rs:6:5
3+
|
4+
LL | bad();
5+
| ^^^ help: use: `good`
6+
|
7+
= note: `-D clippy::disallowed-methods` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::disallowed_methods)]`
9+
10+
error: use of a disallowed method `replaceable_disallowed_methods::questionable`
11+
--> tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.rs:7:5
12+
|
13+
LL | questionable();
14+
| ^^^^^^^^^^^^ help: a better function exists: `good`
15+
16+
error: aborting due to 2 previous errors
17+

0 commit comments

Comments
 (0)