about summary refs log tree commit diff
diff options
context:
space:
mode:
authorSamuel Moelius <sam@moeli.us>2024-11-08 19:40:55 -0500
committerSamuel Moelius <sam@moeli.us>2024-11-15 17:37:08 -0500
commitf159a3eb1dfc3b81088fce364a09e8a79968adda (patch)
tree3881f83e15ebb5af52233dff91c700ac79d33fe4
parent10e29413bc92c2b4d17ab497353604a9015cd705 (diff)
downloadrust-f159a3eb1dfc3b81088fce364a09e8a79968adda.tar.gz
rust-f159a3eb1dfc3b81088fce364a09e8a79968adda.zip
Support replacements in `disallowed_methods`
-rw-r--r--clippy_config/src/types.rs38
-rw-r--r--clippy_lints/src/await_holding_invalid.rs16
-rw-r--r--clippy_lints/src/disallowed_macros.rs13
-rw-r--r--clippy_lints/src/disallowed_methods.rs14
-rw-r--r--tests/ui-toml/toml_replaceable_disallowed_methods/clippy.toml4
-rw-r--r--tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.fixed8
-rw-r--r--tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.rs8
-rw-r--r--tests/ui-toml/toml_replaceable_disallowed_methods/replaceable_disallowed_methods.stderr17
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
+