Skip to content

Commit

Permalink
fix(config): unrecognized config properties don't cause config error (#…
Browse files Browse the repository at this point in the history
…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>
  • Loading branch information
brahmlower and davidkna committed Nov 20, 2022
1 parent c2d3845 commit 1b03ef2
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 4 deletions.
30 changes: 29 additions & 1 deletion src/config.rs
Expand Up @@ -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)
}
})
}
}

Expand Down Expand Up @@ -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"));
Expand Down
34 changes: 31 additions & 3 deletions src/serde_utils.rs
Expand Up @@ -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.
Expand All @@ -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
}
}
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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]
Expand Down

0 comments on commit 1b03ef2

Please sign in to comment.