[PHP] Invalid enum const names

[Yogarine]

Description

When generating clients for the Bungie OpenAPI v2 spec, certain enum types with unnamed integer values result in invalid constant names, consisting only of integers.

What I mean, concretely, is that this:

    "GroupsV2.GroupType": {
      "format": "int32",
      "enum": [
        "0",
        "1"
      ],
      "type": "integer",
      "x-enum-values": [
        {
          "numericValue": "0",
          "identifier": "General"
        },
        {
          "numericValue": "1",
          "identifier": "Clan"
        }
      ]
    }, 

results in this:

class GroupsV2GroupType
{
    /**
     * Possible values of this enum
     */
    const 0 = 0;
    const 1 = 1;

    /**
     * Gets allowable values of the enum
     * @return string[]
     */
    public static function getAllowableEnumValues()
    {
        return [
            self::0,
            self::1,
        ];
    }
}

(note the const 0 = 0;)

For the complete spec file, check: https://raw.githubusercontent.com/Bungie-net/api/master/openapi-2.json

openapi-generator version

3.3.4-SNAPSHOT, commit 32b8d7fee7886c13a8ccd9523dd31791cdf6371b

OpenAPI declaration file content or url

https://raw.githubusercontent.com/Bungie-net/api/master/openapi-2.json

Command line used for generation

java -jar openapi-generator-cli.jar generate \
      --generator-name php \
      --config         swagger-codegen-config.php.json \
      --input-spec     https://raw.githubusercontent.com/Bungie-net/api/master/openapi-2.json \
      --output         bungie-sdk-php \
      -DapiTests=false \
      -DmodelTests=false

swagger-codegen-config.php.json:

{
  "composerVendorName": "yogarine",
  "composerProjectName": "bungie-sdk-php",
  "artifactVersion": "2.3.2",
  "invokerPackage" : "Bungie",
  "modelPackage": "Model",
  "apiPackage": "API",
  "variableNamingConvention": "camelCase",
  "packagePath": "bungie-sdk-php",
  "srcBasePath": "src",
  "gitUserId": "Yogarine",
  "gitRepoId": "bungie-sdk-php",
  "validateSpec": false
}

Steps to reproduce

Generate with the command provided above.

Related issues/PRs

None I could find.

Suggest a fix/enhancement

The same issue is present in swagger-codegen and I tried to fix it there by also prefixing integers with underscore. However this breaks existing enums. You can see my attempt here, together with generated petstore files where enum constants suddenly have double underscores. https://github.com/swagger-api/swagger-codegen/compare/master...Yogarine:fix-php-enums#diff-2f17f44f63e1286045038c552dcfcb66

I'm trying to figure out how to do this properly but I'm not really well-acquainted with the code. Perhaps someone can point me in the right direction?

@jebentier @dkarlovi @mandrean @jfastnacht @ackintosh @ybelenko

[ackintosh]

@Yogarine Thanks for reporting this issue!

The Ruby approach helps us to fix the issue. 👇 https://github.com/OpenAPITools/openapi-generator/blob/5b57eae5deb5a5583db089bfb6589542d8be76eb/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/RubyClientCodegen.java#L421-L425

---

toEnumVarName of PHP generator is here (AbstractPhpCodegen.java) on our project. 📝

[Yogarine]

Actually, that's the approach I took in my attempted fix for Swagger Codegen: https://github.com/swagger-api/swagger-codegen/compare/master...Yogarine:fix-php-enums#diff-0c6382fc2a23f96e761485e6ed6bbb9b

However instead of prefixing with an 'N' I just allow escapeReservedWord() to prefix the enum name with an underscore, which is also valid in PHP. Unfortunately that causes unintentional side effects.

Anyway, is prefixing them with an 'N' the preferred way? I believe that ideally you would want to prefix these enum names with the enum type class name, so it becomes:

class GroupsV2GroupType
{
    /**
     * Possible values of this enum
     */
    const GROUPS_V2_GROUP_TYPE_0 = 0;
    const GROUPS_V2_GROUP_TYPE_1 = 1;

[ackintosh]

const GROUPS_V2_GROUP_TYPE_0 = 0; const GROUPS_V2_GROUP_TYPE_1 = 1;

Looks good idea as that naming rule which based on enum class name is descriptive. 💡

But unfortunately I think implementing that naming rule is a bit difficult as we couldn't find the enum class name in the toEnumVarName scope.

I think prefixing N (or NUMBER_ like the Java generator) could be a solution of compromise.

---

If users want to use more descriptive enum names, they could specify the names via x-enum-varnames.

[wing328]

I agree with @ackintosh that we should go with the x-enum-varnames approach if the user wants full control.