Enable enumPropertyNaming for java generators

[vad-systems]

Copied the enumPropertyNaming functionality from the Kotlin generator, with some adjustments to make it compliant with the current implementation of the enumPropertyNaming=UPPERCASE default case. Most notably, this implementation handles a few further special cases and is more lenient on stripping e.g. a leading underscore.

Users experiencing a change in naming when upgrading to a >=7.7 version should be able to use the enumPropertyNaming=original to restore the prior behaviour.

Pending:

• Test cases and samples
• Local check against cases broken on 7.6 -> 7.7 upgrade
•  Explicit test cases for the cases that changed between 7.6 and 7.7
• Validate new original implementation restores old behaviour to a reasonable extent

Could potentially fix #19066 and resolve #18987

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
  

  (For Windows users, please run the script in Git BASH)
  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*.
  IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
• File the PR against the correct branch: master (upcoming 7.8.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.

[jpfinne]

It is good to have this option in the java generators.

Is original the option to generate upper case names without underscore (like in 7.5.0)?

I don't like the name original because it will not mean anything in the future. Maybe ALPHANUMERIC_UPPERCASE or UPPERCASE_WITHOUT_UNDERSCORE?

I wonder about he option camelCaseDollarSign. Should it be used as well for enum names?

I've worked on a similar PR, but I have no time to work on it.
Feel free to copy some code for unit tests from https://github.com/jpfinne/openapi-generator/blob/JavaEnumPropertyNaming/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java

[vad-systems]

It is good to have this option in the java generators.

Is original the option to generate upper case names without underscore (like in 7.5.0)?

I don't like the name original because it will not mean anything in the future. Maybe ALPHANUMERIC_UPPERCASE or UPPERCASE_WITHOUT_UNDERSCORE?

I wonder about he option camelCaseDollarSign. Should it be used as well for enum names?

To all of the above: For consistency's sake, I have simply repurposed the options from the Kotlin generator (as the option itself as well as its value set are defined globally) with a few adjustments in behaviour for the recently-released changes to enum property naming in 7.7 (see the bug referenced above). Therefore, I'm hesitant to modify the list of option values, and their corresponding behaviours.

"original" is (from what I can tell in the Kotlin generator) meant to mostly preserve the input value. So it would not actually convert to uppercase, but preserve case from input, I think. I'm personally not sure on this either (hence my above description re preserving the old behaviour), and would rather have a distinction between the two modes of UPPERCASE/SCREAMING_SNAKE_CASE, but again, don't want to mess with existing implementations of the option unless unavoidable. I'll think about it some more while I explore the samples and tests, but this is one of the reasons I opened the PR to discussion at this stage already.

I've worked on a similar PR, but I have no time to work on it. Feel free to copy some code for unit tests from https://github.com/jpfinne/openapi-generator/blob/JavaEnumPropertyNaming/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java

Thanks, will have a look.

[vad-systems]

I've worked on a similar PR, but I have no time to work on it.
Feel free to copy some code for unit tests from https://github.com/jpfinne/openapi-generator/blob/JavaEnumPropertyNaming/modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java

I've adjusted some of the test cases to be more in line with the Kotlin example - not entirely happy with the current results, but it's fairly consistent now at least.

[vad-systems]

As the existing samples work with the default and the unit tests cover most of the relevant options, I've omitted additional samples.

[jpfinne]

I like the distinction between UPPERCASE/SCREAMING_SNAKE_CASE.
It would solve the legacy issue

[martin-mfg]

Hi @vad-systems, thanks for the PR!

For some getEnumPropertyNaming() values, the implementation tries to skip calling toVarName. This causes some inconsistencies between the different getEnumPropertyNaming() values. E.g. sometimes "___" is transformed to "U" (or "u"), sometimes it stays unchanged: "___".
Also, by skipping toVarName, we skip resolving name mappings.
It might be better to always call toVarName, and change toVarName so that it takes getEnumPropertyNaming() into consideration. Does that make sense?

[vad-systems]

I would tend to agree. That particular part might need a rework. However, the changes made in toVarName cause some issues otherwise, which I haven't been able to resolve without a much broader rewrite. I'm certainly open to contributions to that effect.

[martin-mfg]

I have created this commit to show how the adaptions could be done. This commit covers only the "original" enumPropertyNaming value, and it might be missing some edge cases. But it should be a good starting point. And all the test cases in AbstractJavaCodegenTest.toEnumVarName_original() are working.

Does this resolve the issues you mentioned? If not, could you give a concrete example what problems are coming up?

[wing328]

thanks for the PR.

I've filed #20172 to add the option (MACRO_CASE, legacy)

please give it a try when you've time.

[martin-mfg]

I've filed #19630 to add the option (MACRO_CASE, legacy)

please give it a try when you've time.

It seems the link is wrong and the new PR is actually #20172.

[wing328]

updated the comment. thanks for pointing it out.

[wing328]

update: PR merged. please give it a try with the latest master or snapshot version.

closing this one for the time being

[jrxap]

@wing328 why was this PR not approved and merged ?

[vad-systems]

@jrxap Because I've been out of commission for a few months and hopefully the other PR resolved this issue already. I unfortunately haven't been able to check because I'm not wholly affiliated with the project in question anymore. Cursory look over the other PR indicates that it should work for most cases.