Skip to content

Commit

Permalink
fix: Add more context when 'missing feild'
Browse files Browse the repository at this point in the history
  • Loading branch information
linyihai committed May 24, 2024
1 parent 48742e0 commit e05d930
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 14 deletions.
32 changes: 23 additions & 9 deletions src/cargo/util/context/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ macro_rules! deserialize_method {
.ok_or_else(|| 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, definition))
res.map_err(|e| e.with_key_context(&self.key, Some(definition)))
}
};
}
Expand All @@ -60,7 +60,7 @@ impl<'de, 'gctx> de::Deserializer<'de> for Deserializer<'gctx> {
CV::Boolean(b, def) => (visitor.visit_bool(b), def),
};
let (res, def) = res;
return res.map_err(|e| e.with_key_context(&self.key, def));
return res.map_err(|e| e.with_key_context(&self.key, Some(def)));
}
Err(ConfigError::missing(&self.key))
}
Expand Down Expand Up @@ -178,7 +178,7 @@ impl<'de, 'gctx> de::Deserializer<'de> for Deserializer<'gctx> {
let Value { val, definition } = value;
visitor
.visit_enum(val.into_deserializer())
.map_err(|e: ConfigError| e.with_key_context(&self.key, definition))
.map_err(|e: ConfigError| e.with_key_context(&self.key, Some(definition)))
}

// These aren't really supported, yet.
Expand Down Expand Up @@ -345,11 +345,25 @@ impl<'de, 'gctx> de::MapAccess<'de> for ConfigMapAccess<'gctx> {
field.replace('-', "_").starts_with(&env_prefix)
});

let result = seed.deserialize(Deserializer {
gctx: self.de.gctx,
key: self.de.key.clone(),
env_prefix_ok,
});
let result = seed
.deserialize(Deserializer {
gctx: self.de.gctx,
key: self.de.key.clone(),
env_prefix_ok,
})
.map_err(|e| {
if !e.is_missing_field() {
return e;
}
e.with_key_context(
&self.de.key,
self.de
.gctx
.get_cv_with_env(&self.de.key)
.ok()
.and_then(|cv| cv.map(|cv| cv.get_definition().clone())),
)
});
self.de.key.pop();
result
}
Expand Down Expand Up @@ -486,7 +500,7 @@ impl<'de, 'gctx> de::MapAccess<'de> for ValueDeserializer<'gctx> {
if let Some(de) = &self.de {
return seed
.deserialize(de.clone())
.map_err(|e| e.with_key_context(&de.key, self.definition.clone()));
.map_err(|e| e.with_key_context(&de.key, Some(self.definition.clone())));
} else {
return seed
.deserialize(self.str_value.as_ref().unwrap().clone().into_deserializer());
Expand Down
36 changes: 34 additions & 2 deletions src/cargo/util/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2030,18 +2030,22 @@ impl ConfigError {
}
}

fn is_missing_field(&self) -> bool {
self.error.downcast_ref::<MissingField>().is_some()
}

fn missing(key: &ConfigKey) -> ConfigError {
ConfigError {
error: anyhow!("missing config key `{}`", key),
definition: None,
}
}

fn with_key_context(self, key: &ConfigKey, definition: Definition) -> ConfigError {
fn with_key_context(self, key: &ConfigKey, definition: Option<Definition>) -> ConfigError {
ConfigError {
error: anyhow::Error::from(self)
.context(format!("could not load config key `{}`", key)),
definition: Some(definition),
definition: definition,
}
}
}
Expand All @@ -2062,13 +2066,31 @@ impl fmt::Display for ConfigError {
}
}

#[derive(Debug)]
struct MissingField(String);

impl fmt::Display for MissingField {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "missing field `{}`", self.0)
}
}

impl std::error::Error for MissingField {}

impl serde::de::Error for ConfigError {
fn custom<T: fmt::Display>(msg: T) -> Self {
ConfigError {
error: anyhow::Error::msg(msg.to_string()),
definition: None,
}
}

fn missing_field(field: &'static str) -> Self {
ConfigError {
error: anyhow::Error::new(MissingField(field.to_string())),
definition: None,
}
}
}

impl From<anyhow::Error> for ConfigError {
Expand Down Expand Up @@ -2111,6 +2133,16 @@ impl fmt::Debug for ConfigValue {
}

impl ConfigValue {
fn get_definition(&self) -> &Definition {
match self {
CV::Boolean(_, def)
| CV::Integer(_, def)
| CV::String(_, def)
| CV::List(_, def)
| CV::Table(_, def) => def,
}
}

fn from_toml(def: Definition, toml: toml::Value) -> CargoResult<ConfigValue> {
match toml {
toml::Value::String(val) => Ok(CV::String(val, def)),
Expand Down
18 changes: 16 additions & 2 deletions tests/testsuite/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1880,7 +1880,14 @@ fn missing_fields() {
let gctx = GlobalContextBuilder::new()
.env("CARGO_FOO_BAR_BAZ", "true")
.build();
assert_error(gctx.get::<Foo>("foo").unwrap_err(), "missing field `bax`");
assert_error(
gctx.get::<Foo>("foo").unwrap_err(),
"\
could not load config key `foo.bar`
Caused by:
missing field `bax`",
);
let gctx: GlobalContext = GlobalContextBuilder::new()
.env("CARGO_FOO_BAR_BAZ", "true")
.env("CARGO_FOO_BAR_BAX", "true")
Expand All @@ -1892,5 +1899,12 @@ fn missing_fields() {
let gctx: GlobalContext = GlobalContextBuilder::new()
.config_arg("foo.bar.baz=true")
.build();
assert_error(gctx.get::<Foo>("foo").unwrap_err(), "missing field `bax`");
assert_error(
gctx.get::<Foo>("foo").unwrap_err(),
"\
error in --config cli option: could not load config key `foo.bar`
Caused by:
missing field `bax`",
);
}
5 changes: 4 additions & 1 deletion tests/testsuite/progress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ fn bad_progress_config_missing_when() {
.with_status(101)
.with_stderr(
"\
error: missing field `when`
error: error in [..]: could not load config key `term.progress`
Caused by:
missing field `when`
",
)
.run();
Expand Down

0 comments on commit e05d930

Please sign in to comment.