Two improvements to disallowed_* (#13669)

The improvements are as follows:
- Remove an [unnecessary `compile-flags`
directive](https://github.com/rust-lang/rust-clippy/blob/f712eb5cdccd121d0569af12f20e6a0fabe4364d/tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs#L1)
in `tests/ui-toml/toml_disallowed_methods/conf_disallowed_methods.rs`
- Support replacements as suggested by @mitsuhiko in
#7609 (comment)

---

changelog: support replacements in `disallowed_methods`

Author: Centri3

Diff for: clippy_config/src/conf.rs

@@ -1,8 +1,8 @@
 use crate::ClippyConfiguration;
 use crate::types::{
-    DisallowedPath, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour, Rename, SourceItemOrdering,
-    SourceItemOrderingCategory, SourceItemOrderingModuleItemGroupings, SourceItemOrderingModuleItemKind,
-    SourceItemOrderingTraitAssocItemKind, SourceItemOrderingTraitAssocItemKinds,
+    DisallowedPath, DisallowedPathWithoutReplacement, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour,
+    Rename, SourceItemOrdering, SourceItemOrderingCategory, SourceItemOrderingModuleItemGroupings,
+    SourceItemOrderingModuleItemKind, SourceItemOrderingTraitAssocItemKind, SourceItemOrderingTraitAssocItemKinds,
 };
 use clippy_utils::msrvs::Msrv;
 use rustc_errors::Applicability;
@@ -448,7 +448,7 @@ define_Conf! {
     avoid_breaking_exported_api: bool = true,
     /// The list of types which may not be held across an await point.
     #[lints(await_holding_invalid_type)]
-    await_holding_invalid_types: Vec<DisallowedPath> = Vec::new(),
+    await_holding_invalid_types: Vec<DisallowedPathWithoutReplacement> = Vec::new(),
     /// DEPRECATED LINT: BLACKLISTED_NAME.
     ///
     /// Use the Disallowed Names lint instead

Diff for: 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;
@@ -12,37 +14,99 @@ pub struct Rename {
     pub rename: String,
 }
 
-#[derive(Debug, Deserialize)]
+pub type DisallowedPathWithoutReplacement = DisallowedPath<false>;
+
+#[derive(Debug, Serialize)]
+pub struct DisallowedPath<const REPLACEMENT_ALLOWED: bool = true> {
+    path: String,
+    reason: Option<String>,
+    replacement: Option<String>,
+}
+
+impl<'de, const REPLACEMENT_ALLOWED: bool> Deserialize<'de> for DisallowedPath<REPLACEMENT_ALLOWED> {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let enum_ = DisallowedPathEnum::deserialize(deserializer)?;
+        if !REPLACEMENT_ALLOWED && enum_.replacement().is_some() {
+            return Err(de::Error::custom("replacement not allowed for this configuration"));
+        }
+        Ok(Self {
+            path: enum_.path().to_owned(),
+            reason: enum_.reason().map(ToOwned::to_owned),
+            replacement: enum_.replacement().map(ToOwned::to_owned),
+        })
+    }
+}
+
+// `DisallowedPathEnum` is an implementation detail to enable the `Deserialize` implementation just
+// above. `DisallowedPathEnum` is not meant to be used outside of this file.
+#[derive(Debug, Deserialize, Serialize)]
 #[serde(untagged)]
-pub enum DisallowedPath {
+enum DisallowedPathEnum {
     Simple(String),
-    WithReason { path: String, reason: Option<String> },
+    WithReason {
+        path: String,
+        reason: Option<String>,
+        replacement: Option<String>,
+    },
 }
 
-impl DisallowedPath {
+impl<const REPLACEMENT_ALLOWED: bool> DisallowedPath<REPLACEMENT_ALLOWED> {
+    pub fn path(&self) -> &str {
+        &self.path
+    }
+
+    pub fn diag_amendment(&self, span: Span) -> impl FnOnce(&mut Diag<'_, ()>) + use<'_, REPLACEMENT_ALLOWED> {
+        move |diag| {
+            if let Some(replacement) = &self.replacement {
+                diag.span_suggestion(
+                    span,
+                    self.reason.as_ref().map_or_else(|| String::from("use"), Clone::clone),
+                    replacement,
+                    Applicability::MachineApplicable,
+                );
+            } else if let Some(reason) = &self.reason {
+                diag.note(reason.clone());
+            }
+        }
+    }
+}
+
+impl DisallowedPathEnum {
     pub fn path(&self) -> &str {
         let (Self::Simple(path) | Self::WithReason { path, .. }) = self;
 
         path
     }
 
-    pub fn reason(&self) -> Option<&str> {
+    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(
+pub fn create_disallowed_map<const REPLACEMENT_ALLOWED: bool>(
     tcx: TyCtxt<'_>,
-    disallowed: &'static [DisallowedPath],
-) -> DefIdMap<(&'static str, Option<&'static str>)> {
+    disallowed: &'static [DisallowedPath<REPLACEMENT_ALLOWED>],
+) -> DefIdMap<(&'static str, &'static DisallowedPath<REPLACEMENT_ALLOWED>)> {
     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()
 }
 
@@ -436,7 +500,6 @@ macro_rules! unimplemented_serialize {
 }
 
 unimplemented_serialize! {
-    DisallowedPath,
     Rename,
     MacroMatcher,
 }

Diff for: 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::{DisallowedPathWithoutReplacement, 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 DisallowedPathWithoutReplacement)>,
 }
 
 impl AwaitHolding {
@@ -247,25 +247,26 @@ 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 DisallowedPathWithoutReplacement,
+) {
     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 for: 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::{
     AmbigArg, Expr, ExprKind, ForeignItem, HirId, ImplItem, Item, ItemKind, OwnerId, Pat, Path, Stmt, TraitItem, Ty,
@@ -59,7 +58,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.
@@ -90,13 +89,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 for: 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" },
     /// ]
     /// ```
     ///
@@ -56,7 +58,7 @@ declare_clippy_lint! {
 }
 
 pub struct DisallowedMethods {
-    disallowed: DefIdMap<(&'static str, Option<&'static str>)>,
+    disallowed: DefIdMap<(&'static str, &'static DisallowedPath)>,
 }
 
 impl DisallowedMethods {
@@ -83,17 +85,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 for: clippy_lints/src/disallowed_types.rs

@@ -1,4 +1,5 @@
 use clippy_config::Conf;
+use clippy_config::types::DisallowedPath;
 use clippy_utils::diagnostics::span_lint_and_then;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::def::Res;
@@ -31,6 +32,8 @@ declare_clippy_lint! {
     ///     # When using an inline table, can add a `reason` for why the type
     ///     # is disallowed.
     ///     { path = "std::net::Ipv4Addr", reason = "no IPv4 allowed" },
+    ///     # Can also add a `replacement` that will be offered as a suggestion.
+    ///     { path = "std::sync::Mutex", reason = "prefer faster & simpler non-poisonable mutex", replacement = "parking_lot::Mutex" },
     /// ]
     /// ```
     ///
@@ -51,24 +54,23 @@ declare_clippy_lint! {
 }
 
 pub struct DisallowedTypes {
-    def_ids: DefIdMap<(&'static str, Option<&'static str>)>,
-    prim_tys: FxHashMap<PrimTy, (&'static str, Option<&'static str>)>,
+    def_ids: DefIdMap<(&'static str, &'static DisallowedPath)>,
+    prim_tys: FxHashMap<PrimTy, (&'static str, &'static DisallowedPath)>,
 }
 
 impl DisallowedTypes {
     pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self {
         let mut def_ids = DefIdMap::default();
         let mut prim_tys = FxHashMap::default();
-        for x in &conf.disallowed_types {
-            let path: Vec<_> = x.path().split("::").collect::<Vec<_>>();
-            let reason = x.reason();
+        for disallowed_path in &conf.disallowed_types {
+            let path: Vec<_> = disallowed_path.path().split("::").collect::<Vec<_>>();
             for res in clippy_utils::def_path_res(tcx, &path) {
                 match res {
                     Res::Def(_, id) => {
-                        def_ids.insert(id, (x.path(), reason));
+                        def_ids.insert(id, (disallowed_path.path(), disallowed_path));
                     },
                     Res::PrimTy(ty) => {
-                        prim_tys.insert(ty, (x.path(), reason));
+                        prim_tys.insert(ty, (disallowed_path.path(), disallowed_path));
                     },
                     _ => {},
                 }
@@ -78,7 +80,7 @@ impl DisallowedTypes {
     }
 
     fn check_res_emit(&self, cx: &LateContext<'_>, res: &Res, span: Span) {
-        let (path, reason) = match res {
+        let (path, disallowed_path) = match res {
             Res::Def(_, did) if let Some(&x) = self.def_ids.get(did) => x,
             Res::PrimTy(prim) if let Some(&x) = self.prim_tys.get(prim) => x,
             _ => return,
@@ -88,11 +90,7 @@ impl DisallowedTypes {
             DISALLOWED_TYPES,
             span,
             format!("use of a disallowed type `{path}`"),
-            |diag| {
-                if let Some(reason) = reason {
-                    diag.note(reason);
-                }
-            },
+            disallowed_path.diag_amendment(span),
         );
     }
 }

Diff for: tests/ui-toml/await_holding_invalid_type_with_replacement/await_holding_invalid_type.rs

@@ -0,0 +1 @@
+fn main() {}

Diff for: tests/ui-toml/await_holding_invalid_type_with_replacement/await_holding_invalid_type.stderr

@@ -0,0 +1,11 @@
+error: error reading Clippy's configuration file: replacement not allowed for this configuration
+  --> $DIR/tests/ui-toml/await_holding_invalid_type_with_replacement/clippy.toml:1:31
+   |
+LL |   await-holding-invalid-types = [
+   |  _______________________________^
+LL | |     { path = "std::string::String", replacement = "std::net::Ipv4Addr" },
+LL | | ]
+   | |_^
+
+error: aborting due to 1 previous error
+

Diff for: tests/ui-toml/await_holding_invalid_type_with_replacement/clippy.toml

@@ -0,0 +1,3 @@
+await-holding-invalid-types = [
+    { path = "std::string::String", replacement = "std::net::Ipv4Addr" },
+]

Diff for: tests/ui-toml/replaceable_disallowed_types/clippy.toml

@@ -0,0 +1,3 @@
+disallowed-types = [
+    { path = "std::string::String", replacement = "wrapper::String" },
+]

Diff for: tests/ui-toml/replaceable_disallowed_types/replaceable_disallowed_types.fixed

@@ -0,0 +1,16 @@
+#![warn(clippy::disallowed_types)]
+
+#[allow(clippy::disallowed_types)]
+mod wrapper {
+    pub struct String(std::string::String);
+
+    impl From<&str> for String {
+        fn from(value: &str) -> Self {
+            Self(std::string::String::from(value))
+        }
+    }
+}
+
+fn main() {
+    let _ = wrapper::String::from("x");
+}