Skip to content

[Kotlin] enums should be uppercase #7466

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

Open
kennetvu opened this issue Jan 22, 2018 · 2 comments
Open

[Kotlin] enums should be uppercase #7466

kennetvu opened this issue Jan 22, 2018 · 2 comments

Comments

@kennetvu
Copy link

Description

In Kotlin enums should be uppercased, its not directly a rule in Kotlin to have enums in uppercase but all examples used in Kotlin documentation is uppercased.

See here https://kotlinlang.org/docs/reference/coding-conventions.html#property-names and https://kotlinlang.org/docs/reference/enum-classes.html

Swagger-codegen version

2.3.1

Steps to reproduce

I dupliacted integrationtests for scala in /swagger-codegen/modules/swagger-codegen/src/test/resources/integrationtests/scala and created kotlin test, and then I ran /swagger-codegen/modules/swagger-codegen/src/test/resources/integrationtests/kotlin/required-attributes.sh

Related issues/PRs

Related to #5769

Suggest a fix/enhancement
protected CodegenConstants.ENUM_PROPERTY_NAMING_TYPE enumPropertyNaming = CodegenConstants.ENUM_PROPERTY_NAMING_TYPE.camelCase;

in KotlinClientCodegen.java should be changed to

protected CodegenConstants.ENUM_PROPERTY_NAMING_TYPE enumPropertyNaming = CodegenConstants.ENUM_PROPERTY_NAMING_TYPE.UPPERCASE;
@jimschubert
Copy link
Contributor

I'm glad you bring this up, because I spent quite a bit of time when I was writing the kotlin client generator worrying about which casing to use. That's why I ended up making it a generator option. I'd like to open a discussion with my thoughts below, if that's cool.

The new official Kotlin style guide actually provides two options:

For enum constants, it's OK to use either uppercase underscore-separated names (enum class Color { RED, GREEN }) or regular camel-humps names starting with an uppercase letter, depending on the usage.

IntelliJ's recommendation leans more toward title cased:

image

As I've seen in most code, uppercase is used more when it doesn't directly represent some string value (or, in other words when you consider it a constant like 'blue is always blue' rather than an attribute or state like 'active' or 'inactive'). You can see the example in the above quote for the Color enum. Most examples I've seen have been title cased (PascalCase option in the generator) when there is an underlying enum value.

You can, in fact, see the title cased and upper cased being used in projects created by Kotlin engineers at Jetbrains. For example:

Ultimately, the reason why I chose camelCase as the default is to help facilitate codegen. Considering it's an option to the generator, and the generator supports custom templates, I didn't see a need to spend much more time than I had spent on working out the best option (which was about 2 hours of trial & error testing).

Here's an example of the situation…

As the generator evolves, we'll want to have support for defaulted values and examples and whatnot. If an enum is defined as camel case in a swagger specification (which is pretty common, along with lowercase)… we can easily do Enum.valueOf(value) to assign enums:

import io.swagger.client.models.Pet

val x = Pet.Status.available
print(x)
val y = Pet.Status.valueOf("available")
print(y)
available
available

But, if the enum name is PascalCase or UPPERCASE, you can't easily create a template that iterates over enum values in lower case and call Enum.valueOf(value). You end up with Pet.Status defined as:

    enum class Status(val value: kotlin.Any){
    
        Available("available"),
    
        Pending("pending"),
    
        Sold("sold");
    
    }

And the tooling can't cleanly construct this enum:

import io.swagger.client.models.Pet

val x = Pet.Status.Available
print(x)
val y = Pet.Status.valueOf("available")
print(y)
Availablejava.lang.IllegalArgumentException: No enum constant io.swagger.client.models.Pet.Status.available
	at java.lang.Enum.valueOf(Enum.java:238)
	at io.swagger.client.models.Pet$Status.valueOf(Pet.kt)

As another example, suppose someone's swagger definition uses all UPPERCASE for enum values. The strings applied to the enum values will all be uppercase and the default enum names would be lowercase. Because there's a generator option, users can easily change this to UPPERCASE and unblock client generation. There aren't utilities in place currently to change the value of the enum. This is really the only situation currently in which the user generating the client would want uppercase as the default.

I've created some helper lambdas in #7412 for the first kotlin-server generator (ktor). I think we could easily solve the assignment issue as well as the enum naming issue if, after that PR is merged, I create lambdas for camelCase and snake_case. I can then work on any assignments or defaulted values for enums in the client using those lambdas based on the enum property naming convention.

Also, I'd recommend that if we change the default, it should be to PascalCase so this matches the recommendation in IntelliJ. Whatever we change it to will be a breaking change, so I'd rather change it on a major version like 3.0.0.

What are your thoughts on all that (sorry, I know it was a lot)?

@aml2610
Copy link

aml2610 commented Apr 16, 2019

@kennetvu FWIW you can get what you want by supplying the x-enum-varnames property in the API spec enum e.g:

    ContactMethod:
      type: string
      enum:
        - EMAIL_ADDRESS
        - PHONE_NUMBER
      x-enum-varnames:
        - EMAIL_ADDRESS
        - PHONE_NUMBER

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants