[BUG] Params named "default" get replaced with "_default", resulting in incompatible clients

[seanlaff]

trafficstars

Description

My api has a request object that contains a key called default, however when I generate client code from this spec, it gets converted to _default. Since my api expects default, the generated client code does not work.

I'm guessing this is escaping that happens in java since default is a java keyword. Is there any way around this?

Expected generated:

export interface RbacAddRoleRequest {
    default?: boolean;

Actual generated:

export interface RbacAddRoleRequest {
    _default?: boolean;

openapi-generator version

4.0.0

OpenAPI declaration file content or url

https://gist.github.com/seanlaff/d2aef0244adeeea95c1396952b28d987

Command line used for generation

docker run --rm -v ${PWD}:/local openapitools/openapi-generator-cli:v4.0.0-beta generate \
    -i /local/test.yaml \
    -g typescript-axios \
    -o /local/out/ts

[auto-labeler[bot]]

👍 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.

[jmini]

related issue: #13 - even in java there is some problem with the "default" keyword.

[wing328]

@seanlaff have you tried the modelPropertyNaming option?

	modelPropertyNaming
	    Naming convention for the property: 'camelCase', 'PascalCase', 'snake_case' and 'original', which keeps the original name (Default: camelCase)

[seanlaff]

@wing328 The original option does not prevent the underscore prefix

How I tried:

docker run --rm -v ${PWD}:/local openapitools/openapi-generator-cli:v4.0.0-beta generate \
    -i /local/test.yaml \
    -g typescript-axios \
    -o /local/out/ts \
    -DmodelPropertyNaming=original

[fgravin]

Hi, same with abstract that is renamed to _abstract in Typescript.

[jmini]

Are default and abstract reserved keyword in typescript?

If not, we should have a look where the list is defined and move java-related stuff only in the java generator part.

[eseliger]

they are keywords, but can be used for property names, function names and variables 🤔

maybe it makes sense looking at all cases where they are used in templates and see if it's safe to remove some keywords from the list

[wpatter6]

@eseliger Will this fix apply to all typescript clients or just typescript-angular? And will it apply to all prefixed properties, or just the ones listed here? I've encountered an issue with the long keyword being prefixed as _long as well, using the typescript-axios client generator.

[eseliger]

this would be for all typescript clients, but it needs more investigation if that won't break with some dialects ... Since in object property names much more keywords are allowed than for function names for example. Unfortunately, I'm really busy these weeks and won't be able to continue the work on the generator. I hope I can find some spare time end of this month

[ffMathy]

@eseliger Will this fix apply to all typescript clients or just typescript-angular? And will it apply to all prefixed properties, or just the ones listed here? I've encountered an issue with the long keyword being prefixed as _long as well, using the typescript-axios client generator.

I'm hitting this exact scenario as well.

The weird part is that changing it from _long to long manually in the generated .d.ts file does not cause a compilation error - so why is this library preventing keyword usage?

I think it is allowed as properties for interfaces, so I don't know why we are trying to prevent that.

Also, if it isn't allowed, can't we use square bracket notation?

E.g.:

interface Foo {
    ["long"]: number;
}

[jfsiii]

One more data point. I found this when looking into why typescript-node & -fetch switched package to _package for ~~interface~~ class properties

[seanlaff]

I use a dirty sed to get around this for now gsed -i 's/_default?:/default?:/' src/models/*;

[yuhr]

Generators shouldn't touch the property names. This behavior is simply BREAKING users' definitions and semantics. Please consider using the computed key syntax e.g. ["default"] no matter whether it's a keyword or not.

[ffMathy]

I switched to Autorest now. Works like a charm, and is so much more stable and reliable. It also supports way more features it seems.

[tdetugny]

Hi @eseliger, I'm going to ask you again, because I don't understand and I need to understand the point of your modification https://github.com/OpenAPITools/openapi-generator/pull/3468/commits/2512ea071af34e73dbd9d932b5d95a0bad08ba8d

I agree with @ffMathy. I changed export to _export in a ts file and was able to compile and obtain the value acl.export without any problem. Both work, but the first is a real problem because the PATCH api doesn't use _export but export.

export interface ApiAclDto {
  read?: boolean;
  write: boolean;
  _export?: boolean;
}


export interface ApiAclDto {
  read?: boolean;
  write: boolean;
  export?: boolean;
} 

please help us :)

[wing328]

some generators support the name mapping features: https://github.com/openapitools/openapi-generator/blob/master/docs/customization.md#name-mapping

so that one can customize the property naming according to the use cases

[tdetugny]

thank you @wing328, but unfortunately not the angular-typescript generator. But that's not the root of the problem. I'd like the generator to be as close as possible to swagger's description and not for us to have to play with a complicated configuration to find what we want. Otherwise it's the same as @seanlaff's solution: gsed is enough.

[wing328]

i did a test and it works:

 java -jar modules/openapi-generator-cli/target/openapi-generator-cli.jar generate -g typescript-angular -i https://gist.githubusercontent.com/seanlaff/d2aef0244adeeea95c1396952b28d987/raw/b508eb408d600bdeb39f7088c8e3d6ce184020ac/keywordparams.yaml -o /tmp/tsangular2/ --name-mappings default=default

and here is the output

export interface Pet {
    id: number;
    name: string;
    default: boolean;
    tag?: string;
}

without the name mapping option, _default was generated instead

[tdetugny]

thanks @wing328 . i made a mistake with the config. In short, all this just goes to show that the change made in https://github.com/OpenAPITools/openapi-generator/commit/2512ea071af34e73dbd9d932b5d95a0bad08ba8d to list the entire array and pass it as a parameter in nameMappings doesn't really make sense. I'd still like to understand the constraints that necessitated this change

[wing328]

did you also try setting modelPropertyNaming=original?

e.g. --additional-properties modelPropertyNaming=original in CLI

[wing328]

i'm not familiar with https://github.com/OpenAPITools/openapi-generator/commit/2512ea071af34e73dbd9d932b5d95a0bad08ba8d (done in 2019) so not much I can tell you about that change in particular.

[wing328]

UPDATE: i've filed https://github.com/OpenAPITools/openapi-generator/pull/19020

@tdetugny do you mind sharing a spec so that we can use to confirm the change is what you want?

[wing328]

should be fixed by https://github.com/OpenAPITools/openapi-generator/pull/19508