Add internal lint derive_deserialize_allowing_unknown

clippy_config/src/lib.rs

@@ -12,6 +12,7 @@
     rustc::diagnostic_outside_of_impl,
     rustc::untranslatable_diagnostic
 )]
+#![deny(clippy::derive_deserialize_allowing_unknown)]
 
 extern crate rustc_errors;
 extern crate rustc_hir;

clippy_config/src/types.rs

@@ -9,6 +9,7 @@ use std::collections::HashMap;
 use std::fmt;
 
 #[derive(Debug, Deserialize)]
+#[serde(deny_unknown_fields)]
 pub struct Rename {
     pub path: String,
     pub rename: String,
@@ -43,7 +44,7 @@ impl<'de, const REPLACEMENT_ALLOWED: bool> Deserialize<'de> for DisallowedPath<R
 // `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)]
+#[serde(untagged, deny_unknown_fields)]
 enum DisallowedPathEnum {
     Simple(String),
     WithReason {

clippy_lints/src/declared_lints.rs

@@ -8,6 +8,8 @@ pub static LINTS: &[&crate::LintInfo] = &[
     #[cfg(feature = "internal")]
     crate::utils::internal_lints::collapsible_calls::COLLAPSIBLE_SPAN_LINT_CALLS_INFO,
     #[cfg(feature = "internal")]
+    crate::utils::internal_lints::derive_deserialize_allowing_unknown::DERIVE_DESERIALIZE_ALLOWING_UNKNOWN_INFO,
+    #[cfg(feature = "internal")]
     crate::utils::internal_lints::interning_defined_symbol::INTERNING_DEFINED_SYMBOL_INFO,
     #[cfg(feature = "internal")]
     crate::utils::internal_lints::interning_defined_symbol::UNNECESSARY_SYMBOL_STR_INFO,

clippy_lints/src/lib.rs

@@ -609,6 +609,9 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
         });
         store.register_early_pass(|| Box::new(utils::internal_lints::produce_ice::ProduceIce));
         store.register_late_pass(|_| Box::new(utils::internal_lints::collapsible_calls::CollapsibleCalls));
+        store.register_late_pass(|_| {
+            Box::new(utils::internal_lints::derive_deserialize_allowing_unknown::DeriveDeserializeAllowingUnknown)
+        });
         store.register_late_pass(|_| Box::new(utils::internal_lints::invalid_paths::InvalidPaths));
         store.register_late_pass(|_| {
             Box::<utils::internal_lints::interning_defined_symbol::InterningDefinedSymbol>::default()

clippy_lints/src/utils/internal_lints.rs

@@ -1,5 +1,6 @@
 pub mod almost_standard_lint_formulation;
 pub mod collapsible_calls;
+pub mod derive_deserialize_allowing_unknown;
 pub mod interning_defined_symbol;
 pub mod invalid_paths;
 pub mod lint_without_lint_pass;

clippy_lints/src/utils/internal_lints/derive_deserialize_allowing_unknown.rs

@@ -0,0 +1,165 @@
+use clippy_utils::diagnostics::span_lint;
+use clippy_utils::{def_path_res, paths};
+use rustc_ast::tokenstream::{TokenStream, TokenTree};
+use rustc_ast::{AttrStyle, DelimArgs};
+use rustc_hir::def::Res;
+use rustc_hir::def_id::LocalDefId;
+use rustc_hir::{
+    AttrArgs, AttrItem, AttrPath, Attribute, HirId, Impl, Item, ItemKind, Path, QPath, TraitRef, Ty, TyKind,
+};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::TyCtxt;
+use rustc_session::declare_lint_pass;
+use rustc_span::sym;
+use std::sync::OnceLock;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for structs or enums that derive `serde::Deserialize` and that
+    /// do not have a `#[serde(deny_unknown_fields)]` attribute.
+    ///
+    /// ### Why is this bad?
+    /// If the struct or enum is used in [`clippy_config::conf::Conf`] and a
+    /// user inserts an unknown field by mistake, the user's error will be
+    /// silently ignored.
+    ///
+    /// ### Example
+    /// ```rust
+    /// #[derive(serde::Deserialize)]
+    /// pub struct DisallowedPath {
+    ///     path: String,
+    ///     reason: Option<String>,
+    ///     replacement: Option<String>,
+    /// }
+    /// ```
+    ///
+    /// Use instead:
+    /// ```rust
+    /// #[derive(serde::Deserialize)]
+    /// #[serde(deny_unknown_fields)]
+    /// pub struct DisallowedPath {
+    ///     path: String,
+    ///     reason: Option<String>,
+    ///     replacement: Option<String>,
+    /// }
+    /// ```
+    pub DERIVE_DESERIALIZE_ALLOWING_UNKNOWN,
+    internal,
+    "`#[derive(serde::Deserialize)]` without `#[serde(deny_unknown_fields)]`"
+}
+
+declare_lint_pass!(DeriveDeserializeAllowingUnknown => [DERIVE_DESERIALIZE_ALLOWING_UNKNOWN]);
+
+impl<'tcx> LateLintPass<'tcx> for DeriveDeserializeAllowingUnknown {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
+        // Is this an `impl` (of a certain form)?
+        let ItemKind::Impl(Impl {
+            of_trait: Some(TraitRef {
+                path: Path { res, .. }, ..
+            }),
+            self_ty:
+                Ty {
+                    kind:
+                        TyKind::Path(QPath::Resolved(
+                            None,
+                            Path {
+                                res: Res::Def(_, self_ty_def_id),
+                                ..
+                            },
+                        )),
+                    ..
+                },
+            ..
+        }) = item.kind
+        else {
+            return;
+        };
+
+        // Is it an `impl` of the trait `serde::Deserialize`?
+        if !is_serde_deserialize_res(cx.tcx, res) {
+            return;
+        }
+
+        // Is it derived?
+        if !cx.tcx.has_attr(item.owner_id, sym::automatically_derived) {
+            return;
+        }
+
+        // Is `self_ty` local?
+        let Some(local_def_id) = self_ty_def_id.as_local() else {
+            return;
+        };
+
+        // Does `self_ty` have a variant with named fields?
+        if !has_variant_with_named_fields(cx.tcx, local_def_id) {
+            return;
+        }
+
+        let hir_id = cx.tcx.local_def_id_to_hir_id(local_def_id);
+
+        // Does `self_ty` have `#[serde(deny_unknown_fields)]`?
+        if let Some(tokens) = find_serde_attr_item(cx.tcx, hir_id)
+            && tokens.iter().any(is_deny_unknown_fields_token)
+        {
+            return;
+        }
+
+        span_lint(
+            cx,
+            DERIVE_DESERIALIZE_ALLOWING_UNKNOWN,
+            item.span,
+            "`#[derive(serde::Deserialize)]` without `#[serde(deny_unknown_fields)]`",
+        );
+    }
+}
+
+fn is_serde_deserialize_res(tcx: TyCtxt<'_>, res: &Res) -> bool {
+    static SERDE_DESERIALIZE_RESES: OnceLock<Vec<Res>> = OnceLock::new();
+
+    let serde_deserialize_reses = SERDE_DESERIALIZE_RESES.get_or_init(|| def_path_res(tcx, &paths::SERDE_DESERIALIZE));
+
+    serde_deserialize_reses.contains(res)
+}
+
+// Determines whether `def_id` corresponds to an ADT with at least one variant with named fields. A
+// variant has named fields if its `ctor` field is `None`.
+fn has_variant_with_named_fields(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
+    let ty = tcx.type_of(def_id).skip_binder();
+
+    let rustc_middle::ty::Adt(adt_def, _) = ty.kind() else {
+        return false;
+    };
+
+    adt_def.variants().iter().any(|variant_def| variant_def.ctor.is_none())
+}
+
+fn find_serde_attr_item(tcx: TyCtxt<'_>, hir_id: HirId) -> Option<&TokenStream> {
+    tcx.hir().attrs(hir_id).iter().find_map(|attribute| {
+        if let Attribute::Unparsed(attr_item) = attribute
+            && let AttrItem {
+                path: AttrPath { segments, .. },
+                args: AttrArgs::Delimited(DelimArgs { tokens, .. }),
+                style: AttrStyle::Outer,
+                ..
+            } = &**attr_item
+            && segments.len() == 1
+            && segments[0].as_str() == "serde"
+        {
+            Some(tokens)
+        } else {
+            None
+        }
+    })
+}
+
+fn is_deny_unknown_fields_token(tt: &TokenTree) -> bool {
+    if let TokenTree::Token(token, _) = tt
+        && token
+            .ident()
+            .is_some_and(|(token, _)| token.as_str() == "deny_unknown_fields")
+    {
+        true
+    } else {
+        false
+    }
+}

tests/dogfood.rs

@@ -84,6 +84,8 @@ fn run_clippy_for_package(project: &str, args: &[&str]) -> bool {
     if cfg!(feature = "internal") {
         // internal lints only exist if we build with the internal feature
         command.args(["-D", "clippy::internal"]);
+        // `derive_deserialize_allowing_unknown` is crate-wide denied in `clippy_config`.
+        command.args(["-A", "clippy::derive_deserialize_allowing_unknown"]);
     } else {
         // running a clippy built without internal lints on the clippy source
         // that contains e.g. `allow(clippy::invalid_paths)`

tests/ui-internal/derive_deserialize_allowing_unknown.rs

@@ -0,0 +1,60 @@
+#![deny(clippy::internal)]
+
+use serde::{Deserialize, Deserializer};
+
+#[derive(Deserialize)]
+struct Struct {
+    flag: bool,
+    limit: u64,
+}
+
+#[derive(Deserialize)]
+enum Enum {
+    A(bool),
+    B { limit: u64 },
+}
+
+// negative tests
+
+#[derive(Deserialize)]
+#[serde(deny_unknown_fields)]
+struct StructWithDenyUnknownFields {
+    flag: bool,
+    limit: u64,
+}
+
+#[derive(Deserialize)]
+#[serde(deny_unknown_fields)]
+enum EnumWithDenyUnknownFields {
+    A(bool),
+    B { limit: u64 },
+}
+
+#[derive(Deserialize)]
+#[serde(untagged, deny_unknown_fields)]
+enum MultipleSerdeAttributes {
+    A(bool),
+    B { limit: u64 },
+}
+
+#[derive(Deserialize)]
+struct TupleStruct(u64, bool);
+
+#[derive(Deserialize)]
+#[serde(deny_unknown_fields)]
+enum EnumWithOnlyTupleVariants {
+    A(bool),
+    B(u64),
+}
+
+struct ManualSerdeImplementation;
+
+impl<'de> Deserialize<'de> for ManualSerdeImplementation {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let () = <() as Deserialize>::deserialize(deserializer)?;
+        Ok(ManualSerdeImplementation)
+    }
+}

tests/ui-internal/derive_deserialize_allowing_unknown.stderr

@@ -0,0 +1,24 @@
+error: `#[derive(serde::Deserialize)]` without `#[serde(deny_unknown_fields)]`
+  --> tests/ui-internal/derive_deserialize_allowing_unknown.rs:5:10
+   |
+LL | #[derive(Deserialize)]
+   |          ^^^^^^^^^^^
+   |
+note: the lint level is defined here
+  --> tests/ui-internal/derive_deserialize_allowing_unknown.rs:1:9
+   |
+LL | #![deny(clippy::internal)]
+   |         ^^^^^^^^^^^^^^^^
+   = note: `#[deny(clippy::derive_deserialize_allowing_unknown)]` implied by `#[deny(clippy::internal)]`
+   = note: this error originates in the derive macro `Deserialize` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: `#[derive(serde::Deserialize)]` without `#[serde(deny_unknown_fields)]`
+  --> tests/ui-internal/derive_deserialize_allowing_unknown.rs:11:10
+   |
+LL | #[derive(Deserialize)]
+   |          ^^^^^^^^^^^
+   |
+   = note: this error originates in the derive macro `Deserialize` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: aborting due to 2 previous errors
+

tests/ui-toml/toml_unknown_config_struct_field/clippy.toml

@@ -0,0 +1,4 @@
+# In the following configuration, "recommendation" should be "reason" or "replacement".
+disallowed-macros = [
+    { path = "std::panic", recommendation = "return a `std::result::Result::Error` instead" },
+]

tests/ui-toml/toml_unknown_config_struct_field/toml_unknown_config_struct_field.rs

@@ -0,0 +1,5 @@
+#[rustfmt::skip]
+//@error-in-other-file: error reading Clippy's configuration file: data did not match any variant of untagged enum DisallowedPathEnum
+fn main() {
+    panic!();
+}

tests/ui-toml/toml_unknown_config_struct_field/toml_unknown_config_struct_field.stderr

@@ -0,0 +1,11 @@
+error: error reading Clippy's configuration file: data did not match any variant of untagged enum DisallowedPathEnum
+  --> $DIR/tests/ui-toml/toml_unknown_config_struct_field/clippy.toml:2:21
+   |
+LL |   disallowed-macros = [
+   |  _____________________^
+LL | |     { path = "std::panic", recommendation = "return a `std::result::Result::Error` instead" },
+LL | | ]
+   | |_^
+
+error: aborting due to 1 previous error
+