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] x-enum-varnames is underspecified #3246

Open
kevinoid opened this issue Jun 29, 2019 · 3 comments · May be fixed by #10325
Open

[BUG] x-enum-varnames is underspecified #3246

kevinoid opened this issue Jun 29, 2019 · 3 comments · May be fixed by #10325

Comments

@kevinoid
Copy link
Contributor

Description

Support for the x-enum-varnames extension was added in #917 to support specifying variable names for enumeration values (e.g. for numeric values, as in #893 (comment)). The documentation from #2010 suggests that the names will undergo language-specific normalization (as enum values would), but they do not, which can result in broken code.

openapi-generator version

The issue has been present since #917 was merged. Tested on master.

OpenAPI declaration file content or url
openapi: '3.0.2'
info:
  title: x-enum-varnames example
  version: '1.0.0'
components:
  schemas:
    WeatherType:
      type: integer
      enum:
      - 0
      - 1
      - 2
      x-enum-varnames:
      - Sunny
      - Partly Cloudy
      - Rainy
paths:
  /weather:
    get:
      responses:
        default:
          description: Current weather
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/WeatherType'
Command line used for generation

openapi-generator-cli.jar generate -i openapi.yaml -g java -o bad-x-enum-varnames-java

Steps to reproduce
  1. Run the above command.
  2. Try to compile the generated code, which will fail due to Partly Cloudy being an invalid variable name.
Suggest a fix

The appropriate fix depends on the desired semantics of x-enum-varnames:

  1. If x-enum-varnames should be a natural language name, then the values should be converted to identifiers in the same way as enum values (as suggested in the documentation), which would fix the above example.
  2. If x-enum-varnames must be a valid identifier (in every language which can be generated) then the documentation should be updated to clarify that. (Ideally the code generation would also fail more gracefully.)

A problem with the first option is that it prevents an existing use case of overriding the generator convention, as described in swagger-api/swagger-codegen#7466 (comment), and could change the generated API by changing the generated names (depending on how identifier normalization is done).

A problem with the second option is that different languages have different casing conventions for enumeration values and there's no way to satisfy them all. Should the spec use PARTLY_CLOUDY, PartlyCloudy, or partlyCloudy for x-enum-varnames? Any choice will be unconventional in several languages. Also, different languages have different rules for valid identifiers and requiring spec authors to comply with all of them is burdensome.

Thanks for considering,
Kevin

@auto-labeler
Copy link

auto-labeler bot commented Jun 29, 2019

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@hleb-albau
Copy link

Any update on this?

@akhileshnair08
Copy link

akhileshnair08 commented Aug 4, 2020

ABCEnum:
enum:
-{0,"aum"}
-{1,"gautham"}
-{2,"Buddha"}
x-enum-varnames:
- AUM
- GAUTHAM
- BUDDHA
type: object
properties:
num_value:
type: integer
format: int32
str_value:
type: string

Why is this not working???
getting below error:
unexpected error in Open-API generation (org.openapitools:openapi-generator-maven-plugin:4.1.3:generate:default:generate-sources)

kevinoid added a commit to kevinoid/appveyor-swagger that referenced this issue Nov 17, 2022
openapi-generator (which is the only generator I am aware of which
supports x-enum-varnames) uses x-enum-varnames as identifiers verbatim.
Therefore, they must be valid identifiers (in all languages).
Unfortunately, whatever capitalization convention we use will look weird
in some languages.  Choose a common one for now.

See OpenAPITools/openapi-generator#3246 for discussion.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
kevinoid added a commit to kevinoid/appveyor-swagger that referenced this issue Nov 17, 2022
openapi-generator (which is the only generator I am aware of which
supports x-enum-varnames) uses x-enum-varnames as identifiers verbatim.
Therefore, they must be valid identifiers (in all languages).
Unfortunately, whatever capitalization convention we use will look weird
in some languages.  Choose a common one for now.

See OpenAPITools/openapi-generator#3246 for discussion.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
kevinoid added a commit to kevinoid/appveyor-swagger that referenced this issue Nov 17, 2022
openapi-generator (which is the only generator I am aware of which
supports x-enum-varnames) uses x-enum-varnames as identifiers verbatim.
Therefore, they must be valid identifiers (in all languages).
Unfortunately, whatever capitalization convention we use will look weird
in some languages.  Choose a common one for now.

See OpenAPITools/openapi-generator#3246 for discussion.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
kevinoid added a commit to kevinoid/appveyor-swagger that referenced this issue Nov 18, 2022
openapi-generator (which is the only generator I am aware of which
supports x-enum-varnames) uses x-enum-varnames as identifiers verbatim.
Therefore, they must be valid identifiers (in all languages).
Unfortunately, whatever capitalization convention we use will look weird
in some languages.  Choose a common one for now.

See OpenAPITools/openapi-generator#3246 for discussion.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants