Revert workaround and patch Deserializer

This patch changes how ConfigMapAccess iterates k/v pairs when
deserializing structs.

Previously we produced keys for exactly the set of fields needed
for a struct, and errored out in the deserializer if we can't find
anything for that field.

This patch makes us produces keys from the union of two sets:

1. All fields that are both needed for the struct and can be found
   in the environment.
2. All fields in the config table.

This change allows serde's codegen to handle both missing and
unknown fields via the usual derive annotations (default or
deny_unknown_fields respectively)

Author: bearcage

src/cargo/core/features.rs

@@ -334,68 +334,32 @@ impl Features {
 ///
 /// If you have any trouble with this, please let us know!
 #[derive(Default, Debug, Deserialize)]
-#[serde(rename_all="kebab-case")]
+#[serde(default, rename_all = "kebab-case")]
 pub struct CliUnstable {
-    #[serde(deserialize_with="default_false")]
     pub print_im_a_teapot: bool,
-    #[serde(deserialize_with="default_false")]
     pub unstable_options: bool,
-    #[serde(deserialize_with="default_false")]
     pub no_index_update: bool,
-    #[serde(deserialize_with="default_false")]
     pub avoid_dev_deps: bool,
-    #[serde(deserialize_with="default_false")]
     pub minimal_versions: bool,
-    #[serde(deserialize_with="default_false")]
     pub package_features: bool,
-    #[serde(deserialize_with="default_false")]
     pub advanced_env: bool,
-    #[serde(deserialize_with="default_false")]
     pub config_include: bool,
-    #[serde(deserialize_with="default_false")]
     pub dual_proc_macros: bool,
-    #[serde(deserialize_with="default_false")]
     pub mtime_on_use: bool,
-    #[serde(deserialize_with="default_false")]
     pub named_profiles: bool,
-    #[serde(deserialize_with="default_false")]
     pub binary_dep_depinfo: bool,
     pub build_std: Option<Vec<String>>,
     pub timings: Option<Vec<String>>,
-    #[serde(deserialize_with="default_false")]
     pub doctest_xcompile: bool,
-    #[serde(deserialize_with="default_false")]
     pub panic_abort_tests: bool,
-    #[serde(deserialize_with="default_false")]
     pub jobserver_per_rustc: bool,
     pub features: Option<Vec<String>>,
-    #[serde(deserialize_with="default_false")]
     pub crate_versions: bool,
-    #[serde(deserialize_with="default_false")]
     pub separate_nightlies: bool,
-    #[serde(deserialize_with="default_false")]
     pub multitarget: bool,
-    #[serde(deserialize_with="default_false")]
     pub rustdoc_map: bool,
 }
 
-/// Treat boolean Zflags as optionals for deserializing them.
-/// This allows missing settings to default to disabled (effectively recreating
-/// the serde `default` behavior). Our Deserializer merges multiple sources
-/// inline, so the serde facilities for handling missing or additional fields
-/// don't quite fit.
-///
-/// TODO: This limitation can likely be removed by refactoring 
-///       `de::ConfigMapAccess` to iterate the union of the config and env sets.
-fn default_false<'de, D, T>(deserializer: D) -> Result<T, D::Error>
-where
-    D: serde::Deserializer<'de>,
-    T: Deserialize<'de> + Default,
-{
-    let option = Option::deserialize(deserializer)?;
-    Ok(option.unwrap_or_default())
-}
-
 impl CliUnstable {
     pub fn parse(&mut self, flags: &[String]) -> CargoResult<()> {
         if !flags.is_empty() && !nightly_features_allowed() {

src/cargo/util/config/de.rs

@@ -4,6 +4,7 @@ use crate::util::config::value;
 use crate::util::config::{Config, ConfigError, ConfigKey};
 use crate::util::config::{ConfigValue as CV, Definition, Value};
 use serde::{de, de::IntoDeserializer};
+use std::collections::HashSet;
 use std::vec;
 
 /// Serde deserializer used to convert config values to a target type using
@@ -269,37 +270,54 @@ impl<'config> ConfigMapAccess<'config> {
 
     fn new_struct(
         de: Deserializer<'config>,
-        fields: &'static [&'static str],
+        given_fields: &'static [&'static str],
     ) -> Result<ConfigMapAccess<'config>, ConfigError> {
-        let fields: Vec<KeyKind> = fields
-            .iter()
-            .map(|field| KeyKind::Normal(field.to_string()))
-            .collect();
+        let table = de.config.get_table(&de.key)?;
 
         // Assume that if we're deserializing a struct it exhaustively lists all
         // possible fields on this key that we're *supposed* to use, so take
         // this opportunity to warn about any keys that aren't recognized as
         // fields and warn about them.
-        if let Some(mut v) = de.config.get_table(&de.key)? {
-            for (t_key, value) in v.val.drain() {
-                if fields.iter().any(|k| match k {
-                    KeyKind::Normal(s) => s == &t_key,
-                    KeyKind::CaseSensitive(s) => s == &t_key,
-                }) {
-                    continue;
-                }
+        if let Some(v) = table.as_ref() {
+            let unused_keys = v
+                .val
+                .iter()
+                .filter(|(k, _v)| !given_fields.iter().any(|gk| gk == k));
+            for (unused_key, unused_value) in unused_keys {
                 de.config.shell().warn(format!(
                     "unused config key `{}.{}` in `{}`",
                     de.key,
-                    t_key,
-                    value.definition()
+                    unused_key,
+                    unused_value.definition()
                 ))?;
             }
         }
 
+        let mut fields = HashSet::new();
+
+        // If the caller is interested in a field which we can provide from
+        // the environment, get it from there.
+        for field in given_fields {
+            let mut field_key = de.key.clone();
+            field_key.push(field);
+            for env_key in de.config.env.keys() {
+                if env_key.starts_with(field_key.as_env_key()) {
+                    fields.insert(KeyKind::Normal(field.to_string()));
+                }
+            }
+        }
+
+        // Add everything from the config table we're interested in that we
+        // haven't already provided via an environment variable
+        if let Some(v) = table {
+            for key in v.val.keys() {
+                fields.insert(KeyKind::Normal(key.clone()));
+            }
+        }
+
         Ok(ConfigMapAccess {
             de,
-            fields,
+            fields: fields.into_iter().collect(),
             field_index: 0,
         })
     }

src/cargo/util/toml/mod.rs

@@ -394,7 +394,7 @@ impl<'de> de::Deserialize<'de> for U32OrBool {
 }
 
 #[derive(Deserialize, Serialize, Clone, Debug, Default, Eq, PartialEq)]
-#[serde(rename_all = "kebab-case")]
+#[serde(default, rename_all = "kebab-case")]
 pub struct TomlProfile {
     pub opt_level: Option<TomlOptLevel>,
     pub lto: Option<StringOrBool>,