diff options
| author | Samuel Moelius <sam@moeli.us> | 2024-11-08 19:40:55 -0500 |
|---|---|---|
| committer | Samuel Moelius <sam@moeli.us> | 2024-11-15 17:37:08 -0500 |
| commit | f159a3eb1dfc3b81088fce364a09e8a79968adda (patch) | |
| tree | 3881f83e15ebb5af52233dff91c700ac79d33fe4 | |
| parent | 10e29413bc92c2b4d17ab497353604a9015cd705 (diff) | |
| download | rust-f159a3eb1dfc3b81088fce364a09e8a79968adda.tar.gz rust-f159a3eb1dfc3b81088fce364a09e8a79968adda.zip | |
Support replacements in `disallowed_methods`
8 files changed, 87 insertions, 31 deletions
diff --git a/clippy_config/src/types.rs b/clippy_config/src/types.rs index c949db9109d..70d9073b59c 100644 --- a/clippy_config/src/types.rs +++ b/clippy_config/src/types.rs @@ -1,6 +1,8 @@ use clippy_utils::def_path_def_ids; +use rustc_errors::{Applicability, Diag}; use rustc_hir::def_id::DefIdMap; use rustc_middle::ty::TyCtxt; +use rustc_span::Span; use serde::de::{self, Deserializer, Visitor}; use serde::{Deserialize, Serialize, ser}; use std::collections::HashMap; @@ -16,7 +18,11 @@ pub struct Rename { #[serde(untagged)] pub enum DisallowedPath { Simple(String), - WithReason { path: String, reason: Option<String> }, + WithReason { + path: String, + reason: Option<String>, + replacement: Option<String>, + }, } impl DisallowedPath { @@ -26,23 +32,47 @@ impl DisallowedPath { path } + pub fn diag_amendment(&self, span: Span) -> impl FnOnce(&mut Diag<'_, ()>) + use<'_> { + move |diag| { + if let Some(replacement) = self.replacement() { + diag.span_suggestion( + span, + self.reason().map_or_else(|| String::from("use"), ToOwned::to_owned), + replacement, + Applicability::MachineApplicable, + ); + } else if let Some(reason) = self.reason() { + diag.note(reason.to_owned()); + } + } + } + pub fn reason(&self) -> Option<&str> { match &self { Self::WithReason { reason, .. } => reason.as_deref(), Self::Simple(_) => None, } } + + fn replacement(&self) -> Option<&str> { + match &self { + Self::WithReason { replacement, .. } => replacement.as_deref(), + Self::Simple(_) => None, + } + } } /// Creates a map of disallowed items to the reason they were disallowed. pub fn create_disallowed_map( tcx: TyCtxt<'_>, disallowed: &'static [DisallowedPath], -) -> DefIdMap<(&'static str, Option<&'static str>)> { +) -> DefIdMap<(&'static str, &'static DisallowedPath)> { disallowed .iter() - .map(|x| (x.path(), x.path().split("::").collect::<Vec<_>>(), x.reason())) - .flat_map(|(name, path, reason)| def_path_def_ids(tcx, &path).map(move |id| (id, (name, reason)))) + .map(|x| (x.path(), x.path().split("::").collect::<Vec<_>>(), x)) + .flat_map(|(name, path, disallowed_path)| { + def_path_def_ids(tcx, &path).map(move |id| (id, (name, disallowed_path))) + }) .collect() } diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs index 2eb0566bf9a..3ae7ab96915 100644 --- a/clippy_lints/src/await_holding_invalid.rs +++ b/clippy_lints/src/await_holding_invalid.rs @@ -1,5 +1,5 @@ use clippy_config::Conf; -use clippy_config::types::create_disallowed_map; +use clippy_config::types::{DisallowedPath, create_disallowed_map}; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::{match_def_path, paths}; use rustc_hir as hir; @@ -174,7 +174,7 @@ declare_clippy_lint! { impl_lint_pass!(AwaitHolding => [AWAIT_HOLDING_LOCK, AWAIT_HOLDING_REFCELL_REF, AWAIT_HOLDING_INVALID_TYPE]); pub struct AwaitHolding { - def_ids: DefIdMap<(&'static str, Option<&'static str>)>, + def_ids: DefIdMap<(&'static str, &'static DisallowedPath)>, } impl AwaitHolding { @@ -247,25 +247,21 @@ impl AwaitHolding { ); }, ); - } else if let Some(&(path, reason)) = self.def_ids.get(&adt.did()) { - emit_invalid_type(cx, ty_cause.source_info.span, path, reason); + } else if let Some(&(path, disallowed_path)) = self.def_ids.get(&adt.did()) { + emit_invalid_type(cx, ty_cause.source_info.span, path, disallowed_path); } } } } } -fn emit_invalid_type(cx: &LateContext<'_>, span: Span, path: &'static str, reason: Option<&'static str>) { +fn emit_invalid_type(cx: &LateContext<'_>, span: Span, path: &'static str, disallowed_path: &'static DisallowedPath) { span_lint_and_then( cx, AWAIT_HOLDING_INVALID_TYPE, span, format!("holding a disallowed type across an await point `{path}`"), - |diag| { - if let Some(reason) = reason { - diag.note(reason); - } - }, + disallowed_path.diag_amendment(span), ); } diff --git a/clippy_lints/src/disallowed_macros.rs b/clippy_lints/src/disallowed_macros.rs index a0cb36f88dc..fe0870e8282 100644 --- a/clippy_lints/src/disallowed_macros.rs +++ b/clippy_lints/src/disallowed_macros.rs @@ -1,9 +1,8 @@ use clippy_config::Conf; -use clippy_config::types::create_disallowed_map; +use clippy_config::types::{DisallowedPath, create_disallowed_map}; use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then}; use clippy_utils::macros::macro_backtrace; use rustc_data_structures::fx::FxHashSet; -use rustc_errors::Diag; use rustc_hir::def_id::DefIdMap; use rustc_hir::{ Expr, ExprKind, ForeignItem, HirId, ImplItem, Item, ItemKind, OwnerId, Pat, Path, Stmt, TraitItem, Ty, @@ -60,7 +59,7 @@ declare_clippy_lint! { } pub struct DisallowedMacros { - disallowed: DefIdMap<(&'static str, Option<&'static str>)>, + disallowed: DefIdMap<(&'static str, &'static DisallowedPath)>, seen: FxHashSet<ExpnId>, // Track the most recently seen node that can have a `derive` attribute. // Needed to use the correct lint level. @@ -91,13 +90,9 @@ impl DisallowedMacros { return; } - if let Some(&(path, reason)) = self.disallowed.get(&mac.def_id) { + if let Some(&(path, disallowed_path)) = self.disallowed.get(&mac.def_id) { let msg = format!("use of a disallowed macro `{path}`"); - let add_note = |diag: &mut Diag<'_, _>| { - if let Some(reason) = reason { - diag.note(reason); - } - }; + let add_note = disallowed_path.diag_amendment(mac.span); if matches!(mac.kind, MacroKind::Derive) && let Some(derive_src) = derive_src { diff --git a/clippy_lints/src/disallowed_methods.rs b/clippy_lints/src/disallowed_methods.rs index 1e660b1957a..0cd9ea13f2e 100644 --- a/clippy_lints/src/disallowed_methods.rs +++ b/clippy_lints/src/disallowed_methods.rs @@ -1,5 +1,5 @@ use clippy_config::Conf; -use clippy_config::types::create_disallowed_map; +use clippy_config::types::{DisallowedPath, create_disallowed_map}; use clippy_utils::diagnostics::span_lint_and_then; use rustc_hir::def::{CtorKind, DefKind, Res}; use rustc_hir::def_id::DefIdMap; @@ -31,6 +31,8 @@ declare_clippy_lint! { /// # When using an inline table, can add a `reason` for why the method /// # is disallowed. /// { path = "std::vec::Vec::leak", reason = "no leaking memory" }, + /// # Can also add a `replacement` that will be offered as a suggestion. + /// { path = "std::sync::Mutex::new", reason = "prefer faster & simpler non-poisonable mutex", replacement = "parking_lot::Mutex::new" }, /// ] /// ``` /// @@ -58,7 +60,7 @@ declare_clippy_lint! { } pub struct DisallowedMethods { - disallowed: DefIdMap<(&'static str, Option<&'static str>)>, + disallowed: DefIdMap<(&'static str, &'static DisallowedPath)>, } impl DisallowedMethods { @@ -85,17 +87,13 @@ impl<'tcx> LateLintPass<'tcx> for DisallowedMethods { }, _ => return, }; - if let Some(&(path, reason)) = self.disallowed.get(&id) { + if let Some(&(path, disallowed_path)) = self.disallowed.get(&id) { span_lint_and_then( cx, DISALLOWED_METHODS, span, format!("use of a disallowed method `{path}`"), - |diag| { - if let Some(reason) = reason { - diag.note(reason); - } - }, + disallowed_path.diag_amendment(span), ); } } diff --git a/tests/ui-toml/toml_replaceable_disallowed_methods/clippy.toml b/tests/ui-toml/toml_replaceable_disallowed_methods/clippy.toml new file mode 100644 index 00000000000..dc393f1355b --- /dev/null +++ b/tests/ui-toml/toml_replaceable_disallowed_methods/clippy.toml @@ -0,0 +1,4 @@ +disallowed-methods = [ + { path = "replaceable_disallowed_methods::bad", replacement = "good" }, + { path = "replaceable_disallowed_methods::questionable", replacement = "good", reason = "a better function exists" }, +] diff --git a/tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.fixed b/tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.fixed new file mode 100644 index 00000000000..dae7ce76ba2 --- /dev/null +++ b/tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.fixed @@ -0,0 +1,8 @@ +fn bad() {} +fn questionable() {} +fn good() {} + +fn main() { + good(); + good(); +} diff --git a/tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.rs b/tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.rs new file mode 100644 index 00000000000..53678ffdf1c --- /dev/null +++ b/tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.rs @@ -0,0 +1,8 @@ +fn bad() {} +fn questionable() {} +fn good() {} + +fn main() { + bad(); + questionable(); +} diff --git a/tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.stderr b/tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.stderr new file mode 100644 index 00000000000..b8559202942 --- /dev/null +++ b/tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.stderr @@ -0,0 +1,17 @@ +error: use of a disallowed method `replaceable_disallowed_methods::bad` + --> tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.rs:6:5 + | +LL | bad(); + | ^^^ help: use: `good` + | + = note: `-D clippy::disallowed-methods` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::disallowed_methods)]` + +error: use of a disallowed method `replaceable_disallowed_methods::questionable` + --> tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.rs:7:5 + | +LL | questionable(); + | ^^^^^^^^^^^^ help: a better function exists: `good` + +error: aborting due to 2 previous errors + |
