[Typescript All] Prevent usage of reserved keyword for model fields #7112
Conversation
|
Just to check that I understand the issues this PR solves correctly: does this change escape Enum values, which are reserved keywords? e.g. escapes |
|
I think enum were already escaped (I did not validate). I'm escaping model members which are reserved keywords. For example, if you have a model called Without this escape, the typescript is not compiling. Does it make more sense? :) |
| @@ -405,6 +405,10 @@ public String toEnumName(CodegenProperty property) { | |||
| if (Boolean.TRUE.equals(var.isEnum)) { | |||
| var.datatypeWithEnum = var.datatypeWithEnum.replace(var.enumName, cm.classname + "." + var.enumName); | |||
| } | |||
|
|
|||
| if (isReservedWord(var.name)) { | |||
There was a problem hiding this comment.
@JFCote Shall we do it in toVarName instead similar to other generators? e.g. https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/RubyClientCodegen.java#L453
There was a problem hiding this comment.
@wing328 As you can see here: https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/AbstractTypeScriptClientCodegen.java#L169
In typescript, the toVarName is simply returning the variable name itself without transformation. Should I change it to the same thing as javascript?
https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/JavascriptClientCodegen.java#L490
Let me know and I'll do the changes!
There was a problem hiding this comment.
Please follow the JS approach, which is used by other generators (e.g. Ruby, PHP, etc)
There was a problem hiding this comment.
Ok! Will try to commit a fix this week! Thanks for the review!
|
@wing328 Changes are done! Let me know if something is still missing! |
|
@JFCote saw your change. Do we still need the reserved word check in |
|
@wing328 Do you think it's called from somewhere else automatically? |
…use it is already called from somewhere else.
|
@wing328 I've just test it and you are right, it seems to be called from somewhere, so no need in the |
|
@JFCote looks good to me 👍 |
|
Thanks for this.
…On Wed, Dec 13, 2017, 11:47 PM William Cheng ***@***.***> wrote:
Merged #7112 <#7112>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7112 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AMPLtaWhkB1YxUKCb9c9EY-eGsZ1hHamks5tAKhTgaJpZM4Q2SVk>
.
|
|
The config Parameter modelPropertyNaming is now ignored |
|
@gennadiylitvinyuk : I see. Can you create a bug related to this. I think that changing this line Sorry about the regression @gennadiylitvinyuk |
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif 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\.3.0.0branch for changes related to OpenAPI spec 3.0. Default:master.@TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09)
Description of the PR
Reserved keywords were escaped everywhere but for this. I just add it in the
AbstractTypescriptClientCodegen. I run all the typescript sample I could find. All the changes are not related to my change because the petstore doesn't contain any reserved keyword.