dtolnay / serde-yaml

Strongly typed YAML library for Rust
Apache License 2.0
964 stars 164 forks source link

confusion around `Value::TaggedValue` and `is_null` #375

Open 0rphon opened 1 year ago

0rphon commented 1 year ago

the following code shows the issue

let x = Value::Tagged(Box::new(TaggedValue {
    tag: Tag::new("hello"),
    value: Value::Null,
}));
assert!(!x.is_null()); // fails

x is treated as Null despite being a TaggedValue. This causes issues when dealing with enums that have optional values like the following:

#[derive(Serialize, Deserialize, Debug)]
enum TestEnum {
    V(Option<String>),
}

#[derive(Serialize, Deserialize, Debug)]
struct TestStruct {
    f: Option<TestEnum>,
}

let x = TestStruct {
    f: Some(TestEnum::V(None)),
};

let y = serde_yaml::to_string(&x).unwrap(); // serialized to `f: !V null`
let v: Value = serde_yaml::from_str(&y).unwrap();
assert_eq!(x.f.is_none(), v.get("f").unwrap().is_null()); // fails

This makes it appear as if x.f is None, when in reality it is Some(TestEnum::V(None)). This didn't use to be the case and in version 0.8.0 both the above examples would pass. This is an obvious enough change that I'm guessing it must be intended, but I feel like the current behavior is severely misleading and, at the very least, a warning should be given in the documentation of is_null