Code Monkey home page Code Monkey logo

Comments (8)

matthiasbeyer avatar matthiasbeyer commented on June 9, 2024

How would you specify that in a configuration file?

from config-rs.

zhongzc avatar zhongzc commented on June 9, 2024

How would you specify that in a configuration file?

#[derive(Debug, Serialize, Deserialize, PartialEq)]
struct A {
    b: Option<B>,
}
#[derive(Debug, Serialize, Deserialize, PartialEq)]
struct B {}

impl Default for A {
    fn default() -> Self {
        Self { b: Some(B {}) }
    }
}

let a = A::default();
let toml_str = toml::to_string(&a).unwrap();
println!("toml str: {}", toml_str);

// let de_from_toml = toml::from_str::<A>(&toml_str).unwrap();
// assert_eq!(a, de_from_toml); // Passed

let de_from_toml: A = Config::builder()
    .add_source(File::from_str(&toml_str, FileFormat::Toml))
    .build()
    .unwrap()
    .try_deserialize()
    .unwrap();
assert_eq!(a, de_from_toml); // Passed

let de_from_default_object: A = Config::builder()
    .add_source(Config::try_from(&a).unwrap())
    .build()
    .unwrap()
    .try_deserialize()
    .unwrap();
assert_eq!(a, de_from_default_object); // Failed

Output:

toml str: [b]

assertion failed: `(left == right)`
  left: `A { b: Some(B) }`,
 right: `A { b: None }`

from config-rs.

matthiasbeyer avatar matthiasbeyer commented on June 9, 2024

And again I ask: How would you specify a instance of A { b: Some(B) } in a configuration file?

from config-rs.

zhongzc avatar zhongzc commented on June 9, 2024

Just like the previous example, for toml file, I specify it like this:

[b]

from config-rs.

matthiasbeyer avatar matthiasbeyer commented on June 9, 2024

Ah, I missed that bit. Hnm, indeed that's an issue. Thanks for reporting!

from config-rs.

matthiasbeyer avatar matthiasbeyer commented on June 9, 2024

I opened #463 with a testcase for this issue. If you have an idea solving that problem, you're of course very much welcome!

from config-rs.

zhongzc avatar zhongzc commented on June 9, 2024

The scenarios that cause B to be lost are related to iterators. An example that better reflects the situation I found:

#[derive(Debug, Serialize, Deserialize, PartialEq)]
struct A {
    b_option: Option<B>,
    b_vec: Vec<B>,
    b_set: HashSet<B>,
    b_map: HashMap<String, B>,
}
#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Hash)]
struct B {}

let a = A {
    b_option: Some(B {}),
    b_vec: vec![B {}],
    b_set: HashSet::from([B {}]),
    b_map: HashMap::from([("".to_owned(), B {})]),
};

// Serialize that `a` with `ConfigSerializer` will produce an empty config
let config = Config::try_from(&a).unwrap();

from config-rs.

polarathene avatar polarathene commented on June 9, 2024

TL;DR: Looks like this can be resolved easily enough with the below bolded fixes.

NOTE: Depending on when this is acted on, the referenced snippets below may change due to #465 (suggested fixes remain compatible, other referenced snippets may be refactored)


Note that there is two ways to define a unit-like struct (technically more?):

// Officially documented way:
// https://doc.rust-lang.org/book/ch05-01-defining-structs.html#unit-like-structs-without-any-fields
// https://learning-rust.github.io/docs/structs/
struct B;

// RFC:
// https://rust-lang.github.io/rfcs/0218-empty-struct-with-braces.html
// Previously not supported: https://github.com/rust-lang/rfcs/pull/147#issuecomment-47589168
// Accepted 2015: https://github.com/rust-lang/rfcs/pull/218#issuecomment-91558974
// Stabilized 2016: https://github.com/rust-lang/rust/issues/29720#issuecomment-189523437
struct B {}

These are roughly the same but have some differences.

If you omit the Option<>, and always have the unit struct:

  • struct B {} will fail with try_deserialize() due to error: value: missing field 'b'. Does not happen for struct B; (successful with struct).
  • toml::to_string(a) will fail for struct B; with: Error { inner: UnsupportedType(Some("B")) }. This also happens with the same error when wrapped in an Option<>.

struct B; would hit this route:

config-rs/src/ser.rs

Lines 165 to 182 in 6946069

