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

Support custom variant resolution mechanism #1212

Closed
udoprog opened this issue Apr 10, 2018 · 4 comments
Closed

Support custom variant resolution mechanism #1212

udoprog opened this issue Apr 10, 2018 · 4 comments
Labels

Comments

@udoprog
Copy link

udoprog commented Apr 10, 2018

I have a project where I need to pick the variant based on a strict set of the available fields.

Today I have the untagged container attribute to work with. While flexible, it's unable to disambiguate which variant to use in cases like these:

#[serde(untagged)]
enum Untagged {
    A {
        a: string,
    }
    B {
        b: string,
    }
    C {
        a: string,
        b: string,
    }
}

I require deserialization to happen strictly based on the presence of a set of required fields which might overlap with required fields in other variants. This can be accomplished by looking at a strict set of available fields names at once, instead of only firing of a sequence of deserialization attempts.

Option 1: Support another container attribute

Support another container attribute which is specialized towards only matching a set of required fields.

#[serde(required_fields)]
enum Untagged {
    A {
        a: string,
        other: Option<String>,
    }
    B {
        b: string,
        other: Option<String>,
    }
    C {
        a: string,
        b: string,
        other: Option<String>,
    }
}

This requires that each variant is also validated to not contain any optional fields. E.g. variant A would not be permitted to have an optional field b.

This approach could be considered overly specialized for a single use-case.

Solution 2: Provide the user with hooks to implement a custom variant resolution mechanism

One somewhat hacky option could be to permit the specification of a custom variant resolver through a custom function:

#[serde(untagged(variant_names = "UntaggedNames", variant_resolution = "Untagged::solve_variant"))]
enum Untagged {
    A {
        a: string,
    }
    B {
        b: string,
    }
    C {
        a: string,
        b: string,
    }
}

impl Untagged {
    fn solve_variant<'de, D, V>(deserializer: D, deserialize_variant: V) -> Result<Untagged, D::Error>
        where V: FnOnce(UntaggedNames) -> Result<Untagged, D::Error>
    {
        deserialize_variant(UntaggedNames::A)
    }
}

Because I'm not overly familiar with the internals of serde, I currently don't know how viable this approach would be.

@dtolnay
Copy link
Member

dtolnay commented Apr 10, 2018

You could either use the existing deny_unknown_fields attribute:

#[derive(Deserialize)]
#[serde(untagged, deny_unknown_fields)]
enum Untagged {
    A {
        a: String,
    },
    AB {
        a: String,
        b: String,
    },
}

or order the variants such that any variant with a superset of the fields of a different variant always appears earlier:

#[derive(Deserialize)]
#[serde(untagged)]
enum Untagged {
    AB {
        a: String,
        b: String,
    },
    A {
        a: String,
    },
}

depending on whether you want unrecognized fields to error out or select the maximally matched variant (respectively).

Beyond that, you could lose the #[derive(Deserialize)] and emit your own Deserialize trait implementation to perform variant resolution in whatever way you want.

@udoprog
Copy link
Author

udoprog commented Apr 10, 2018

The solution with deny_unknown_fields is neat, and I believe does everything I need. It was not obvious to me that this was a solution, so I appreciate the pointer!

I'm curious if it would valuable to have an option to untagged that causes it to solely discriminate based on the presence of fields to improve error reporting*? Committing to one of the variants would improve diagnostics. It could be a step towards making progress on #773.

*: https://play.rust-lang.org/?gist=61b91ff79ddd3ad4bb014c1378a28c5b&version=stable

@dtolnay
Copy link
Member

dtolnay commented Apr 10, 2018

I would prefer to solve #773 without introducing another attribute.

@udoprog
Copy link
Author

udoprog commented Apr 10, 2018

Ok, I'll close this. With improved error reporting I don't have anything that is unique to this issue. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants