Auto merge of #13956 - linyihai:missing-fields, r=weihanglo

Improve error description when deserializing partial field struct

### What does this PR try to resolve?

Fixes #13688

### How should we test and review this PR?
one commit add test, one commit fixed and update the test.

### Additional information

Author: bors

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,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
     }
@@ -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());

src/cargo/util/context/mod.rs

@@ -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,
         }
     }
 }
@@ -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 {
@@ -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)),

tests/testsuite/config.rs

@@ -1863,3 +1863,48 @@ fn trim_paths_parsing() {
     let trim_paths: TomlTrimPaths = gctx.get("profile.dev.trim-paths").unwrap();
     assert_eq!(trim_paths, expected, "failed to parse {val}");
 }
+
+#[cargo_test]
+fn missing_fields() {
+    #[derive(Deserialize, Default, Debug)]
+    struct Foo {
+        bar: Bar,
+    }
+
+    #[derive(Deserialize, Default, Debug)]
+    struct Bar {
+        bax: bool,
+        baz: bool,
+    }
+
+    let gctx = GlobalContextBuilder::new()
+        .env("CARGO_FOO_BAR_BAZ", "true")
+        .build();
+    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")
+        .build();
+    let foo = gctx.get::<Foo>("foo").unwrap();
+    assert_eq!(foo.bar.bax, true);
+    assert_eq!(foo.bar.baz, true);
+
+    let gctx: GlobalContext = GlobalContextBuilder::new()
+        .config_arg("foo.bar.baz=true")
+        .build();
+    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`",
+    );
+}

tests/testsuite/progress.rs

@@ -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();