Skip to content
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.

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

Missing spans for errors in variant fields #535

Open
Patryk27 opened this issue Mar 29, 2023 · 2 comments
Open

Missing spans for errors in variant fields #535

Patryk27 opened this issue Mar 29, 2023 · 2 comments
Labels
A-error Area: Error reporting A-serde Area: Serde integration C-bug Category: Things not working as expected

Comments

@Patryk27
Copy link

Patryk27 commented Mar 29, 2023

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:

fn deserialize_any<V>(self, visitor: V) -> Result<V::Value, Self::Error>

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

See also

@epage
Copy link
Member

epage commented Mar 29, 2023

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

@epage epage added C-bug Category: Things not working as expected A-serde Area: Serde integration labels Mar 29, 2023
@Patryk27
Copy link
Author

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)
    }
}

@epage epage changed the title Missing spans for variant fields Missing spans for errors in variant fields Mar 8, 2024
@epage epage added the A-error Area: Error reporting label Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-error Area: Error reporting A-serde Area: Serde integration C-bug Category: Things not working as expected
Projects
None yet
Development

No branches or pull requests

2 participants