Address review comments

Author: smoelius

clippy.toml

@@ -7,14 +7,14 @@ lint-commented-code = true
 [[disallowed-methods]]
 path = "rustc_lint::context::LintContext::lint"
 reason = "this function does not add a link to our documentation, please use the `clippy_utils::diagnostics::span_lint*` functions instead"
-allow-invalid = true
+may-not-refer-to-existing-definition = true
 
 [[disallowed-methods]]
 path = "rustc_lint::context::LintContext::span_lint"
 reason = "this function does not add a link to our documentation, please use the `clippy_utils::diagnostics::span_lint*` functions instead"
-allow-invalid = true
+may-not-refer-to-existing-definition = true
 
 [[disallowed-methods]]
 path = "rustc_middle::ty::context::TyCtxt::node_span_lint"
 reason = "this function does not add a link to our documentation, please use the `clippy_utils::diagnostics::span_lint_hir*` functions instead"
-allow-invalid = true
+may-not-refer-to-existing-definition = true

clippy_config/src/conf.rs

@@ -225,7 +225,7 @@ macro_rules! define_Conf {
         $(#[doc = $doc:literal])+
         $(#[conf_deprecated($dep:literal, $new_conf:ident)])?
         $(#[default_text = $default_text:expr])?
-        $(#[disallowed_path_replacements_allowed = $replacements_allowed:expr])?
+        $(#[disallowed_paths_allow_replacements = $replacements_allowed:expr])?
         $(#[lints($($for_lints:ident),* $(,)?)])?
         $name:ident: $ty:ty = $default:expr,
     )*) => {
@@ -526,7 +526,7 @@ define_Conf! {
     )]
     avoid_breaking_exported_api: bool = true,
     /// The list of types which may not be held across an await point.
-    #[disallowed_path_replacements_allowed = false]
+    #[disallowed_paths_allow_replacements = false]
     #[lints(await_holding_invalid_type)]
     await_holding_invalid_types: Vec<DisallowedPathWithoutReplacement> = Vec::new(),
     /// DEPRECATED LINT: BLACKLISTED_NAME.
@@ -572,11 +572,11 @@ define_Conf! {
     #[conf_deprecated("Please use `cognitive-complexity-threshold` instead", cognitive_complexity_threshold)]
     cyclomatic_complexity_threshold: u64 = 25,
     /// The list of disallowed macros, written as fully qualified paths.
-    #[disallowed_path_replacements_allowed = true]
+    #[disallowed_paths_allow_replacements = true]
     #[lints(disallowed_macros)]
     disallowed_macros: Vec<DisallowedPath> = Vec::new(),
     /// The list of disallowed methods, written as fully qualified paths.
-    #[disallowed_path_replacements_allowed = true]
+    #[disallowed_paths_allow_replacements = true]
     #[lints(disallowed_methods)]
     disallowed_methods: Vec<DisallowedPath> = Vec::new(),
     /// The list of disallowed names to lint about. NB: `bar` is not here since it has legitimate uses. The value
@@ -585,7 +585,7 @@ define_Conf! {
     #[lints(disallowed_names)]
     disallowed_names: Vec<String> = DEFAULT_DISALLOWED_NAMES.iter().map(ToString::to_string).collect(),
     /// The list of disallowed types, written as fully qualified paths.
-    #[disallowed_path_replacements_allowed = true]
+    #[disallowed_paths_allow_replacements = true]
     #[lints(disallowed_types)]
     disallowed_types: Vec<DisallowedPath> = Vec::new(),
     /// The list of words this lint should not consider as identifiers needing ticks. The value

clippy_config/src/types.rs

@@ -23,11 +23,12 @@ pub struct DisallowedPath<const REPLACEMENT_ALLOWED: bool = true> {
     path: String,
     reason: Option<String>,
     replacement: Option<String>,
-    /// Setting `allow_invalid` to true suppresses a warning if `path` is invalid.
+    /// Setting `may_not_refer_to_existing_definition` to true suppresses a warning if `path` does
+    /// not refer to an existing definition.
     ///
     /// This could be useful when conditional compilation is used, or when a clippy.toml file is
     /// shared among multiple projects.
-    allow_invalid: bool,
+    may_not_refer_to_existing_definition: bool,
     /// The span of the `DisallowedPath`.
     ///
     /// Used for diagnostics.
@@ -48,7 +49,7 @@ impl<'de, const REPLACEMENT_ALLOWED: bool> Deserialize<'de> for DisallowedPath<R
             path: enum_.path().to_owned(),
             reason: enum_.reason().map(ToOwned::to_owned),
             replacement: enum_.replacement().map(ToOwned::to_owned),
-            allow_invalid: enum_.allow_invalid(),
+            may_not_refer_to_existing_definition: enum_.may_not_refer_to_existing_definition(),
             span: Span::default(),
         })
     }
@@ -64,8 +65,8 @@ enum DisallowedPathEnum {
         path: String,
         reason: Option<String>,
         replacement: Option<String>,
-        #[serde(rename = "allow-invalid")]
-        allow_invalid: Option<bool>,
+        #[serde(rename = "may-not-refer-to-existing-definition")]
+        may_not_refer_to_existing_definition: Option<bool>,
     },
 }
 
@@ -74,7 +75,7 @@ impl<const REPLACEMENT_ALLOWED: bool> DisallowedPath<REPLACEMENT_ALLOWED> {
         &self.path
     }
 
-    pub fn diag_amendment(&self, span: Span) -> impl FnOnce(&mut Diag<'_, ()>) + use<'_, REPLACEMENT_ALLOWED> {
+    pub fn diag_amendment(&self, span: Span) -> impl FnOnce(&mut Diag<'_, ()>) {
         move |diag| {
             if let Some(replacement) = &self.replacement {
                 diag.span_suggestion(
@@ -119,9 +120,12 @@ impl DisallowedPathEnum {
         }
     }
 
-    fn allow_invalid(&self) -> bool {
+    fn may_not_refer_to_existing_definition(&self) -> bool {
         match &self {
-            Self::WithReason { allow_invalid, .. } => allow_invalid.unwrap_or_default(),
+            Self::WithReason {
+                may_not_refer_to_existing_definition,
+                ..
+            } => may_not_refer_to_existing_definition.unwrap_or_default(),
             Self::Simple(_) => false,
         }
     }
@@ -133,6 +137,7 @@ pub fn create_disallowed_map<const REPLACEMENT_ALLOWED: bool>(
     tcx: TyCtxt<'_>,
     disallowed_paths: &'static [DisallowedPath<REPLACEMENT_ALLOWED>],
     def_kind_predicate: impl Fn(DefKind) -> bool,
+    predicate_description: &str,
     allow_prim_tys: bool,
 ) -> (
     DefIdMap<(&'static str, &'static DisallowedPath<REPLACEMENT_ALLOWED>)>,
@@ -143,13 +148,13 @@ pub fn create_disallowed_map<const REPLACEMENT_ALLOWED: bool>(
         FxHashMap::default();
     for disallowed_path in disallowed_paths {
         let path = disallowed_path.path();
-        let mut reses = clippy_utils::def_path_res(tcx, &path.split("::").collect::<Vec<_>>());
+        let mut resolutions = clippy_utils::def_path_res(tcx, &path.split("::").collect::<Vec<_>>());
 
-        let mut found_def_id = false;
+        let mut found_def_id = None;
         let mut found_prim_ty = false;
-        reses.retain(|res| match res {
-            Res::Def(def_kind, _) => {
-                found_def_id = true;
+        resolutions.retain(|res| match res {
+            Res::Def(def_kind, def_id) => {
+                found_def_id = Some(*def_id);
                 def_kind_predicate(*def_kind)
             },
             Res::PrimTy(_) => {
@@ -159,23 +164,32 @@ pub fn create_disallowed_map<const REPLACEMENT_ALLOWED: bool>(
             _ => false,
         });
 
-        if reses.is_empty() {
+        if resolutions.is_empty() {
             let span = disallowed_path.span();
 
-            if found_def_id {
-                tcx.sess
-                    .dcx()
-                    .span_warn(span, format!("`{path}` is not of an appropriate kind"));
+            if let Some(def_id) = found_def_id {
+                tcx.sess.dcx().span_warn(
+                    span,
+                    format!(
+                        "expected a {predicate_description}, found {} {}",
+                        tcx.def_descr_article(def_id),
+                        tcx.def_descr(def_id)
+                    ),
+                );
             } else if found_prim_ty {
-                tcx.sess
-                    .dcx()
-                    .span_warn(span, format!("primitive types are not allowed: {path}"));
-            } else if !disallowed_path.allow_invalid {
-                tcx.sess.dcx().span_warn(span, format!("`{path}` is invalid"));
+                tcx.sess.dcx().span_warn(
+                    span,
+                    format!("expected a {predicate_description}, found a primitive type",),
+                );
+            } else if !disallowed_path.may_not_refer_to_existing_definition {
+                tcx.sess.dcx().span_warn(
+                    span,
+                    format!("`{path}` does not refer to an existing {predicate_description}"),
+                );
             }
         }
 
-        for res in reses {
+        for res in resolutions {
             match res {
                 Res::Def(_, def_id) => {
                     def_ids.insert(def_id, (path, disallowed_path));

clippy_lints/src/await_holding_invalid.rs

@@ -183,6 +183,7 @@ impl AwaitHolding {
             tcx,
             &conf.await_holding_invalid_types,
             crate::disallowed_types::def_kind_predicate,
+            "type",
             false,
         );
         Self { def_ids }

clippy_lints/src/disallowed_macros.rs

@@ -76,6 +76,7 @@ impl DisallowedMacros {
             tcx,
             &conf.disallowed_macros,
             |def_kind| matches!(def_kind, DefKind::Macro(_)),
+            "macro",
             false,
         );
         Self {

clippy_lints/src/disallowed_methods.rs

@@ -72,6 +72,7 @@ impl DisallowedMethods {
                     DefKind::Fn | DefKind::Ctor(_, CtorKind::Fn) | DefKind::AssocFn
                 )
             },
+            "function",
             false,
         );
         Self { disallowed }
@@ -83,9 +84,6 @@ impl_lint_pass!(DisallowedMethods => [DISALLOWED_METHODS]);
 impl<'tcx> LateLintPass<'tcx> for DisallowedMethods {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         let (id, span) = match &expr.kind {
-            // Previous versions of this lint checked the `DefKind` in the next arm. However, that is no longer
-            // necessary. Paths are filtered by `DefKind` when `self.disallowed` is created. So if a path is not of an
-            // appropriate `DefKind`, `self.disallowed.get(..)` will return `None` below.
             ExprKind::Path(path) if let Res::Def(_, id) = cx.qpath_res(path, expr.hir_id) => (id, expr.span),
             ExprKind::MethodCall(name, ..) if let Some(id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) => {
                 (id, name.ident.span)

clippy_lints/src/disallowed_types.rs

@@ -60,7 +60,7 @@ pub struct DisallowedTypes {
 
 impl DisallowedTypes {
     pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self {
-        let (def_ids, prim_tys) = create_disallowed_map(tcx, &conf.disallowed_types, def_kind_predicate, true);
+        let (def_ids, prim_tys) = create_disallowed_map(tcx, &conf.disallowed_types, def_kind_predicate, "type", true);
         Self { def_ids, prim_tys }
     }
 

tests/ui-toml/toml_invalid_path/clippy.toml

@@ -11,4 +11,4 @@ path = "std::process::current_exe"
 
 [[disallowed-methods]]
 path = "std::current_exe"
-allow-invalid = true
+may-not-refer-to-existing-definition = true

tests/ui-toml/toml_invalid_path/conf_invalid_path.rs

@@ -1,5 +1,5 @@
-//@error-in-other-file: primitive types are not allowed: bool
-//@error-in-other-file: `std::process::current_exe` is invalid
-//@error-in-other-file: `std::result::Result::Err` is not of an appropriate kind
+//@error-in-other-file: expected a macro, found a primitive type
+//@error-in-other-file: `std::process::current_exe` does not refer to an existing function
+//@error-in-other-file: expected a type, found a tuple variant
 
 fn main() {}

tests/ui-toml/toml_invalid_path/conf_invalid_path.stderr

@@ -1,18 +1,18 @@
-warning: primitive types are not allowed: bool
+warning: expected a macro, found a primitive type
   --> $DIR/tests/ui-toml/toml_invalid_path/clippy.toml:4:1
    |
 LL | / [[disallowed-macros]]
 LL | | path = "bool"
    | |_____________^
 
-warning: `std::process::current_exe` is invalid
+warning: `std::process::current_exe` does not refer to an existing function
   --> $DIR/tests/ui-toml/toml_invalid_path/clippy.toml:7:1
    |
 LL | / [[disallowed-methods]]
 LL | | path = "std::process::current_exe"
    | |__________________________________^
 
-warning: `std::result::Result::Err` is not of an appropriate kind
+warning: expected a type, found a tuple variant
   --> $DIR/tests/ui-toml/toml_invalid_path/clippy.toml:1:1
    |
 LL | / [[disallowed-types]]