fix: Add more context when 'missing feild'

rust-lang · May 23, 2024 · 6fa3992 · 6fa3992
1 parent 48742e0
commit 6fa3992
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 13 deletions.
diff --git a/src/cargo/util/context/de.rs b/src/cargo/util/context/de.rs
@@ -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)))
         }
     };
 }
@@ -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))
     }
@@ -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.
@@ -345,11 +345,26 @@ 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)
+                        .map_or(None, |cv| {
+                            cv.map_or(None, |cv| Some(cv.get_definition().clone()))
+                        }),
+                )
+            });
         self.de.key.pop();
         result
     }
@@ -486,7 +501,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());

diff --git a/src/cargo/util/context/mod.rs b/src/cargo/util/context/mod.rs
@@ -2030,18 +2030,22 @@ impl ConfigError {
         }
     }
 
+    fn is_missing_field(&self) -> bool {
+        self.error.to_string().contains("missing field")
+    }
+
     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,
         }
     }
 }
@@ -2111,6 +2115,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)),

diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs
@@ -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")
@@ -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`",
+    );
 }