Refactor ConfigKey to its own file

Also make it a little less allocation-heavy by tweaking the API to
encourage incremental building of the key and incremental destruction as
we walk throughout the configuration tree.

Author: alexcrichton

src/cargo/util/config/de.rs

@@ -1,14 +1,16 @@
 //! Support for deserializing configuration via `serde`
 
-use crate::util::config::{Config, ConfigError, ConfigKey, ConfigKeyPart};
-use crate::util::config::{ConfigValue as CV, Value, Definition};
-use std::path::PathBuf;
+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::path::PathBuf;
 use std::vec;
 
 /// Serde deserializer used to convert config values to a target type using
 /// `Config::get`.
+#[derive(Clone)]
 pub(crate) struct Deserializer<'config> {
     pub(crate) config: &'config Config,
     pub(crate) key: ConfigKey,
@@ -21,10 +23,10 @@ macro_rules! deserialize_method {
             V: de::Visitor<'de>,
         {
             let v = self.config.$getter(&self.key)?.ok_or_else(||
-                ConfigError::missing(&self.key.to_config()))?;
+                ConfigError::missing(&self.key))?;
             let Value{val, definition} = v;
             let res: Result<V::Value, ConfigError> = visitor.$visit(val);
-            res.map_err(|e| e.with_key_context(&self.key.to_config(), definition))
+            res.map_err(|e| e.with_key_context(&self.key, definition))
         }
     }
 }
