Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed

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

wants to merge 6 commits into from

Conversation

githubERIK
Copy link

@githubERIK githubERIK commented Aug 1, 2019

@githubERIK
Copy link
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
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
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
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
Contributor

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

@asabramo
Copy link

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