[typescript] Prevent generating invalid enum code due to empty variable names

[moznion]

After sanitizing all characters (e.g. multibyte characters), the enum variable name may become an empty string. Since an empty string would cause a syntax error, this patch pads the pseudo variable name (STRING) to avoid that issue.

For example, given the following OpenAPI definition:

openapi: "3.0.0"
info:
  title: Sample project
  version: '1.0'
  description: 'Sample API Check "API Key" '
  license:
    name: Apache 2.0
    url: 'https://www.apache.org/licenses/LICENSE-2.0'
paths: {}
components:
  schemas:
    Greeting:
      type: string
      enum:
        - 'こんにちは'
        - '你好'
        - '안녕하세요'

The current logic generates the following code for Greeting:

export enum Greeting {
     = 'こんにちは',
    2 = '你好',
    3 = '안녕하세요'
}

This code is invalid. With this patch, the generated code becomes:

export enum Greeting {
    STRING = 'こんにちは',
    STRING2 = '你好',
    STRING3 = '안녕하세요'
}

cc. @joscha

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 || exit
  ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
  ./bin/utils/export_docs_generators.sh || exit
  

  (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.x.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.

[moznion]

I realize that this change has the potential to break client code if the order of enum values changes. For example, consider the following initial enum:

      enum:
        - 'こんにちは'
        - '你好'

which generates the code:

export enum Greeting {
    STRING = 'こんにちは',
    STRING2 = '你好'
}

However, if a new item is prepended:

      enum:
        - '안녕하세요'
        - 'こんにちは'
        - '你好'

the generated code becomes:

export enum Greeting {
    STRING = '안녕하세요',
    STRING2 = 'こんにちは',
    STRING3 = '你好'
}

Thus, any client code that relies on past STRING and/or STRING2 would break.

[joscha]

Typescript supports multibyte enum names I think, doesn't it? Can/should we not just change the sanitization logic in this case?

[moznion]

Typescript supports multibyte enum names

Yes, that’s true, but personally, I wouldn’t like to use multi-byte characters for variable names (including enum key names). I mean, even if an enum value consists of multi-byte characters, it would be nicer if an ASCII string serves as its identifier.

Anyway, using a multi-byte string as a key is a good idea, but if we chose that approach, I guess we would need to implement an additional sanitizer for that purpose.

[joscha]

We don't really know what the values are, but if we use them as-is they will stay stable as long as the source stays stable. So I think another sanitizer would be best. Possibly prefixing with _ followed by the multibyte string would be enough?
@macjohnny what are your thoughts on this one, please?

[moznion]

@joscha I agree with that. I think we might need to apply the new sanitizer only in this case (i.e., for all multi-byte strings) to maintain backward compatibility.

[wing328]

@moznion some other generators support the enumNameMappings option for this use case

What about updating typescript client generator with enum name mapping support instead?

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

cc @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @topce (2018/10) @akehir (2019/07) @petejohansonxo (2019/11) @amakhrov (2020/02) @davidgamero (2022/03) @mkusaka (2022/04) @joscha (2024/10)

[moznion]

@wing328
The enumNameMappings approach appears to be a solution, but that would mean specifying a rename mapping for every multi-byte item name using --name-mappings, right?
We have a large number of such names in our OpenAPI definitions, so that would be quite challenging for us.

So, as @joscha suggested, introducing another sanitizer for the TypeScript generator sounds good to me.

[moznion]

I just tried introducing new sanitizer: edc3b9d

[joscha]

The enumNameMappings approach appears to be a solution, but that would mean specifying a rename mapping for every multi-byte item name using --name-mappings, right?

a shame the enum name mappings don't take a function - that could have been quite good otherwise. You could pass identity for multibyte strings, e.g. it would look like this:

enumNameMappings = (enumName, originalSanitizer) => {
  if(isMultiByteEnumName(enumName)) {
    return `_${enumName}`;
  }
  return originalSanitizer(enumName);
}

this would scale much better long-term as well. Do we have a precedent for an option with a lambda callback or similar like this @wing328 ?