@@ -39,7 +41,7 @@ impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> {
         // Future note: If you ever need to deserialize a non-self describing
         // map type, this should implement a starts_with check (similar to how
         // ConfigMapAccess does).
-        if let Some(v) = self.config.env.get(&self.key.to_env()) {
+        if let Some(v) = self.config.env.get(self.key.as_env_key()) {
             let res: Result<V::Value, ConfigError> = if v == "true" || v == "false" {
                 visitor.visit_bool(v.parse().unwrap())
             } else if let Ok(v) = v.parse::<i64>() {
@@ -48,38 +50,34 @@ impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> {
                 && v.starts_with('[')
                 && v.ends_with(']')
             {
-                visitor.visit_seq(ConfigSeqAccess::new(self.config, &self.key)?)
+                visitor.visit_seq(ConfigSeqAccess::new(self.clone())?)
             } else {
                 visitor.visit_string(v.clone())
             };
             return res.map_err(|e| {
                 e.with_key_context(
-                    &self.key.to_config(),
-                    Definition::Environment(self.key.to_env()),
+                    &self.key,
+                    Definition::Environment(self.key.as_env_key().to_string()),
                 )
             });
         }
 
-        let o_cv = self.config.get_cv(&self.key.to_config())?;
+        let o_cv = self.config.get_cv(self.key.as_config_key())?;
         if let Some(cv) = o_cv {
             let res: (Result<V::Value, ConfigError>, PathBuf) = match cv {
                 CV::Integer(i, path) => (visitor.visit_i64(i), path),
                 CV::String(s, path) => (visitor.visit_string(s), path),
-                CV::List(_, path) => (
-                    visitor.visit_seq(ConfigSeqAccess::new(self.config, &self.key)?),
-                    path,
-                ),
+                CV::List(_, path) => (visitor.visit_seq(ConfigSeqAccess::new(self.clone())?), path),
                 CV::Table(_, path) => (
-                    visitor.visit_map(ConfigMapAccess::new_map(self.config, self.key.clone())?),
+                    visitor.visit_map(ConfigMapAccess::new_map(self.clone())?),
                     path,
                 ),
                 CV::Boolean(b, path) => (visitor.visit_bool(b), path),
             };
             let (res, path) = res;
-            return res
-                .map_err(|e| e.with_key_context(&self.key.to_config(), Definition::Path(path)));
+            return res.map_err(|e| e.with_key_context(&self.key, Definition::Path(path)));
         }
-        Err(ConfigError::missing(&self.key.to_config()))
+        Err(ConfigError::missing(&self.key))
     }
 
     deserialize_method!(deserialize_bool, visit_bool, get_bool_priv);
@@ -107,35 +105,41 @@ impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> {
 
     fn deserialize_struct<V>(
         self,
-        _name: &'static str,
+        name: &'static str,
         fields: &'static [&'static str],
         visitor: V,
     ) -> Result<V::Value, Self::Error>
     where
         V: de::Visitor<'de>,
     {
-        visitor.visit_map(ConfigMapAccess::new_struct(self.config, self.key, fields)?)
+        if name == value::NAME && fields == value::FIELDS {
+            return visitor.visit_map(ValueDeserializer {
+                hits: 0,
+                deserializer: self,
+            });
+        }
+        visitor.visit_map(ConfigMapAccess::new_struct(self, fields)?)
     }
 
     fn deserialize_map<V>(self, visitor: V) -> Result<V::Value, Self::Error>
     where
         V: de::Visitor<'de>,
     {
-        visitor.visit_map(ConfigMapAccess::new_map(self.config, self.key)?)
+        visitor.visit_map(ConfigMapAccess::new_map(self)?)
     }
 
     fn deserialize_seq<V>(self, visitor: V) -> Result<V::Value, Self::Error>
     where
         V: de::Visitor<'de>,
     {
-        visitor.visit_seq(ConfigSeqAccess::new(self.config, &self.key)?)
+        visitor.visit_seq(ConfigSeqAccess::new(self)?)
     }
 
     fn deserialize_tuple<V>(self, _len: usize, visitor: V) -> Result<V::Value, Self::Error>
     where
         V: de::Visitor<'de>,
     {
-        visitor.visit_seq(ConfigSeqAccess::new(self.config, &self.key)?)
+        visitor.visit_seq(ConfigSeqAccess::new(self)?)
     }
 
     fn deserialize_tuple_struct<V>(
@@ -147,7 +151,7 @@ impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> {
     where
         V: de::Visitor<'de>,
     {
-        visitor.visit_seq(ConfigSeqAccess::new(self.config, &self.key)?)
+        visitor.visit_seq(ConfigSeqAccess::new(self)?)
     }
 
     fn deserialize_newtype_struct<V>(
@@ -169,7 +173,7 @@ impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> {
                         .to_string();
                     visitor.visit_newtype_struct(path.into_deserializer())
                 }
-                None => Err(ConfigError::missing(&self.key.to_config())),
+                None => Err(ConfigError::missing(&self.key)),
             }
         } else {
             visitor.visit_newtype_struct(self)
@@ -185,70 +189,76 @@ impl<'de, 'config> de::Deserializer<'de> for Deserializer<'config> {
 }
 
 struct ConfigMapAccess<'config> {
-    config: &'config Config,
-    key: ConfigKey,
-    set_iter: <HashSet<ConfigKeyPart> as IntoIterator>::IntoIter,
-    next: Option<ConfigKeyPart>,
+    de: Deserializer<'config>,
+    set_iter: <HashSet<KeyKind> as IntoIterator>::IntoIter,
+    next: Option<KeyKind>,
+}
+
+#[derive(PartialEq, Eq, Hash)]
+enum KeyKind {
+    Normal(String),
+    CaseSensitive(String),
 }
 
 impl<'config> ConfigMapAccess<'config> {
-    fn new_map(
-        config: &'config Config,
-        key: ConfigKey,
-    ) -> Result<ConfigMapAccess<'config>, ConfigError> {
+    fn new_map(de: Deserializer<'config>) -> Result<ConfigMapAccess<'config>, ConfigError> {
         let mut set = HashSet::new();
-        if let Some(mut v) = config.get_table(&key.to_config())? {
+        if let Some(mut v) = de.config.get_table(de.key.as_config_key())? {
             // `v: Value<HashMap<String, CV>>`
             for (key, _value) in v.val.drain() {
-                set.insert(ConfigKeyPart::CasePart(key));
+                set.insert(KeyKind::CaseSensitive(key));
             }
         }
-        if config.cli_unstable().advanced_env {
+        if de.config.cli_unstable().advanced_env {
             // `CARGO_PROFILE_DEV_OVERRIDES_`
-            let env_pattern = format!("{}_", key.to_env());
-            for env_key in config.env.keys() {
+            let env_pattern = format!("{}_", de.key.as_env_key());
+            for env_key in de.config.env.keys() {
                 if env_key.starts_with(&env_pattern) {
                     // `CARGO_PROFILE_DEV_OVERRIDES_bar_OPT_LEVEL = 3`
                     let rest = &env_key[env_pattern.len()..];
                     // `rest = bar_OPT_LEVEL`
                     let part = rest.splitn(2, '_').next().unwrap();
                     // `part = "bar"`
-                    set.insert(ConfigKeyPart::CasePart(part.to_string()));
+                    set.insert(KeyKind::CaseSensitive(part.to_string()));
                 }
             }
         }
         Ok(ConfigMapAccess {
-            config,
-            key,
+            de,
             set_iter: set.into_iter(),
             next: None,
         })
     }
 
     fn new_struct(
-        config: &'config Config,
-        key: ConfigKey,
+        de: Deserializer<'config>,
         fields: &'static [&'static str],
     ) -> Result<ConfigMapAccess<'config>, ConfigError> {
         let mut set = HashSet::new();
         for field in fields {
-            set.insert(ConfigKeyPart::Part(field.to_string()));
+            set.insert(KeyKind::Normal(field.to_string()));
         }
-        if let Some(mut v) = config.get_table(&key.to_config())? {
+
+        // 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.as_config_key())? {
             for (t_key, value) in v.val.drain() {
-                let part = ConfigKeyPart::Part(t_key);
-                if !set.contains(&part) {
-                    config.shell().warn(format!(
-                        "unused key `{}` in config file `{}`",
-                        key.join(part).to_config(),
-                        value.definition_path().display()
-                    ))?;
+                if set.contains(&KeyKind::Normal(t_key.to_string())) {
+                    continue;
                 }
+                de.config.shell().warn(format!(
+                    "unused key `{}.{}` in config file `{}`",
+                    de.key.as_config_key(),
+                    t_key,
+                    value.definition_path().display()
+                ))?;
             }
         }
+
         Ok(ConfigMapAccess {
-            config,
-            key,
+            de,
             set_iter: set.into_iter(),
             next: None,
         })
@@ -264,9 +274,12 @@ impl<'de, 'config> de::MapAccess<'de> for ConfigMapAccess<'config> {
     {
         match self.set_iter.next() {
             Some(key) => {
-                let de_key = key.to_config();
+                let name = match &key {
+                    KeyKind::Normal(s) | KeyKind::CaseSensitive(s) => s.as_str(),
+                };
+                let result = seed.deserialize(name.into_deserializer()).map(Some);
                 self.next = Some(key);
-                seed.deserialize(de_key.into_deserializer()).map(Some)
+                return result;
             }
             None => Ok(None),
         }
@@ -276,12 +289,16 @@ impl<'de, 'config> de::MapAccess<'de> for ConfigMapAccess<'config> {
     where
         V: de::DeserializeSeed<'de>,
     {
-        let next_key = self.next.take().expect("next field missing");
-        let next_key = self.key.join(next_key);
-        seed.deserialize(Deserializer {
-            config: self.config,
-            key: next_key,
-        })
+        match self.next.take().expect("next field missing") {
+            KeyKind::Normal(key) => self.de.key.push(&key),
+            KeyKind::CaseSensitive(key) => self.de.key.push_sensitive(&key),
+        }
+        let result = seed.deserialize(Deserializer {
+            config: self.de.config,
+            key: self.de.key.clone(),
+        });
+        self.de.key.pop();
+        return result;
     }
 }
 
@@ -290,34 +307,32 @@ struct ConfigSeqAccess {
 }
 
 impl ConfigSeqAccess {
-    fn new(config: &Config, key: &ConfigKey) -> Result<ConfigSeqAccess, ConfigError> {
+    fn new(de: Deserializer<'_>) -> Result<ConfigSeqAccess, ConfigError> {
         let mut res = Vec::new();
-        if let Some(v) = config.get_list(&key.to_config())? {
+        if let Some(v) = de.config.get_list(de.key.as_config_key())? {
             for (s, path) in v.val {
                 res.push((s, Definition::Path(path)));
             }
         }
 
-        if config.cli_unstable().advanced_env {
+        if de.config.cli_unstable().advanced_env {
             // Parse an environment string as a TOML array.
-            let env_key = key.to_env();
-            let def = Definition::Environment(env_key.clone());
-            if let Some(v) = config.env.get(&env_key) {
+            if let Some(v) = de.config.env.get(de.key.as_env_key()) {
+                let def = Definition::Environment(de.key.as_env_key().to_string());
                 if !(v.starts_with('[') && v.ends_with(']')) {
                     return Err(ConfigError::new(
                         format!("should have TOML list syntax, found `{}`", v),
                         def,
                     ));
                 }
-                let temp_key = key.last().to_env();
-                let toml_s = format!("{}={}", temp_key, v);
+                let toml_s = format!("value={}", v);
                 let toml_v: toml::Value = toml::de::from_str(&toml_s).map_err(|e| {
                     ConfigError::new(format!("could not parse TOML list: {}", e), def.clone())
                 })?;
                 let values = toml_v
                     .as_table()
                     .unwrap()
-                    .get(&temp_key)
+                    .get("value")
                     .unwrap()
                     .as_array()
                     .expect("env var was not array");
@@ -353,3 +368,47 @@ impl<'de> de::SeqAccess<'de> for ConfigSeqAccess {
         }
     }
 }
+
+struct ValueDeserializer<'config> {
+    hits: u32,
+    deserializer: Deserializer<'config>,
+}
+
+impl<'de, 'config> de::MapAccess<'de> for ValueDeserializer<'config> {
+    type Error = ConfigError;
+
+    fn next_key_seed<K>(&mut self, seed: K) -> Result<Option<K::Value>, Self::Error>
+    where
+        K: de::DeserializeSeed<'de>,
+    {
+        self.hits += 1;
+        match self.hits {
+            1 => seed
+                .deserialize(value::VALUE_FIELD.into_deserializer())
+                .map(Some),
+            2 => seed
+                .deserialize(value::DEFINITION_FIELD.into_deserializer())
+                .map(Some),
+            _ => Ok(None),
+        }
+    }
+
+    fn next_value_seed<V>(&mut self, seed: V) -> Result<V::Value, Self::Error>
+    where
+        V: de::DeserializeSeed<'de>,
+    {
+        if self.hits == 1 {
+            seed.deserialize(Deserializer {
+                config: self.deserializer.config,
+                key: self.deserializer.key.clone(),
+            })
+        } else {
+            // let env = self.deserializer.key.to_env();
+            // if self.deserializer.config.env.contains_key(&env) {
+            // } else {
+            // }
+            // if let someself.deserializer.config.get_env(&self.deserializer.key)
+            panic!()
+        }
+    }
+}