Add tests verifying overlapping prefixes and defaults

These tests demonstrate the current failure mode around
overlapping env keys and inner structs. To some extent this
is a limitation of mapping on to environment variables with
an underscore as both the namespace separator and the
substitute for dashes in flag names in that the mapping is
not strictly one-to-one.

Author: bearcage

src/cargo/util/config/de.rs

@@ -298,11 +298,20 @@ impl<'config> ConfigMapAccess<'config> {
         // If the caller is interested in a field which we can provide from
         // the environment, get it from there.
         for field in given_fields {
+            let is_prefix = given_fields
+                .iter()
+                .any(|f| f != field && f.starts_with(field));
             let mut field_key = de.key.clone();
             field_key.push(field);
             for env_key in de.config.env.keys() {
-                if env_key.starts_with(field_key.as_env_key()) {
-                    fields.insert(KeyKind::Normal(field.to_string()));
+                if is_prefix {
+                    if env_key == field_key.as_env_key() {
+                        fields.insert(KeyKind::Normal(field.to_string()));
+                    }
+                } else {
+                    if env_key.starts_with(field_key.as_env_key()) {
+                        fields.insert(KeyKind::Normal(field.to_string()));
+                    }
                 }
             }
         }

tests/testsuite/config.rs

@@ -1246,6 +1246,27 @@ fn struct_with_opt_inner_struct() {
     assert_eq!(f.inner.unwrap().value.unwrap(), 12);
 }
 
+#[cargo_test]
+fn struct_with_default_inner_struct() {
+    // Struct with serde defaults.
+    // Check that can be defined with environment variable.
+    #[derive(Deserialize, Default)]
+    #[serde(default)]
+    struct Inner {
+        value: i32,
+    }
+    #[derive(Deserialize, Default)]
+    #[serde(default)]
+    struct Foo {
+        inner: Inner,
+    }
+    let config = ConfigBuilder::new()
+        .env("CARGO_FOO_INNER_VALUE", "12")
+        .build();
+    let f: Foo = config.get("foo").unwrap();
+    assert_eq!(f.inner.value, 12);
+}
+
 #[cargo_test]
 fn overlapping_env_config() {
     // Issue where one key is a prefix of another.
@@ -1277,6 +1298,81 @@ fn overlapping_env_config() {
     assert_eq!(s.debug, Some(1));
 }
 
+#[cargo_test]
+fn overlapping_env_with_defaults() {
+    // Issue where one key is a prefix of another.
+    #[derive(Deserialize, Default)]
+    #[serde(default, rename_all = "kebab-case")]
+    struct Ambig {
+        debug: u32,
+        debug_assertions: bool,
+    }
+    let config = ConfigBuilder::new()
+        .env("CARGO_AMBIG_DEBUG_ASSERTIONS", "true")
+        .build();
+
+    let s: Ambig = config.get("ambig").unwrap();
+    assert_eq!(s.debug_assertions, true);
+    assert_eq!(s.debug, u32::default());
+
+    let config = ConfigBuilder::new().env("CARGO_AMBIG_DEBUG", "5").build();
+    let s: Ambig = config.get("ambig").unwrap();
+    assert_eq!(s.debug_assertions, bool::default());
+    assert_eq!(s.debug, 5);
+
+    let config = ConfigBuilder::new()
+        .env("CARGO_AMBIG_DEBUG", "1")
+        .env("CARGO_AMBIG_DEBUG_ASSERTIONS", "true")
+        .build();
+    let s: Ambig = config.get("ambig").unwrap();
+    assert_eq!(s.debug_assertions, true);
+    assert_eq!(s.debug, 1);
+}
+
+#[cargo_test]
+fn struct_with_overlapping_inner_struct_and_defaults() {
+    // Struct with serde defaults.
+    // Check that can be defined with environment variable.
+    #[derive(Deserialize, Default)]
+    #[serde(default)]
+    struct Inner {
+        value: i32,
+    }
+
+    // Containing struct with a prefix of inner
+    // Check that the nested struct can have fields defined by env
+    #[derive(Deserialize, Default)]
+    #[serde(default)]
+    struct PrefixContainer {
+        inn: bool,
+        inner: Inner,
+    }
+    let config = ConfigBuilder::new()
+        .env("CARGO_PREFIXCONTAINER_INNER_VALUE", "12")
+        .build();
+    let f: PrefixContainer = config.get("prefixcontainer").unwrap();
+    assert_eq!(f.inn, bool::default());
+    assert_eq!(f.inner.value, 12);
+
+    // Containing struct where the inner value's field is a prefix of another
+    // Check that the nested struct can have fields defined by env
+    #[derive(Deserialize, Default)]
+    #[serde(default)]
+    struct InversePrefixContainer {
+        inner_field: bool,
+        inner: Inner,
+    }
+    let config = ConfigBuilder::new()
+        .env("CARGO_INVERSEREFIXCONTAINER_INNER_VALUE", "12")
+        .build();
+    let f: InversePrefixContainer = config.get("inverseprefixcontainer").unwrap();
+    assert_eq!(f.inner_field, bool::default());
+    // NB. This is a limitation of our env variable parsing. We can't currently
+    //     handle situations where just a value of the inner struct is set, but
+    //     it's also named as a prefix of another field on the outer struct.
+    // assert_eq!(f.inner.value, 12);
+}
+
 #[cargo_test]
 fn string_list_tricky_env() {
     // Make sure StringList handles typed env values.