Skip to content
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

Incorrect code generation for enum cases containing numbers #2749

Closed
hispanico94 opened this issue Dec 29, 2022 · 4 comments · Fixed by #2745
Closed

Incorrect code generation for enum cases containing numbers #2749

hispanico94 opened this issue Dec 29, 2022 · 4 comments · Fixed by #2745
Assignees
Labels
bug Generally incorrect behavior codegen Issues related to or arising from code generation

Comments

@hispanico94
Copy link

Summary

We started migration to Apollo 1.0.5 (from 0.51.2), we ran the apollo-ios-cli code generation tool and then we got several compiler errors because the new tool generated a Swift enum where the cases names differs from what the 0.51.2 code generation tool generates.

If the graphql enum case name contains all uppercase letters and numbers (e.g BEFORE1940) then only the first letter of the generated Swift enum case is lowercased (.bEFORE1940) , all the other letters remains uppercased. Apollo 0.51.2 code generation would lowercase all the letters of the enum case name (.before1940).

Version

1.0.5

Steps to reproduce the behavior

Given the following graphql enum

enum HomeConstructionPeriod {
  BEFORE1940
  BEFORE1970
  DECADE1940
  DECADE1950
  DECADE1960
  DECADE1970
  DECADE1980
  DECADE1990
  DECADE2000
  DECADE2010
  DECADE2020
  UNKNOWN
}

Apollo 1.0.5 generates

enum HomeConstructionPeriod: String, EnumType {
    case bEFORE1940 = "BEFORE1940"
    case bEFORE1970 = "BEFORE1970"
    case dECADE1940 = "DECADE1940"
    case dECADE1950 = "DECADE1950"
    case dECADE1960 = "DECADE1960"
    case dECADE1970 = "DECADE1970"
    case dECADE1980 = "DECADE1980"
    case dECADE1990 = "DECADE1990"
    case dECADE2000 = "DECADE2000"
    case dECADE2010 = "DECADE2010"
    case dECADE2020 = "DECADE2020"
    case unknown = "UNKNOWN"
  }

While Apollo 0.51.2 generates

 public enum HomeConstructionPeriod: RawRepresentable, Equatable, Hashable, CaseIterable, Apollo.JSONDecodable, Apollo.JSONEncodable {
    public typealias RawValue = String
    case before1940
    case before1970
    case decade1940
    case decade1950
    case decade1960
    case decade1970
    case decade1980
    case decade1990
    case decade2000
    case decade2010
    case decade2020
    case unknown

    // ... Other code omitted ...
}

Logs

No response

Anything else?

Our naming convention for graphql enums is using snake_case with UPPERCASED words for the cases (e.g. GENERIC_ERROR). In the Swift codebase we follow the classic lowercase words and camelCase (e.g. .genericError).

All the other generated Swift enums are consistent between Apollo 1.0.5 and 0.51.2, with lowercased words and camelCase. The enum reported above is the only one we have that also contains numbers and the only one that presents the described issue with the code generation.

@hispanico94 hispanico94 added bug Generally incorrect behavior needs investigation labels Dec 29, 2022
@calvincestari
Copy link
Member

Hi 👋 - I think this is probably fixed with #2745. You should try that branch to see if it resolved the issue; it'll go out in the next version.

@hispanico94
Copy link
Author

Hi @calvincestari, thanks for the quick response. I tried regenerating the enum using the fix/enum-case-conversion-strategy branch you suggested, but sadly it didn't solve the issue. The generated enum is the same. I removed the old generated file and made sure to reinstall the CLI, the problem persists.

@calvincestari calvincestari added codegen Issues related to or arising from code generation and removed needs investigation labels Jan 3, 2023
@calvincestari calvincestari added this to the Release 1.0.6 milestone Jan 3, 2023
@calvincestari calvincestari self-assigned this Jan 3, 2023
@calvincestari
Copy link
Member

@hispanico94, I've just pushed a new commit to #2745 to handle your test case, we'll get it merged shortly.

@calvincestari
Copy link
Member

@hispanico94 - this is fixed and available on the main branch. It will be included in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Generally incorrect behavior codegen Issues related to or arising from code generation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants