From 1b03ef21f34fc4acf890b01cfca3d07158ef5c46 Mon Sep 17 00:00:00 2001
From: Brahm Lower <bplower@gmail.com>
Date: Sun, 20 Nov 2022 09:27:48 -0800
Subject: [PATCH] fix(config): unrecognized config properties don't cause
 config error (#4547)

* Fix #4481, config does not error when unrecognized properties are present

* cleanup: use stuct update syntax to improve readability

from review feedback

Co-authored-by: David Knaack <davidkna@users.noreply.github.com>

* cleanup: renamed ValueDeserializer func w/ better name

* cleanup: added test to cover unknown key retry condition

Co-authored-by: David Knaack <davidkna@users.noreply.github.com>
---
 src/config.rs      | 30 +++++++++++++++++++++++++++++-
 src/serde_utils.rs | 34 +++++++++++++++++++++++++++++++---
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/src/config.rs b/src/config.rs
index 35cb8656..e0aa016a 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -48,7 +48,17 @@ impl<'a, T: Deserialize<'a> + Default> ModuleConfig<'a, ValueError> for T {
     /// Create `ValueDeserializer` wrapper and use it to call `Deserialize::deserialize` on it.
     fn from_config(config: &'a Value) -> Result<Self, ValueError> {
         let deserializer = ValueDeserializer::new(config);
-        T::deserialize(deserializer)
+        T::deserialize(deserializer).or_else(|err| {
+            // If the error is an unrecognized key, print a warning and run
+            // deserialize ignoring that error. Otherwise, just return the error
+            if err.to_string().contains("Unknown key") {
+                log::warn!("{}", err);
+                let deserializer2 = ValueDeserializer::new(config).with_allow_unknown_keys();
+                T::deserialize(deserializer2)
+            } else {
+                Err(err)
+            }
+        })
     }
 }
 
@@ -582,6 +592,24 @@ mod tests {
         assert_eq!(rust_config.switch_c, Switch::Off);
     }
 
+    #[test]
+    fn test_load_unknown_key_config() {
+        #[derive(Clone, Default, Deserialize)]
+        #[serde(default)]
+        struct TestConfig<'a> {
+            pub foo: &'a str,
+        }
+
+        let config = toml::toml! {
+            foo = "test"
+            bar = "ignore me"
+        };
+        let rust_config = TestConfig::from_config(&config);
+
+        assert!(rust_config.is_ok());
+        assert_eq!(rust_config.unwrap().foo, "test");
+    }
+
     #[test]
     fn test_from_string() {
         let config = Value::String(String::from("S"));
diff --git a/src/serde_utils.rs b/src/serde_utils.rs
index c1c38384..b47c02cd 100644
--- a/src/serde_utils.rs
+++ b/src/serde_utils.rs
@@ -13,6 +13,7 @@ pub struct ValueDeserializer<'de> {
     value: &'de Value,
     info: Option<StructInfo>,
     current_key: Option<&'de str>,
+    error_on_ignored: bool,
 }
 
 /// When deserializing a struct, this struct stores information about the struct.
@@ -28,14 +29,28 @@ impl<'de> ValueDeserializer<'de> {
             value,
             info: None,
             current_key: None,
+            error_on_ignored: true,
         }
     }
 
-    fn with_info(value: &'de Value, info: Option<StructInfo>, current_key: &'de str) -> Self {
+    fn with_info(
+        value: &'de Value,
+        info: Option<StructInfo>,
+        current_key: &'de str,
+        ignored: bool,
+    ) -> Self {
         ValueDeserializer {
             value,
             info,
             current_key: Some(current_key),
+            error_on_ignored: ignored,
+        }
+    }
+
+    pub fn with_allow_unknown_keys(self) -> Self {
+        ValueDeserializer {
+            error_on_ignored: false,
+            ..self
         }
     }
 }
@@ -83,7 +98,12 @@ impl<'de> Deserializer<'de> for ValueDeserializer<'de> {
                 let map = MapDeserializer::new(t.iter().map(|(k, v)| {
                     (
                         k.as_str(),
-                        ValueDeserializer::with_info(v, self.info, k.as_str()),
+                        ValueDeserializer::with_info(
+                            v,
+                            self.info,
+                            k.as_str(),
+                            self.error_on_ignored,
+                        ),
                     )
                 }));
                 map.deserialize_map(visitor)
@@ -131,6 +151,10 @@ impl<'de> Deserializer<'de> for ValueDeserializer<'de> {
             return visitor.visit_none();
         }
 
+        if !self.error_on_ignored {
+            return visitor.visit_none();
+        }
+
         let did_you_mean = match (self.current_key, self.info) {
             (Some(key), Some(StructInfo { fields, .. })) => fields
                 .iter()
@@ -322,13 +346,17 @@ mod test {
         let value = toml::toml! {
             unknown_key = "foo"
         };
-        let deserializer = ValueDeserializer::new(&value);
 
+        let deserializer = ValueDeserializer::new(&value);
         let result = StarshipRootConfig::deserialize(deserializer).unwrap_err();
         assert_eq!(
             format!("{result}"),
             "Error in 'StarshipRoot' at 'unknown_key': Unknown key"
         );
+
+        let deserializer2 = ValueDeserializer::new(&value).with_allow_unknown_keys();
+        let result = StarshipRootConfig::deserialize(deserializer2);
+        assert!(result.is_ok());
     }
 
     #[test]