Skip to content

refactor: unflatten (String, Def) to ConfigValue #16084

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 10, 2025
+149 −86
Merged

refactor: unflatten (String, Def) to ConfigValue #16084

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
341962a
refactor(gctx): remove duplicate function
weihanglo Oct 10, 2025
1850029
refactor: switch to `Vec<ConfigValue>` for `ConfigValue::List`
weihanglo Oct 8, 2025
df4f7fc
refactor: rename `ConfigValue::list` to `string_list`
weihanglo Oct 8, 2025
c762238
refactor: split two code paths in ValueDeserializer to enum
weihanglo Oct 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/cargo/ops/cargo_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
" {}, # {}",
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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) => {
Expand Down
129 changes: 85 additions & 44 deletions src/cargo/util/context/de.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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)
Expand Down Expand Up @@ -387,7 +395,7 @@ impl<'de, 'gctx> de::MapAccess<'de> for ConfigMapAccess<'gctx> {
.gctx
.get_cv_with_env(&self.de.key)
.ok()
.and_then(|cv| cv.map(|cv| cv.get_definition().clone())),
.and_then(|cv| cv.map(|cv| cv.definition().clone())),
)
});
self.de.key.pop();
Expand All @@ -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 {
Expand All @@ -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(),
})
}
Expand All @@ -430,17 +441,34 @@ impl<'de> de::SeqAccess<'de> for ConfigSeqAccess {
{
match self.list_iter.next() {
// TODO: add `def` to error?
Some((value, def)) => {
Some(val @ CV::String(..)) => {
// 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)
// ArrayItemDeserializer will handle figuring out which one it is.
seed.deserialize(ArrayItemDeserializer::new(val)).map(Some)
}
Some(val) => Err(ConfigError::expected(&self.key, "list of string", &val)),
None => Ok(None),
}
}
}

/// Source of data for [`ValueDeserializer`]
enum ValueSource<'gctx> {
/// The deserializer used to actually deserialize a Value struct.
Deserializer {
de: Deserializer<'gctx>,
definition: Definition,
},
/// A [`ConfigValue`](CV).
///
/// This is used for situations where you can't address a string via a TOML key,
/// such as a string inside an array.
/// The [`ConfigSeqAccess`] doesn't know if the type it should deserialize to
/// is a `String` or `Value<String>`,
/// so [`ArrayItemDeserializer`] needs to be able to handle both.
ConfigValue(CV),
}

/// This is a deserializer that deserializes into a `Value<T>` for
/// configuration.
///
Expand All @@ -450,18 +478,7 @@ impl<'de> de::SeqAccess<'de> for ConfigSeqAccess {
/// See more comments in `value.rs` for the protocol used here.
struct ValueDeserializer<'gctx> {
hits: u32,
definition: Definition,
/// The deserializer, used to actually deserialize a Value struct.
/// This is `None` if deserializing a string.
de: Option<Deserializer<'gctx>>,
/// A string value to deserialize.
///
/// This is used for situations where you can't address a string via a
/// TOML key, such as a string inside an array. The `ConfigSeqAccess`
/// doesn't know if the type it should deserialize to is a `String` or
/// `Value<String>`, so `ValueDeserializer` needs to be able to handle
/// both.
str_value: Option<String>,
source: ValueSource<'gctx>,
}

impl<'gctx> ValueDeserializer<'gctx> {
Expand All @@ -486,20 +503,24 @@ impl<'gctx> ValueDeserializer<'gctx> {
(_, None) => env_def,
}
};

Ok(ValueDeserializer {
hits: 0,
definition,
de: Some(de),
str_value: None,
source: ValueSource::Deserializer { de, definition },
})
}

fn new_with_string(s: String, definition: Definition) -> ValueDeserializer<'gctx> {
fn with_cv(cv: CV) -> ValueDeserializer<'gctx> {
ValueDeserializer {
hits: 0,
definition,
de: None,
str_value: Some(s),
source: ValueSource::ConfigValue(cv),
}
}

