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

Add support for inlined enum variants #963

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JMLX42
Copy link
Contributor

@JMLX42 JMLX42 commented Jun 20, 2024

Implementation for #727

The following is now supported:

    #[derive(ToSchema)]
    enum Number {
        One,
        Two,
        Three,
        Four,
        Five,
        Six,
        Seven,
        Height,
        Nine,
    }

    #[derive(ToSchema)]
    enum Color {
        Spade,
        Heart,
        Club,
        Diamond,
    }

    #[derive(ToSchema)]
    enum Card {
        Number(#[schema(inline)] Number),
        Color(#[schema(inline)] Color),
    }

and will produce the following schema:

{
            "oneOf": [
                {
                    "properties": {
                        "Number": {
                            "enum": [
                                "One",
                                "Two",
                                "Three",
                                "Four",
                                "Five",
                                "Six",
                                "Seven",
                                "Height",
                                "Nine",
                            ],
                            "type": "string",
                        },
                    },
                    "required": [
                        "Number",
                    ],
                    "type": "object",
                },
                {
                    "properties": {
                        "Color": {
                            "enum": [
                                "Spade",
                                "Heart",
                                "Club",
                                "Diamond",
                            ],
                            "type": "string",
                        },
                    },
                    "required": [
                        "Color",
                    ],
                    "type": "object",
                },
            ],
        }

@@ -619,6 +619,14 @@ impl ToTokensDiagnostics for UnnamedStructSchema<'_> {

if fields_len == 1 {
if let Some(ref mut features) = unnamed_struct_features {
let inline =
features::parse_schema_features_with(&first_field.attrs, |input| {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juhaku that looks kinda naive. Isn't there a way to properly specify which features are supported by this specific case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is I'd rather have something like this:

Ok(EnumUnnamedFieldVariantFeatures(parse_features!(

But those features are for the variant themselves, so Inline would be:

    #[derive(ToSchema)]
    enum Card {
        #[schema(inline)]
        Number(Number),
        #[schema(inline)]
        Color(Color),
    }

Which IMO does not fit with the rest of the API and is counter-intuitive.

But I haven't found the features struct for the unnamed field.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juhaku that looks kinda naive. Isn't there a way to properly specify which features are supported by this specific case?

No features have been originally defined for unnamed field enum variants itself thus there is no such struct. Though if there is just a single feature to be parsed the parse_with is just fine but if there would be multiple features or the set of features is known to be needed in multiple locations then a feature struct is preferred.

Which IMO does not fit with the rest of the API and is counter-intuitive.

Yeah, I agree the API would be counter intuitive and the features defined directly for the unnamed enum variant field is cleaner approach.

But I haven't found the features struct for the unnamed field.

Such struct does not exists by design but could be added if needed.

Copy link
Owner

@juhaku juhaku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good for me 🎉

Though docs should be updated with the added feature at ToSchema macro by changing the doc comment here

#[proc_macro_derive(ToSchema, attributes(schema, aliases))]
.

@JMLX42
Copy link
Contributor Author

JMLX42 commented Jul 23, 2024

This is good for me 🎉

Though docs should be updated with the added feature at ToSchema macro by changing the doc comment here

#[proc_macro_derive(ToSchema, attributes(schema, aliases))]

.

@juhaku I guess this section would be the right place to document the feature?

@juhaku
Copy link
Owner

juhaku commented Jul 23, 2024

@juhaku I guess this section would be the right place to document the feature?

Yes that chapter would be the right place for the doc. It should describe that it is used directly for the variant field and not for the variant itself. And perhaps an example of this would be good to add as well under the examples section a little bit further down.

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

Successfully merging this pull request may close these issues.

None yet

2 participants