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

[Java] Fix numeric field names #3436

Merged

Conversation

Fjolnir-Dvorak
Copy link
Contributor

@Fjolnir-Dvorak Fjolnir-Dvorak commented Jul 24, 2019

…nerators are escaped now.

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.
  • Filed the PR against the correct branch: master, 4.1.x, 5.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Somehow the generator started somewhen to generate broken java code if the fields in the openapi spec are staring with a number. Numbers are not allowed at the start of a word in java so they need to be escaped. This bugfix escapes all varNames which start with a number with an underscore like it was sometime before

Fixed Issues:

Technical Commitee for Java:

@bbdouglas (2017/07) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01) @karismann (2019/03) @Zomzog (2019/04)

@Fjolnir-Dvorak Fjolnir-Dvorak changed the title Fix for OpenAPITools/openapi-generator#3293. Number fields in java ge… [Java] Fix for OpenAPITools/openapi-generator#3293. Number fields in java ge… Jul 24, 2019
Copy link
Member

@macjohnny macjohnny left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -585,6 +585,11 @@ public String toVarName(String name) {
name = "_u";
}

// numbers are not allowed at the beginning
if (name.matches("^\\d.*")) {
Copy link
Member

Choose a reason for hiding this comment

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

I guess this is needed here, in addition to


if the name is all uppercase and thus matches this early return:

Copy link
Contributor

@pe-st pe-st left a comment

Choose a reason for hiding this comment

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

I guess you should add one or two examples with leading digits to the unit test convertVarName() in
modules/openapi-generator/src/test/java/org/openapitools/codegen/java/AbstractJavaCodegenTest.java

@Fjolnir-Dvorak
Copy link
Contributor Author

I will look into the tests tomorrow

@wing328 wing328 added this to the 4.1.0 milestone Jul 25, 2019
@wing328 wing328 changed the title [Java] Fix for OpenAPITools/openapi-generator#3293. Number fields in java ge… [Java] Fix numeric field name Jul 25, 2019
@wing328 wing328 changed the title [Java] Fix numeric field name [Java] Fix numeric field names Jul 25, 2019
Copy link
Member

@macjohnny macjohnny left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks for the PR!

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.

[BUG] Java generated code compilation failure
4 participants