fn definition(&self) -> &Definition {
match &self.source {
ValueSource::Deserializer { definition, .. } => definition,
ValueSource::ConfigValue(cv) => cv.definition(),
}
}
}
Expand Down Expand Up @@ -530,19 +551,19 @@ impl<'de, 'gctx> de::MapAccess<'de> for ValueDeserializer<'gctx> {
// If this is the first time around we deserialize the `value` field
// which is the actual deserializer
if self.hits == 1 {
if let Some(de) = &self.de {
return seed
return match &self.source {
ValueSource::Deserializer { de, definition } => seed
.deserialize(de.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());
}
.map_err(|e| e.with_key_context(&de.key, Some(definition.clone()))),
ValueSource::ConfigValue(cv) => {
seed.deserialize(ArrayItemDeserializer::new(cv.clone()))
}
};
}

// ... otherwise we're deserializing the `definition` field, so we need
// to figure out where the field we just deserialized was defined at.
match &self.definition {
match self.definition() {
Definition::Path(path) => {
seed.deserialize(Tuple2Deserializer(0i32, path.to_string_lossy()))
}
Expand All @@ -560,25 +581,45 @@ impl<'de, 'gctx> de::MapAccess<'de> for ValueDeserializer<'gctx> {
}
}

// Deserializer is only implemented to handle deserializing a String inside a
// sequence (like `Vec<String>` or `Vec<Value<String>>`). `Value<String>` is
// handled by deserialize_struct, and the plain `String` is handled by all the
// other functions here.
impl<'de, 'gctx> de::Deserializer<'de> for ValueDeserializer<'gctx> {
/// A deserializer for individual [`ConfigValue`](CV) items in arrays
///
/// This deserializer is only implemented to handle deserializing a String
/// inside a sequence (like `Vec<String>` or `Vec<Value<String>>`).
/// `Value<String>` is handled by `deserialize_struct` in [`ValueDeserializer`],
/// and the plain `String` is handled by all the other functions here.
#[derive(Clone)]
struct ArrayItemDeserializer {
cv: CV,
}

impl ArrayItemDeserializer {
fn new(cv: CV) -> Self {
Self { cv }
}

fn into_inner(self) -> String {
match self.cv {
CV::String(s, _def) => s,
_ => unreachable!("string expected"),
}
}
}

impl<'de> de::Deserializer<'de> for ArrayItemDeserializer {
type Error = ConfigError;

fn deserialize_str<V>(self, visitor: V) -> Result<V::Value, Self::Error>
where
V: de::Visitor<'de>,
{
visitor.visit_str(&self.str_value.expect("string expected"))
visitor.visit_str(&self.into_inner())
}

fn deserialize_string<V>(self, visitor: V) -> Result<V::Value, Self::Error>
where
V: de::Visitor<'de>,
{
visitor.visit_string(self.str_value.expect("string expected"))
visitor.visit_string(self.into_inner())
}

fn deserialize_struct<V>(
Expand All @@ -595,7 +636,7 @@ impl<'de, 'gctx> de::Deserializer<'de> for ValueDeserializer<'gctx> {
//
// See more comments in `value.rs` for the protocol used here.
if name == value::NAME && fields == value::FIELDS {
return visitor.visit_map(self);
return visitor.visit_map(ValueDeserializer::with_cv(self.cv));
}
unimplemented!("only strings and Value can be deserialized from a sequence");
}
Expand All @@ -604,7 +645,7 @@ impl<'de, 'gctx> de::Deserializer<'de> for ValueDeserializer<'gctx> {
where
V: de::Visitor<'de>,
{
visitor.visit_string(self.str_value.expect("string expected"))
visitor.visit_string(self.into_inner())
}

fn deserialize_ignored_any<V>(self, visitor: V) -> Result<V::Value, Self::Error>
Expand Down
Loading