Skip to content

[php][DefaultCodegen.java] Fix invalid enum const names#3524

Closed
githubERIK wants to merge 6 commits intoOpenAPITools:masterfrom
githubERIK:php-enums-prefix
Closed

[php][DefaultCodegen.java] Fix invalid enum const names#3524
githubERIK wants to merge 6 commits intoOpenAPITools:masterfrom
githubERIK:php-enums-prefix

Conversation

@githubERIK
Copy link
Copy Markdown

@githubERIK githubERIK commented Aug 1, 2019

@githubERIK
Copy link
Copy Markdown
Author

githubERIK commented Aug 8, 2019

How is it possible that some enum 1 is transformed to ENUM_INTEGER_1 while some numeric enums just stay the same (this pull request wants (EDIT: wanted) to add the prefix "N" to these cases)?

Screenshot 2019-08-08 at 12 09 42

Screenshot 2019-08-08 at 12 13 28

One potential idea: enums that are pure schema objects should get the name of the schema as their prefix.

@githubERIK
Copy link
Copy Markdown
Author

githubERIK commented Aug 8, 2019

Reverted add-N-prefix changes with aa36b80 and d07efac.

The new approach, however, does not affect the clients (the generated PHP clients still have const 0=0;) which means that the changed method toEnumVarName is not called properly.

@githubERIK
Copy link
Copy Markdown
Author

githubERIK commented Aug 9, 2019

It seems to be a good idea to debug the generated jar and start with a breakpoint in DefaultCodegen and then perform "Step Into" when "importPath→OpenAPI\Client\Model.EnumTest" appears.

Screenshot 2019-08-09 at 10 20 29

and then see what happens with outerEnumInteger

    Enum_Test:
      type: object
      required:
        - enum_string_required
      properties:
        enum_string:
          type: string
          enum:
            - UPPER
            - lower
            - ''
        enum_string_required:
          type: string
          enum:
            - UPPER
            - lower
            - ''
        enum_integer:
          type: integer
          format: int32
          enum:
            - 1
            - -1
        enum_number:
          type: number
          format: double
          enum:
            - 1.1
            - -1.2
        outerEnum:
          $ref: '#/components/schemas/OuterEnum'
        outerEnumInteger:
          $ref: '#/components/schemas/OuterEnumInteger'
        outerEnumDefaultValue:
          $ref: '#/components/schemas/OuterEnumDefaultValue'
        outerEnumIntegerDefaultValue:
          $ref: '#/components/schemas/OuterEnumIntegerDefaultValue'

d07efac leads to

Screenshot 2019-08-09 at 10 55 46

at some point.

But in the end

class OuterEnumInteger
{
    /**
     * Possible values of this enum
     */
    const 0 = 0;
    const 1 = 1;
    const 2 = 2;
    
    /**
     * Gets allowable values of the enum
     * @return string[]
     */
    public static function getAllowableEnumValues()
    {
        return [
            self::0,
            self::1,
            self::2,
        ];
    }
}

is generated.

Probably because after OuterEnumInteger is processed as part of EnumTest it is also processed later separately:

Screenshot 2019-08-09 at 12 36 12

.

@githubERIK githubERIK changed the title [PHP] Fix invalid enum const names [DefaultCodegen.java] Fix invalid enum const names Aug 9, 2019
@githubERIK
Copy link
Copy Markdown
Author

githubERIK commented Aug 9, 2019

The final approach: change postProcessModelsEnum in DefaultCodegen.java to prevent adding enum variable names that are numbers. If number is detected, add a prefix NUMBER_ (see 46b9927).

Generated all samples by executing shell scripts in bin and bin/openapi3 folders using a script generate_all_sample_clients.sh.

@jmini jmini changed the title [DefaultCodegen.java] Fix invalid enum const names [php][DefaultCodegen.java] Fix invalid enum const names Aug 14, 2019
@ybelenko
Copy link
Copy Markdown
Contributor

Is this PR is still relevant or it should be closed?

@asabramo
Copy link
Copy Markdown

asabramo commented Jul 2, 2020

Hi guys,
I see this issue was reported 2 years ago.
When I use the latest:
swagger-codegen-cli-3.0.20.jar generate -l php -i api.yaml -o generated_api

I'm still seeing this issue, which means I can't use the files as they are generated - I have to add another step manually, which beats the purpose.

Oddly enough, when I use the version that the Jetbrains plugin retrieves automatically (https://repo1.maven.org/maven2/io/swagger/codegen/v3/swagger-codegen-cli/3.0.3/swagger-codegen-cli-3.0.3.jar) , the enum problem doesn't exist (but then there's another bug - BigDecimal instead of float).

Is there a version where both issues are fixed?
Is there any workaround for the enum issue?
Thanks,
Assi

@ybelenko
Copy link
Copy Markdown
Contributor

ybelenko commented Jul 4, 2020

@asabramo It seems that you're talking about Swagger Codegen which is different project. More information you can read at: https://github.com/OpenAPITools/openapi-generator/blob/master/docs/qna.md

About bug itself, I don't know whether it fixed or not. Maybe we should add part of the spec from related issue to samples and check PHP syntax of generated models with phplint package during CI.

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.

[PHP] Invalid enum const names

5 participants