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

[csharp] Introducing the enumPropertyNaming option to the C# Generator #16981

Merged
merged 2 commits into from Nov 7, 2023

Conversation

fujieda
Copy link
Contributor

@fujieda fujieda commented Nov 4, 2023

This pull request introduces the enumPropertyNaming option to the C# generator. This option, which is currently supported by the Kotlin and TypeScript generators, allows you to control the naming style of enum names.

In our projects, we consistently use UPPER_SNAKE_CASE for constants in our OpenAPI definitions, and we prefer to maintain this naming style in our C# code. However, the current C# generator changes the enum names by removing the underscores, resulting in enum names like UPPERSNAKECASE.

While searching for a solution to this problem, I came across the enumPropertyNaming option available in the Kotlin and TypeScript generators. To resolve this discrepancy and ensure that our enum naming style remains consistent with UPPER_SNAKE_CASE, I ported this option to the C# generator.

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh ./bin/configs/*.yaml
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master (upcoming 7.1.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@mandrean @shibayan @Blackclaws @lucamazzanti @iBicha

@wing328
Copy link
Member

wing328 commented Nov 4, 2023

@fujieda thanks for the PR.

What about adding an option similar to modelNameMapping, parameterNameMapping such as enumNameMapping to have complete control of the enum naming in the C# generator?

ref: https://github.com/OpenAPITools/openapi-generator/blob/master/docs/customization.md#name-mapping

@fujieda
Copy link
Contributor Author

fujieda commented Nov 5, 2023

@wing328

I couldn't find an option to define enum name mappings similar to modelNameMapping and parameterNameMapping in the C# generator. Upon reviewing the toEnumVarName method in AbstractCsharpCodegen, it appears that there's no code implementation for applying name mappings, unlike toVarName, toParamName, and toModelName, which support such mappings.

However, in our projects, our primary goal is not to establish specific name mappings, but rather to exert control over the naming styles of enum names. We have a significant number of enum names within our OpenAPI definitions, over fifty in total. Given this scale, defining individual name mappings for each enum name isn't a practical or feasible approach for us.

@wing328
Copy link
Member

wing328 commented Nov 5, 2023

I couldn't find an option to define enum name mappings similar to modelNameMapping and parameterNameMapping in the C# generator.

Sorry I wasn't clear. I mean "adding an option" to your requirement (which means the option doesn't exist yet)

Thanks for sharing the use cases. I'll review and get back to you.

{ "foo-bar", "fooBar", camelCase },
{ "foo_bar", "fooBar", camelCase },
{ "foo bar", "fooBar", camelCase },
{ "FOO-BAR", "fOOBAR", camelCase }, // camelize doesn't support uppercase
Copy link
Member

Choose a reason for hiding this comment

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

we will try to fix that in another PR.

@wing328 wing328 merged commit 8e98671 into OpenAPITools:master Nov 7, 2023
16 checks passed
@wing328
Copy link
Member

wing328 commented Nov 7, 2023

LGTM. Let's give it a try.

Thanks again for the PR.

@wing328 wing328 added this to the 7.1.0 milestone Nov 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants