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

[BUG][RUST] oneOf with multiple arrays produces invalid rust code #18527

Open
5 of 6 tasks
felixauringer opened this issue Apr 28, 2024 · 7 comments
Open
5 of 6 tasks

Comments

@felixauringer
Copy link

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

If there is no title, the rust generator tries to derive a meaningful name from the type (see #17896). However, this fails in some cases, for example if there is a oneOf with multiple arrays inside.

The OpenAPI spec below produces the following enum:

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
pub enum Option1OrOption2Options {
    Array(Vec<String>),
    Array(Vec<i32>),
}

This cannot be compiled, the error message is error[E0428]: the name 'Array' is defined multiple times. I would expect that all code generated from a valid OpenAPI spec generates compilable rust code.

openapi-generator version

I tested with version 06ed7c8 (the current main).

OpenAPI declaration file content or url
openapi: 3.0.0
info:
  title: oneOf with arrays
  description: Ensure different names for kinds in enum when using oneOf with arrays.
  version: 1.0.0
paths: {}
components:
  schemas:
    Option1OrOption2:
      type: object
      properties:
        Options:
          oneOf:
            - type: array
              items:
                type: string
            - type: array
              items:
                type: integer
Steps to reproduce

I'll provide the steps to create a failing test using the valid OpenAPI spec above.

  • Add the snippet above to the test resources (e.g. modules/openapi-generator/src/test/resources/3_0/rust/oneof-with-arrays.yaml).

  • Create a config for the test (e.g. bin/configs/rust-reqwest-oneOf-with-arrays.yaml):

    generatorName: rust
    outputDir: samples/client/others/rust/reqwest/oneOf-with-arrays
    library: reqwest
    inputSpec: modules/openapi-generator/src/test/resources/3_0/rust/oneof-with-arrays.yaml
    templateDir: modules/openapi-generator/src/main/resources/rust
    additionalProperties:
      supportAsync: false
      packageName: oneof-with-arrays-reqwest
  • Generate the rust samples with ./bin/generate-samples.sh ./bin/configs/rust-*.

  • Execute the tests with mvn integration-test -f samples/client/others/rust/pom.xml.

Related issues/PRs
Suggest a fix

I'm new to rust and to openapi-generator, so I am no help here, sorry.

@wing328
Copy link
Member

wing328 commented Apr 30, 2024

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
pub enum Option1OrOption2Options {
    Array(Vec<String>),
    Array(Vec<i32>),
}

ideally what's the correct/expected code look like?

@felixauringer
Copy link
Author

There is probably not a really nice way as there is just not much information. Maybe one could enumerate the enum fields:

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
pub enum Option1OrOption2Options {
    Array1(Vec<String>),
    Array2(Vec<i32>),
}

Those are not helpful variable names but at least it could be compiled. Maybe the generators for the other languages in this project have a nicer way to solve this?

@wing328
Copy link
Member

wing328 commented May 6, 2024

what about the following instead?

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
pub enum Option1OrOption2Options {
    ArrayVecString(Vec<String>),
    ArrayVeci32(Vec<i32>),
}

@felixauringer
Copy link
Author

Theoretically, the following is also a valid OpenAPI spec:

openapi: 3.0.0
info:
  title: oneOf with arrays
  description: Ensure different names for kinds in enum when using oneOf with arrays.
  version: 1.0.0
paths: {}
components:
  schemas:
    Option1OrOption2:
      type: object
      properties:
        Options:
          oneOf:
            - type: array
              items:
                type: string
            - type: array
              items:
                type: string

With your proposition, this would result in a name clash again. However, depending on whether you want to support all edge cases of OpenAPI, that might be okay because the semantics of the spec above are not clear (why even have a oneOf if both alternatives are exactly the same?).

@nagua
Copy link

nagua commented Jun 21, 2024

I found something similar to this today.
We have something like this:

openapi: 3.0.0
info:
  title: oneOf with arrays
  description: Ensure different names for kinds in enum when using oneOf with arrays.
  version: 1.0.0
paths: {}
components:
  schemas:
    Option1OrOption2:
      type: object
      properties:
        Options:
          oneOf:
            - type: string
              filter: '^\d{4}$'
            - type: string
              maxLength: 0

This should also generate distinct types.
This also has another problem as this will currently create a String enum member which is already defined for the native Rust's String type.
I would also think that just generating distinct names would be a good idea. Then you can map this name with --type-mappings.

@geoffreygarrett
Copy link

Any further plans on tackling this?

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
pub enum Option1OrOption2Options {
ArrayVecString(Vec),
ArrayVeci32(Vec),
}

Your suggestion is the imo the most idiomatic solution. If someone could point me in the direction of where the name variable is constructed in the java source, I'll get it done.

@geoffreygarrett
Copy link

@wing328

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

No branches or pull requests

4 participants