fn serialize_none(self) -> Result<Self::Ok> {
self.serialize_unit()
}
fn serialize_some<T>(self, value: &T) -> Result<Self::Ok>
where
T: ?Sized + ser::Serialize,
{
value.serialize(self)
}
fn serialize_unit(self) -> Result<Self::Ok> {
self.serialize_primitive(Value::from(ValueKind::Nil))
}
fn serialize_unit_struct(self, _name: &'static str) -> Result<Self::Ok> {
self.serialize_unit()
}

Even though it does reach serialize_some() you can see that it takes the same path as serialize_none(), as value.serialize(self) will resolve to the serialize_unit_struct() method, which like serialize_none() redirects to serialize_unit() which normalizes to the Nil type, aka None.

Fix: Instead of self.serialize_unit(), a unit struct is more likely to map to a Table/Map like other structs. So an empty struct seems more appropriate?:

// Call this instead:
self.serialize_primitive(Value::from(crate::value::Table::new()))

EDIT: This fails to deserialize the unit struct properly with error:

It requires adding an additional deserialize method for Value in de.rs instead of relying on unit_struct in serde::forward_to_deserialize_any!:

#[inline]
fn deserialize_unit_struct<V: de::Visitor<'de>>(self, _name: &'static str, visitor: V) -> Result<V::Value> {
    visitor.visit_unit()
}

struct B {} instead hits the same serialize_some() route to start, but value.serialize(self) recognizes it as a struct thus calls serialize_struct():

config-rs/src/ser.rs

Lines 243 to 249 in 6946069

fn serialize_map(self, _len: Option<usize>) -> Result<Self::SerializeMap> {
Ok(self)
}
fn serialize_struct(self, _name: &'static str, len: usize) -> Result<Self::SerializeStruct> {
self.serialize_map(Some(len))
}

  • The _name is the struct B, and len is 0.
  • The returned result for both may look like different types but they are type aliases to Self?:

config-rs/src/ser.rs

Lines 85 to 94 in 6946069

impl<'a> ser::Serializer for &'a mut ConfigSerializer {
type Ok = ();
type Error = ConfigError;
type SerializeSeq = Self;
type SerializeTuple = Self;
type SerializeTupleStruct = Self;
type SerializeTupleVariant = Self;
type SerializeMap = Self;
type SerializeStruct = Self;
type SerializeStructVariant = Self;

This is also None presumably because there is no logic involved that serializes to the config-rs internal Value type (which serialize_unit() did for struct B; earlier by calling serialize_primitive()):

config-rs/src/ser.rs

Lines 15 to 22 in 6946069

impl ConfigSerializer {
fn serialize_primitive<T>(&mut self, value: T) -> Result<()>
where
T: Into<Value> + Display,
{
let key = match self.last_key_index_pair() {
Some((key, Some(index))) => format!("{}[{}]", key, index),
Some((key, None)) => key.to_string(),

config-rs/src/ser.rs

Lines 31 to 32 in 6946069

#[allow(deprecated)]
self.output.set(&key, value.into())?;

Fix: A similar fix here is to also create a table, presumably only when len is 0. This should not replace the existing call, due to conflicting return type. Modify serialize_struct():

// Struct is empty (unit struct with empty braces):
if len == 0 {
    self.serialize_primitive(Value::from(crate::value::Table::new()))?;
}
// Keep the original call:
self.serialize_map(Some(len))

In both cases, I assume the expected serialization is what I've observed the deserialized format to Value is, an empty variant of Table (Map<String, Value>).

Applying either fix for that as suggested above now serializes the unit struct type option successfully, which will properly deserialize the option (since ValueKind::Nil would resolve to None but any other ValueKind variant like Table resolves to Some):

config-rs/src/de.rs

Lines 132 to 142 in 6946069

#[inline]
fn deserialize_option<V>(self, visitor: V) -> Result<V::Value>
where
V: de::Visitor<'de>,
{
// Match an explicit nil as None and everything else as Some
match self.kind {
ValueKind::Nil => visitor.visit_none(),
_ => visitor.visit_some(self),
}
}


Deserialization isn't an issue without the Option wrapping type, since Nil seems to resolve implicitly via the unit_struct -> unit derived methods generated here (deserialize_unit_struct() that calls self.deserialize_unit(visitor) that calls visitor.visit_unit()):

config-rs/src/de.rs

Lines 167 to 171 in 6946069

serde::forward_to_deserialize_any! {
char seq
bytes byte_buf map struct unit
identifier ignored_any unit_struct tuple_struct tuple
}

The value prior to calling visitor.visit_unit() is Value of ValueKind::Nil:

Value {
    origin: None,
    kind: Nil,
}

I assume directly after this call it's converted to the unit struct, but I'm not sure how that works 😅

from config-rs.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.