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

Merge oneOf with many compatible enums of a single value #497

Closed
jayvdb opened this issue Jan 31, 2024 · 4 comments · Fixed by #500
Closed

Merge oneOf with many compatible enums of a single value #497

jayvdb opened this issue Jan 31, 2024 · 4 comments · Fixed by #500

Comments

@jayvdb
Copy link
Contributor

jayvdb commented Jan 31, 2024

OAI/OpenAPI-Specification#348 (comment) is apparently the best way to describe enum values, like

    "enum-value-descriptions": {
      "type": "object",
      "description": "https://github.com/OAI/OpenAPI-Specification/issues/348#issuecomment-1484163189",
      "oneOf": [
        {
          "enum": ["A"],
          "description": "An A"
        },
        {
          "enum": ["B"],
          "description": "A B"
        }
      ]
    }

Typify generates a bunch of variants for the above, when they could be collapsed into a single Rust enum.

@ahl
Copy link
Collaborator

ahl commented Jan 31, 2024

Thanks for filing this. Today we get something like this:

pub enum Thing {
    Variant0(ThingVariant0),
    Variant1(ThingVariant1),
}

pub enum ThingVariant0 {
    A,
}

pub enum ThingVariant1 {
    B,
}

Awful! The good news is that a bunch of recent refactoring has been oriented around addressing precisely this kind of weirdness. In particular, my plan is to do a "simplification" or "canonicalization" pass in the oneOf handling before trying to map it against various encodings of enums.

As another example:

pub enum WeirdEnum {
Variant0 {
pattern: String,
},
Variant1 {
patterns: String,
},
Variant2 {
#[serde(rename = "pattern-either")]
pattern_either: String,
},
Variant3 {
#[serde(rename = "pattern-regex")]
pattern_regex: String,
},
}

There's no reason this shouldn't be an externally tagged variant:

pub enum WeirdEnum {
   #[serde(rename = "pattern")]
    Pattern(String),
   #[serde(rename = "patterns")]
    Patterns(String),
   #[serde(rename = "pattern-either")]
    PatternEither(String),
    #[serde(rename = "pattern-regex")]
    PatternRegex(String),
}

@ahl
Copy link
Collaborator

ahl commented Feb 5, 2024

On closer inspection, there are two slightly different issues. The first is that the type of your schema is object. Arguably, typify should be rejecting this because there is no value that satisfies the schema (i.e. is an object and is either "A" or "B").

A more obvious error from typify is that it requires the subschemas to have "type": "string". This should be unnecessary, and I've got a PR to fix it (see #500).

@jayvdb
Copy link
Contributor Author

jayvdb commented Feb 5, 2024

Sorry the 'object' bit was my mistake.

OAI/OpenAPI-Specification#348 (comment) says to use type: string

@ahl ahl closed this as completed in #500 Feb 5, 2024
@ahl
Copy link
Collaborator

ahl commented Feb 5, 2024

Sorry the 'object' bit was my mistake.

I was just observing that this is another gap in typify's logic--it should either register some sort of error saying that this type is unsatisfiable or produce some sort of Never type (e.g. enum Never{}).

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 a pull request may close this issue.

2 participants