Support replacements in disallowed_methods

Author: smoelius

clippy_config/src/types.rs

@@ -1,3 +1,5 @@
+use rustc_errors::{Applicability, Diag};
+use rustc_span::Span;
 use serde::de::{self, Deserializer, Visitor};
 use serde::{Deserialize, Serialize, ser};
 use std::collections::HashMap;
@@ -13,7 +15,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 {
@@ -23,12 +29,34 @@ 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,
+        }
+    }
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)]

clippy_lints/src/await_holding_invalid.rs

@@ -1,4 +1,5 @@
 use clippy_config::Conf;
+use clippy_config::types::DisallowedPath;
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::{create_disallowed_map, match_def_path, paths};
 use rustc_hir as hir;
@@ -173,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 {
@@ -246,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),
     );
 }
 

clippy_lints/src/disallowed_macros.rs

@@ -1,9 +1,9 @@
 use clippy_config::Conf;
+use clippy_config::types::DisallowedPath;
 use clippy_utils::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 +60,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 +91,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
                 {

clippy_lints/src/disallowed_methods.rs

@@ -1,4 +1,5 @@
 use clippy_config::Conf;
+use clippy_config::types::DisallowedPath;
 use clippy_utils::create_disallowed_map;
 use clippy_utils::diagnostics::span_lint_and_then;
 use rustc_hir::def::{CtorKind, DefKind, Res};
@@ -31,6 +32,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 +61,7 @@ declare_clippy_lint! {
 }
 
 pub struct DisallowedMethods {
-    disallowed: DefIdMap<(&'static str, Option<&'static str>)>,
+    disallowed: DefIdMap<(&'static str, &'static DisallowedPath)>,
 }
 
 impl DisallowedMethods {
@@ -85,17 +88,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),
             );
         }
     }

clippy_utils/src/lib.rs

@@ -753,11 +753,13 @@ pub fn def_path_def_ids(tcx: TyCtxt<'_>, path: &[&str]) -> impl Iterator<Item =
 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()
 }
 

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" },
+]

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();
+}

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();
+}

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
+