Add new disallowed_fields lint (#16218)

Fixes https://github.com/rust-lang/rust-clippy/issues/9278.

This is something that we need for `cg_gcc` (cc @antoyo). It's almost
the same as https://github.com/rust-lang/rust-clippy/pull/6674 (which I
used as base).

changelog: Add new `disallowed_fields` lint

r? @samueltardieu

CHANGELOG.md

@@ -6433,6 +6433,7 @@ Released 2018-09-13
 [`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord
 [`derive_partial_eq_without_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_partial_eq_without_eq
 [`derived_hash_with_manual_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derived_hash_with_manual_eq
+[`disallowed_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_fields
 [`disallowed_macros`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_macros
 [`disallowed_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method
 [`disallowed_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods
@@ -7252,6 +7253,7 @@ Released 2018-09-13
 [`check-private-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-private-items
 [`cognitive-complexity-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#cognitive-complexity-threshold
 [`const-literal-digits-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#const-literal-digits-threshold
+[`disallowed-fields`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-fields
 [`disallowed-macros`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-macros
 [`disallowed-methods`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-methods
 [`disallowed-names`]: https://doc.rust-lang.org/clippy/lint_configuration.html#disallowed-names

book/src/lint_configuration.md

@@ -505,6 +505,23 @@ The minimum digits a const float literal must have to supress the `excessive_pre
 * [`excessive_precision`](https://rust-lang.github.io/rust-clippy/master/index.html#excessive_precision)
 
 
+## `disallowed-fields`
+The list of disallowed fields, written as fully qualified paths.
+
+**Fields:**
+- `path` (required): the fully qualified path to the field that should be disallowed
+- `reason` (optional): explanation why this field is disallowed
+- `replacement` (optional): suggested alternative method
+- `allow-invalid` (optional, `false` by default): when set to `true`, it will ignore this entry
+  if the path doesn't exist, instead of emitting an error
+
+**Default Value:** `[]`
+
+---
+**Affected lints:**
+* [`disallowed_fields`](https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_fields)
+
+
 ## `disallowed-macros`
 The list of disallowed macros, written as fully qualified paths.
 

clippy_config/src/conf.rs

@@ -581,6 +581,17 @@ fn span_from_toml_range(file: &SourceFile, span: Range<usize>) -> Span {
     /// Use the Cognitive Complexity lint instead.
     #[conf_deprecated("Please use `cognitive-complexity-threshold` instead", cognitive_complexity_threshold)]
     cyclomatic_complexity_threshold: u64 = 25,
+    /// The list of disallowed fields, written as fully qualified paths.
+    ///
+    /// **Fields:**
+    /// - `path` (required): the fully qualified path to the field that should be disallowed
+    /// - `reason` (optional): explanation why this field is disallowed
+    /// - `replacement` (optional): suggested alternative method
+    /// - `allow-invalid` (optional, `false` by default): when set to `true`, it will ignore this entry
+    ///   if the path doesn't exist, instead of emitting an error
+    #[disallowed_paths_allow_replacements = true]
+    #[lints(disallowed_fields)]
+    disallowed_fields: Vec<DisallowedPath> = Vec::new(),
     /// The list of disallowed macros, written as fully qualified paths.
     ///
     /// **Fields:**

clippy_lints/src/declared_lints.rs

@@ -105,6 +105,7 @@
     crate::derive::DERIVE_PARTIAL_EQ_WITHOUT_EQ_INFO,
     crate::derive::EXPL_IMPL_CLONE_ON_COPY_INFO,
     crate::derive::UNSAFE_DERIVE_DESERIALIZE_INFO,
+    crate::disallowed_fields::DISALLOWED_FIELDS_INFO,
     crate::disallowed_macros::DISALLOWED_MACROS_INFO,
     crate::disallowed_methods::DISALLOWED_METHODS_INFO,
     crate::disallowed_names::DISALLOWED_NAMES_INFO,

clippy_lints/src/disallowed_fields.rs

@@ -0,0 +1,160 @@
+use clippy_config::Conf;
+use clippy_config::types::{DisallowedPath, create_disallowed_map};
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::paths::PathNS;
+use clippy_utils::ty::get_field_def_id_by_name;
+use rustc_hir::def::{DefKind, Res};
+use rustc_hir::def_id::DefIdMap;
+use rustc_hir::{Expr, ExprKind, Pat, PatKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::TyCtxt;
+use rustc_session::impl_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Denies the configured fields in clippy.toml
+    ///
+    /// Note: Even though this lint is warn-by-default, it will only trigger if
+    /// fields are defined in the clippy.toml file.
+    ///
+    /// ### Why is this bad?
+    /// Some fields are undesirable in certain contexts, and it's beneficial to
+    /// lint for them as needed.
+    ///
+    /// ### Example
+    /// An example clippy.toml configuration:
+    /// ```toml
+    /// # clippy.toml
+    /// disallowed-fields = [
+    ///     # Can use a string as the path of the disallowed field.
+    ///     "std::ops::Range::start",
+    ///     # Can also use an inline table with a `path` key.
+    ///     { path = "std::ops::Range::start" },
+    ///     # When using an inline table, can add a `reason` for why the field
+    ///     # is disallowed.
+    ///     { path = "std::ops::Range::start", reason = "The start of the range is not used" },
+    /// ]
+    /// ```
+    ///
+    /// ```rust
+    /// use std::ops::Range;
+    ///
+    /// let range = Range { start: 0, end: 1 };
+    /// println!("{}", range.start); // `start` is disallowed in the config.
+    /// ```
+    ///
+    /// Use instead:
+    /// ```rust
+    /// use std::ops::Range;
+    ///
+    /// let range = Range { start: 0, end: 1 };
+    /// println!("{}", range.end); // `end` is _not_ disallowed in the config.
+    /// ```
+    #[clippy::version = "1.93.0"]
+    pub DISALLOWED_FIELDS,
+    style,
+    "declaration of a disallowed field use"
+}
+
+pub struct DisallowedFields {
+    disallowed: DefIdMap<(&'static str, &'static DisallowedPath)>,
+}
+
+impl DisallowedFields {
+    pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self {
+        let (disallowed, _) = create_disallowed_map(
+            tcx,
+            &conf.disallowed_fields,
+            PathNS::Field,
+            |def_kind| matches!(def_kind, DefKind::Field),
+            "field",
+            false,
+        );
+        Self { disallowed }
+    }
+}
+
+impl_lint_pass!(DisallowedFields => [DISALLOWED_FIELDS]);
+
+impl<'tcx> LateLintPass<'tcx> for DisallowedFields {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        let (id, span) = match &expr.kind {
+            ExprKind::Path(path) if let Res::Def(_, id) = cx.qpath_res(path, expr.hir_id) => (id, expr.span),
+            ExprKind::Field(e, ident) => {
+                // Very round-about way to get the field `DefId` from the expr: first we get its
+                // parent `Ty`. Then we go through all its fields to find the one with the expected
+                // name and get the `DefId` from it.
+                if let Some(parent_ty) = cx.typeck_results().expr_ty_adjusted_opt(e)
+                    && let Some(field_def_id) = get_field_def_id_by_name(parent_ty, ident.name)
+                {
+                    (field_def_id, ident.span)
+                } else {
+                    return;
+                }
+            },
+            _ => return,
+        };
+        if let Some(&(path, disallowed_path)) = self.disallowed.get(&id) {
+            span_lint_and_then(
+                cx,
+                DISALLOWED_FIELDS,
+                span,
+                format!("use of a disallowed field `{path}`"),
+                disallowed_path.diag_amendment(span),
+            );
+        }
+    }
+
+    fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
+        let PatKind::Struct(struct_path, pat_fields, _) = pat.kind else {
+            return;
+        };
+        match cx.typeck_results().qpath_res(&struct_path, pat.hir_id) {
+            Res::Def(DefKind::Struct, struct_def_id) => {
+                let adt_def = cx.tcx.adt_def(struct_def_id);
+                for field in pat_fields {
+                    if let Some(def_id) = adt_def.all_fields().find_map(|adt_field| {
+                        if field.ident.name == adt_field.name {
+                            Some(adt_field.did)
+                        } else {
+                            None
+                        }
+                    }) && let Some(&(path, disallowed_path)) = self.disallowed.get(&def_id)
+                    {
+                        span_lint_and_then(
+                            cx,
+                            DISALLOWED_FIELDS,
+                            field.span,
+                            format!("use of a disallowed field `{path}`"),
+                            disallowed_path.diag_amendment(field.span),
+                        );
+                    }
+                }
+            },
+            Res::Def(DefKind::Variant, variant_def_id) => {
+                let enum_def_id = cx.tcx.parent(variant_def_id);
+                let variant = cx.tcx.adt_def(enum_def_id).variant_with_id(variant_def_id);
+
+                for field in pat_fields {
+                    if let Some(def_id) = variant.fields.iter().find_map(|adt_field| {
+                        if field.ident.name == adt_field.name {
+                            Some(adt_field.did)
+                        } else {
+                            None
+                        }
+                    }) && let Some(&(path, disallowed_path)) = self.disallowed.get(&def_id)
+                    {
+                        span_lint_and_then(
+                            cx,
+                            DISALLOWED_FIELDS,
+                            field.span,
+                            format!("use of a disallowed field `{path}`"),
+                            disallowed_path.diag_amendment(field.span),
+                        );
+                    }
+                }
+            },
+            _ => {},
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -103,6 +103,7 @@
 mod dereference;
 mod derivable_impls;
 mod derive;
+mod disallowed_fields;
 mod disallowed_macros;
 mod disallowed_methods;
 mod disallowed_names;
@@ -857,6 +858,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
         Box::new(|_| Box::new(toplevel_ref_arg::ToplevelRefArg)),
         Box::new(|_| Box::new(volatile_composites::VolatileComposites)),
         Box::new(|_| Box::<replace_box::ReplaceBox>::default()),
+        Box::new(move |tcx| Box::new(disallowed_fields::DisallowedFields::new(tcx, conf))),
         Box::new(move |_| Box::new(manual_ilog2::ManualIlog2::new(conf))),
         Box::new(|_| Box::new(same_length_and_capacity::SameLengthAndCapacity)),
         Box::new(move |tcx| Box::new(duration_suboptimal_units::DurationSuboptimalUnits::new(tcx, conf))),

clippy_utils/src/paths.rs

@@ -26,6 +26,7 @@ pub enum PathNS {
     Type,
     Value,
     Macro,
+    Field,
 
     /// Resolves to the name in the first available namespace, e.g. for `std::vec` this would return
     /// either the macro or the module but **not** both
@@ -41,6 +42,7 @@ fn matches(self, ns: Option<Namespace>) -> bool {
             PathNS::Type => TypeNS,
             PathNS::Value => ValueNS,
             PathNS::Macro => MacroNS,
+            PathNS::Field => return false,
             PathNS::Arbitrary => return true,
         };
 
@@ -286,6 +288,20 @@ fn local_item_child_by_name(tcx: TyCtxt<'_>, local_id: LocalDefId, ns: PathNS, n
             &root_mod
         },
         Node::Item(item) => &item.kind,
+        Node::Variant(variant) if ns == PathNS::Field => {
+            return if let rustc_hir::VariantData::Struct { fields, .. } = variant.data
+                && let Some(field_def_id) = fields.iter().find_map(|field| {
+                    if field.ident.name == name {
+                        Some(field.def_id.to_def_id())
+                    } else {
+                        None
+                    }
+                }) {
+                Some(field_def_id)
+            } else {
+                None
+            };
+        },
         _ => return None,
     };
 
@@ -299,6 +315,7 @@ fn local_item_child_by_name(tcx: TyCtxt<'_>, local_id: LocalDefId, ns: PathNS, n
                         PathNS::Type => opt_def_id(path.res.type_ns),
                         PathNS::Value => opt_def_id(path.res.value_ns),
                         PathNS::Macro => opt_def_id(path.res.macro_ns),
+                        PathNS::Field => None,
                         PathNS::Arbitrary => unreachable!(),
                     }
                 } else {
@@ -318,6 +335,24 @@ fn local_item_child_by_name(tcx: TyCtxt<'_>, local_id: LocalDefId, ns: PathNS, n
             .filter_by_name_unhygienic(name)
             .find(|assoc_item| ns.matches(Some(assoc_item.namespace())))
             .map(|assoc_item| assoc_item.def_id),
+        ItemKind::Struct(_, _, rustc_hir::VariantData::Struct { fields, .. }) if ns == PathNS::Field => {
+            fields.iter().find_map(|field| {
+                if field.ident.name == name {
+                    Some(field.def_id.to_def_id())
+                } else {
+                    None
+                }
+            })
+        },
+        ItemKind::Enum(_, _, rustc_hir::EnumDef { variants }) if ns == PathNS::Type => {
+            variants.iter().find_map(|variant| {
+                if variant.ident.name == name {
+                    Some(variant.def_id.to_def_id())
+                } else {
+                    None
+                }
+            })
+        },
         _ => None,
     }
 }
@@ -336,6 +371,11 @@ fn non_local_item_child_by_name(tcx: TyCtxt<'_>, def_id: DefId, ns: PathNS, name
             .iter()
             .copied()
             .find(|assoc_def_id| tcx.item_name(*assoc_def_id) == name && ns.matches(tcx.def_kind(assoc_def_id).ns())),
+        DefKind::Struct => tcx
+            .associated_item_def_ids(def_id)
+            .iter()
+            .copied()
+            .find(|assoc_def_id| tcx.item_name(*assoc_def_id) == name),
         _ => None,
     }
 }

clippy_utils/src/ty/mod.rs

@@ -1265,6 +1265,13 @@ pub fn get_field_by_name<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, name: Symbol) ->
     }
 }
 
+pub fn get_field_def_id_by_name(ty: Ty<'_>, name: Symbol) -> Option<DefId> {
+    let ty::Adt(adt_def, ..) = ty.kind() else { return None };
+    adt_def
+        .all_fields()
+        .find_map(|field| if field.name == name { Some(field.did) } else { None })
+}
+
 /// Check if `ty` is an `Option` and return its argument type if it is.
 pub fn option_arg_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
     match *ty.kind() {

tests/ui-toml/toml_disallowed_fields/clippy.toml

@@ -0,0 +1,14 @@
+disallowed-fields = [
+    # just a string is shorthand for path only
+    "std::ops::Range::start",
+    # can give path and reason with an inline table
+    { path = "std::ops::Range::end", reason = "no end allowed" },
+    # can use an inline table but omit reason
+    { path = "std::ops::RangeTo::end" },
+    # local paths
+    "conf_disallowed_fields::X::y",
+    # re-exports
+    "conf_disallowed_fields::Y::y",
+    # field of a variant
+    "conf_disallowed_fields::Z::B::x",
+]

tests/ui-toml/toml_disallowed_fields/conf_disallowed_fields.rs

@@ -0,0 +1,50 @@
+#![warn(clippy::disallowed_fields)]
+#![allow(clippy::match_single_binding)]
+
+use std::ops::{Range, RangeTo};
+
+struct X {
+    y: u32,
+}
+
+enum Z {
+    A { x: u32 },
+    B { x: u32 },
+}
+
+use crate::X as Y;
+
+fn b(X { y }: X) {}
+//~^ disallowed_fields
+
+fn main() {
+    let x = X { y: 0 };
+    let _ = x.y;
+    //~^ disallowed_fields
+
+    let x = Y { y: 0 };
+    let _ = x.y;
+    //~^ disallowed_fields
+
+    let x = Range { start: 0, end: 0 };
+    let _ = x.start;
+    //~^ disallowed_fields
+    let _ = x.end;
+    //~^ disallowed_fields
+    let Range { start, .. } = x;
+    //~^ disallowed_fields
+
+    let x = RangeTo { end: 0 };
+    let _ = x.end;
+    //~^ disallowed_fields
+
+    match x {
+        RangeTo { end } => {}, //~ disallowed_fields
+    }
+
+    let x = Z::B { x: 0 };
+    match x {
+        Z::A { x } => {},
+        Z::B { x } => {}, //~ disallowed_fields
+    }
+}

tests/ui-toml/toml_disallowed_fields/conf_disallowed_fields.stderr

@@ -0,0 +1,61 @@
+error: use of a disallowed field `conf_disallowed_fields::Y::y`
+  --> tests/ui-toml/toml_disallowed_fields/conf_disallowed_fields.rs:17:10
+   |
+LL | fn b(X { y }: X) {}
+   |          ^
+   |
+   = note: `-D clippy::disallowed-fields` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::disallowed_fields)]`
+
+error: use of a disallowed field `conf_disallowed_fields::Y::y`
+  --> tests/ui-toml/toml_disallowed_fields/conf_disallowed_fields.rs:22:15
+   |
+LL |     let _ = x.y;
+   |               ^
+
+error: use of a disallowed field `conf_disallowed_fields::Y::y`
+  --> tests/ui-toml/toml_disallowed_fields/conf_disallowed_fields.rs:26:15
+   |
+LL |     let _ = x.y;
+   |               ^
+
+error: use of a disallowed field `std::ops::Range::start`
+  --> tests/ui-toml/toml_disallowed_fields/conf_disallowed_fields.rs:30:15
+   |
+LL |     let _ = x.start;
+   |               ^^^^^
+
+error: use of a disallowed field `std::ops::Range::end`
+  --> tests/ui-toml/toml_disallowed_fields/conf_disallowed_fields.rs:32:15
+   |
+LL |     let _ = x.end;
+   |               ^^^
+   |
+   = note: no end allowed
+
+error: use of a disallowed field `std::ops::Range::start`
+  --> tests/ui-toml/toml_disallowed_fields/conf_disallowed_fields.rs:34:17
+   |
+LL |     let Range { start, .. } = x;
+   |                 ^^^^^
+
+error: use of a disallowed field `std::ops::RangeTo::end`
+  --> tests/ui-toml/toml_disallowed_fields/conf_disallowed_fields.rs:38:15
+   |
+LL |     let _ = x.end;
+   |               ^^^
+
+error: use of a disallowed field `std::ops::RangeTo::end`
+  --> tests/ui-toml/toml_disallowed_fields/conf_disallowed_fields.rs:42:19
+   |
+LL |         RangeTo { end } => {},
+   |                   ^^^
+
+error: use of a disallowed field `conf_disallowed_fields::Z::B::x`
+  --> tests/ui-toml/toml_disallowed_fields/conf_disallowed_fields.rs:48:16
+   |
+LL |         Z::B { x } => {},
+   |                ^
+
+error: aborting due to 9 previous errors
+

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

@@ -37,6 +37,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
            check-private-items
            cognitive-complexity-threshold
            const-literal-digits-threshold
+           disallowed-fields
            disallowed-macros
            disallowed-methods
            disallowed-names
@@ -136,6 +137,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
            check-private-items
            cognitive-complexity-threshold
            const-literal-digits-threshold
+           disallowed-fields
            disallowed-macros
            disallowed-methods
            disallowed-names
@@ -235,6 +237,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
            check-private-items
            cognitive-complexity-threshold
            const-literal-digits-threshold
+           disallowed-fields
            disallowed-macros
            disallowed-methods
            disallowed-names