toml-rs / toml

Rust TOML Parser
https://docs.rs/toml
Apache License 2.0
715 stars 105 forks source link

Missing spans for errors in variant fields #535

Open Patryk27 opened 1 year ago

Patryk27 commented 1 year ago

Hi,

Following test fails:

#[test]
fn test_spanned_enum() {
    #[derive(Debug, Deserialize)]
    #[serde(tag = "type")]
    enum Foo {
        Bar(Bar),
    }

    #[derive(Debug, Deserialize)]
    struct Bar {
        hello: Spanned<String>,
    }

    let toml = "
        type = 'Bar'
        hello = 'world'
    ";

    let foo: Foo = toml::from_str(toml).unwrap();

    println!("{foo:?}");
}

... saying:

Error { inner: Error { inner: TomlError { message: "invalid type: string \"world\", expected a spanned value", original: None, keys: [], span: None } } }

(ofc. changing Spanned<String> to just String works.)

I think the underlying reason is that using an enum (at least in the way presented above) gets expanded by Serde into a call to deserialize_any(), which later makes this boi:

https://github.com/toml-rs/toml/blob/9e43da13b04601d9115747610753d73852fbf4c9/crates/toml_edit/src/de/value.rs#L55

... unaware of the "spanned" context it's being invoked on :cry:

See also

epage commented 1 year ago

I'm not seeing how we could support this.

serde_derive, when deserializing untagged enums and internally tagged enums, buffers into a Content enum. This buffering loses all context of what we are deserializing into so we don't know we can attempt Spanned. We can't even retry with spanned on error because the deserializing of Content will be happening later.

See also #589

Patryk27 commented 1 year ago

True; for what it's worth, in my concrete use case the tag is always the first field, so using a custom deserializer works pretty well - adjusted for the example above, it would say:

impl<'de> Deserialize<'de> for Foo {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: serde::Deserializer<'de>,
    {
        struct FooVisitor;

        impl<'de> de::Visitor<'de> for FooVisitor {
            type Value = Foo;

            fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
                write!(formatter, "a Foo")
            }

            fn visit_map<A>(self, mut map: A) -> Result<Self::Value, A::Error>
            where
                A: de::MapAccess<'de>,
            {
                let tag_key = map.next_key::<String>()?;

                assert_eq!("type", tag_key.unwrap());

                let tag_value = map.next_value::<String>()?;

                match tag_value.as_str() {
                    "Bar" => Ok(Foo::Bar(Deserialize::deserialize(
                        de::value::MapAccessDeserializer::new(map),
                    )?)),

                    tag => Err(A::Error::custom(format!("unknown tag: {tag}"))),
                }
            }
        }

        deserializer.deserialize_map(FooVisitor)
    }
}