refactor: switch to Vec<ConfigValue> for ConfigValue::List

weihanglo · weihanglo · commit 185002901f3b · 2025-10-10T10:58:32.000-04:00
This is a preparation for supporting list of types other than string.
diff --git a/src/cargo/ops/cargo_config.rs b/src/cargo/ops/cargo_config.rs
@@ -128,7 +128,13 @@ fn print_toml(gctx: &GlobalContext, opts: &GetOptions<'_>, key: &ConfigKey, cv:
         CV::List(vals, _def) => {
             if opts.show_origin {
                 drop_println!(gctx, "{} = [", key);
-                for (val, def) in vals {
+                for cv in vals {
+                    let (val, def) = match cv {
+                        CV::String(s, def) => (s.as_str(), def),
+                        // This is actually unreachable until we start supporting list of different types.
+                        // It should be validated already during the deserialization.
+                        v => todo!("support {} type ", v.desc()),
+                    };
                     drop_println!(
                         gctx,
                         "    {}, # {}",
@@ -139,7 +145,15 @@ fn print_toml(gctx: &GlobalContext, opts: &GetOptions<'_>, key: &ConfigKey, cv:
                 }
                 drop_println!(gctx, "]");
             } else {
-                let vals: toml_edit::Array = vals.iter().map(|x| &x.0).collect();
+                let vals: toml_edit::Array = vals
+                    .iter()
+                    .map(|cv| match cv {
+                        CV::String(s, _) => toml_edit::Value::from(s.as_str()),
+                        // This is actually unreachable until we start supporting list of different types.
+                        // It should be validated already during the deserialization.
+                        v => todo!("support {} type ", v.desc()),
+                    })
+                    .collect();
                 drop_println!(gctx, "{} = {}", key, vals);
             }
         }
@@ -204,7 +218,7 @@ fn print_json(gctx: &GlobalContext, key: &ConfigKey, cv: &CV, include_key: bool)
             CV::Integer(val, _def) => json!(val),
             CV::String(val, _def) => json!(val),
             CV::List(vals, _def) => {
-                let jvals: Vec<_> = vals.iter().map(|(val, _def)| json!(val)).collect();
+                let jvals: Vec<_> = vals.iter().map(cv_to_json).collect();
                 json!(jvals)
             }
             CV::Table(map, _def) => {
diff --git a/src/cargo/util/context/de.rs b/src/cargo/util/context/de.rs
@@ -160,7 +160,9 @@ impl<'de, 'gctx> de::Deserializer<'de> for Deserializer<'gctx> {
             match self.gctx.get_cv(&self.key)? {
                 Some(CV::List(val, _def)) => res.extend(val),
                 Some(CV::String(val, def)) => {
-                    let split_vs = val.split_whitespace().map(|s| (s.to_string(), def.clone()));
+                    let split_vs = val
+                        .split_whitespace()
+                        .map(|s| CV::String(s.to_string(), def.clone()));
                     res.extend(split_vs);
                 }
                 Some(val) => {
@@ -172,7 +174,13 @@ impl<'de, 'gctx> de::Deserializer<'de> for Deserializer<'gctx> {
 
             self.gctx.get_env_list(&self.key, &mut res)?;
 
-            let vals: Vec<String> = res.into_iter().map(|vd| vd.0).collect();
+            let vals: Vec<String> = res
+                .into_iter()
+                .map(|val| match val {
+                    CV::String(s, _defintion) => Ok(s),
+                    other => Err(ConfigError::expected(&self.key, "string", &other)),
+                })
+                .collect::<Result<_, _>>()?;
             visitor.visit_newtype_struct(vals.into_deserializer())
         } else {
             visitor.visit_newtype_struct(self)
@@ -396,7 +404,9 @@ impl<'de, 'gctx> de::MapAccess<'de> for ConfigMapAccess<'gctx> {
 }
 
 struct ConfigSeqAccess {
-    list_iter: vec::IntoIter<(String, Definition)>,
+    /// The config key to the sequence.
+    key: ConfigKey,
+    list_iter: vec::IntoIter<CV>,
 }
 
 impl ConfigSeqAccess {
@@ -416,6 +426,7 @@ impl ConfigSeqAccess {
         de.gctx.get_env_list(&de.key, &mut res)?;
 
         Ok(ConfigSeqAccess {
+            key: de.key,
             list_iter: res.into_iter(),
         })
     }
@@ -430,12 +441,13 @@ impl<'de> de::SeqAccess<'de> for ConfigSeqAccess {
     {
         match self.list_iter.next() {
             // TODO: add `def` to error?
-            Some((value, def)) => {
+            Some(CV::String(value, def)) => {
                 // This might be a String or a Value<String>.
                 // ValueDeserializer will handle figuring out which one it is.
                 let maybe_value_de = ValueDeserializer::new_with_string(value, def);
                 seed.deserialize(maybe_value_de).map(Some)
             }
+            Some(val) => Err(ConfigError::expected(&self.key, "list of string", &val)),
             None => Ok(None),
         }
     }
diff --git a/src/cargo/util/context/mod.rs b/src/cargo/util/context/mod.rs
@@ -989,11 +989,7 @@ impl GlobalContext {
 
     /// Internal method for getting an environment variable as a list.
     /// If the key is a non-mergeable list and a value is found in the environment, existing values are cleared.
-    fn get_env_list(
-        &self,
-        key: &ConfigKey,
-        output: &mut Vec<(String, Definition)>,
-    ) -> CargoResult<()> {
+    fn get_env_list(&self, key: &ConfigKey, output: &mut Vec<ConfigValue>) -> CargoResult<()> {
         let Some(env_val) = self.env.get_str(key.as_env_key()) else {
             self.check_environment_key_case_mismatch(key);
             return Ok(());
@@ -1018,16 +1014,16 @@ impl GlobalContext {
                         def.clone(),
                     )
                 })?;
-                output.push((s.to_string(), def.clone()));
+                output.push(CV::String(s.to_string(), def.clone()))
             }
         } else {
             output.extend(
                 env_val
                     .split_whitespace()
-                    .map(|s| (s.to_string(), def.clone())),
+                    .map(|s| CV::String(s.to_string(), def.clone())),
             );
         }
-        output.sort_by(|a, b| a.1.cmp(&b.1));
+        output.sort_by(|a, b| a.definition().cmp(b.definition()));
         Ok(())
     }
 
@@ -1386,7 +1382,16 @@ impl GlobalContext {
             Some(CV::String(s, def)) => {
                 vec![abs(s, def)]
             }
-            Some(CV::List(list, _def)) => list.iter().map(|(s, def)| abs(s, def)).collect(),
+            Some(CV::List(list, _def)) => list
+                .iter()
+                .map(|cv| match cv {
+                    CV::String(s, def) => Ok(abs(s, def)),
+                    other => bail!(
+                        "`include` expected a string or list of strings, but found {} in list",
+                        other.desc()
+                    ),
+                })
+                .collect::<CargoResult<Vec<_>>>()?,
             Some(other) => bail!(
                 "`include` expected a string or list, but found {} in `{}`",
                 other.desc(),
@@ -1774,7 +1779,16 @@ impl GlobalContext {
         let key = ConfigKey::from_str("paths");
         // paths overrides cannot be set via env config, so use get_cv here.
         match self.get_cv(&key)? {
-            Some(CV::List(val, definition)) => Ok(Some(Value { val, definition })),
+            Some(CV::List(val, definition)) => {
+                let val = val
+                    .into_iter()
+                    .map(|cv| match cv {
+                        CV::String(s, def) => Ok((s, def)),
+                        other => self.expected("string", &key, &other),
+                    })
+                    .collect::<CargoResult<Vec<_>>>()?;
+                Ok(Some(Value { val, definition }))
+            }
             Some(val) => self.expected("list", &key, &val),
             None => Ok(None),
         }
@@ -2138,7 +2152,7 @@ enum KeyOrIdx {
 pub enum ConfigValue {
     Integer(i64, Definition),
     String(String, Definition),
-    List(Vec<(String, Definition)>, Definition),
+    List(Vec<ConfigValue>, Definition),
     Table(HashMap<String, ConfigValue>, Definition),
     Boolean(bool, Definition),
 }
@@ -2151,11 +2165,11 @@ impl fmt::Debug for ConfigValue {
             CV::String(s, def) => write!(f, "{} (from {})", s, def),
             CV::List(list, def) => {
                 write!(f, "[")?;
-                for (i, (s, def)) in list.iter().enumerate() {
+                for (i, item) in list.iter().enumerate() {
                     if i > 0 {
                         write!(f, ", ")?;
                     }
-                    write!(f, "{} (from {})", s, def)?;
+                    write!(f, "{item:?}")?;
                 }
                 write!(f, "] (from {})", def)
             }
@@ -2196,7 +2210,7 @@ impl ConfigValue {
                 val.into_iter()
                     .enumerate()
                     .map(|(i, toml)| match toml {
-                        toml::Value::String(val) => Ok((val, def.clone())),
+                        toml::Value::String(val) => Ok(CV::String(val, def.clone())),
                         v => {
                             path.push(KeyOrIdx::Idx(i));
                             bail!("expected string but found {} at index {i}", v.type_str())
@@ -2231,9 +2245,7 @@ impl ConfigValue {
             CV::Boolean(s, _) => toml::Value::Boolean(s),
             CV::String(s, _) => toml::Value::String(s),
             CV::Integer(i, _) => toml::Value::Integer(i),
-            CV::List(l, _) => {
-                toml::Value::Array(l.into_iter().map(|(s, _)| toml::Value::String(s)).collect())
-            }
+            CV::List(l, _) => toml::Value::Array(l.into_iter().map(|cv| cv.into_toml()).collect()),
             CV::Table(l, _) => {
                 toml::Value::Table(l.into_iter().map(|(k, v)| (k, v.into_toml())).collect())
             }
@@ -2275,7 +2287,7 @@ impl ConfigValue {
                         mem::swap(new, old);
                     }
                 }
-                old.sort_by(|a, b| a.1.cmp(&b.1));
+                old.sort_by(|a, b| a.definition().cmp(b.definition()));
             }
             (&mut CV::Table(ref mut old, _), CV::Table(ref mut new, _)) => {
                 for (key, value) in mem::take(new) {
@@ -2344,9 +2356,15 @@ impl ConfigValue {
         }
     }
 
-    pub fn list(&self, key: &str) -> CargoResult<&[(String, Definition)]> {
+    pub fn list(&self, key: &str) -> CargoResult<Vec<(String, Definition)>> {
         match self {
-            CV::List(list, _) => Ok(list),
+            CV::List(list, _) => list
+                .iter()
+                .map(|cv| match cv {
+                    CV::String(s, def) => Ok((s.clone(), def.clone())),
+                    _ => self.expected("string", key),
+                })
+                .collect::<CargoResult<_>>(),
             _ => self.expected("list", key),
         }
     }
diff --git a/src/cargo/util/context/target.rs b/src/cargo/util/context/target.rs
@@ -238,11 +238,11 @@ fn parse_links_overrides(
     Ok(links_overrides)
 }
 
-fn extra_link_args<'a>(
+fn extra_link_args(
     link_type: LinkArgTarget,
     key: &str,
-    value: &'a CV,
-) -> CargoResult<impl Iterator<Item = (LinkArgTarget, String)> + 'a> {
+    value: &CV,
+) -> CargoResult<Vec<(LinkArgTarget, String)>> {
     let args = value.list(key)?;
-    Ok(args.iter().map(move |v| (link_type.clone(), v.0.clone())))
+    Ok(args.into_iter().map(|v| (link_type.clone(), v.0)).collect())
 }
