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

Retain leading underscores in enum case names #123

Conversation

antonnyman
Copy link
Contributor

Running the Codegen on our GraphQL endpoint failed with the error:

The operation couldn’t be completed. (SwiftFormat.SwiftFormatError error 2.)
Error: There was an error formatting the code. Make sure your Swift version (i.e. `swift-format`) matches the `swift-format` version. If you need any help, don't hesitate to open an issue and include the log above!

I did some investigation and found out it was because the generated enums contained integers as identifiers yielding the error:

Identifier can only start with a letter or underscore, not a number

The generated code looks like this:

extension Enums {
    /// CDLType
    public enum CdlType: String, CaseIterable, Codable {

case unknown = "UNKNOWN"

case a = "A"

case az = "AZ"

case b = "B"

case c = "C"

case d = "D"

case e = "E"

case 1 = "_1"

case 2 = "_2"

case 3 = "_3"

case 4 = "_4"

case 5 = "_5"

case unrecognized = "UNRECOGNIZED"
    }
}

This PR checks if the identifier is an integer and will prepend an underscore if it is, resulting in:

extension Enums {
    /// CDLType
    public enum CdlType: String, CaseIterable, Codable {

case unknown = "UNKNOWN"

case a = "A"

case az = "AZ"

case b = "B"

case c = "C"

case d = "D"

case e = "E"

case _1 = "_1"

case _2 = "_2"

case _3 = "_3"

case _4 = "_4"

case _5 = "_5"

case unrecognized = "UNRECOGNIZED"
    }
}

Now, I don't know if my solution is any good - I am very new to Swift. Any feedback is appreciated.

@vercel
Copy link

vercel bot commented Apr 10, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
swift-graphql ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 14, 2023 7:29pm

…r-is-an-integer

Prepend an underscore to the identifier if it is an integer
@maticzav
Copy link
Owner

hey 👋 ,

Thank you for making this PR. Before I merge it in, could you help me understand what kind of enum declaration you are using. I thought the GraphQLSpec forbids the use of digits-only enum value names (http://spec.graphql.org/October2021/#sec-Enums).

@antonnyman
Copy link
Contributor Author

Hey @maticzav,

Thanks for considering my PR and for pointing me to the spec. 🙂

It's odd, our schema's enum looks like this:

enum CDLType {
  UNKNOWN
  A
  AZ
  B
  C
  D
  E
  _1
  _2
  _3
  _4
  _5
  UNRECOGNIZED
}

so I'm unsure why the generated code doesn't have the underscore. Any ideas?
I'll take a second look and see if there's anything obvious.

@antonnyman antonnyman changed the title Prepend an Underscore to the Identifier if it is an Integer Retain underscores to the value name if it is the first char Apr 15, 2023
@antonnyman
Copy link
Contributor Author

Hey @maticzav,

I've updated my PR to allow for all prepending underscores according to the GraphQL spec.

The issue was here:

extension String {
  public var pascalCase: String {
  // ...
  // line 22: 

  // Skip all special characters.
  guard char.isLetter else {
    capitalize = true
    isAbbreviation = false
    continue
  } 

  // ...
  }
}

which would strip all special chars.

@maticzav maticzav changed the title Retain underscores to the value name if it is the first char Retain leading underscores in the field name May 2, 2023
@maticzav maticzav changed the title Retain leading underscores in the field name Retain leading underscores in enum case names May 2, 2023
@maticzav
Copy link
Owner

maticzav commented May 2, 2023

Excuse my absence. I am really thankful that you took time and created this PR. Now that I spent some time pondering and investigating the issue, I think we should do more than just handle this case. Ideally, we would actually rename camelCase method to camelCasePreservingLeadingUnderscores and use the new method everywhere in place of the existing one.

I think this is the way to go because from my new understanding of the spec, any object field name might encounter the same problem and only fixing the issue in enums is going to lead to code duplication.

It's also quite likely that #113 is related to this issue.

@maticzav maticzav self-assigned this May 2, 2023
@antonnyman
Copy link
Contributor Author

Sounds good!

I see that you self assigned to this issue. Feel free to close this PR or continue working on it to consider the cases you mentioned.

Anyway, this was a good exercise since I'm fairly new to Swift.

@shaps80
Copy link
Collaborator

shaps80 commented Oct 4, 2023

Hey there 👋

I'm having a similar (but not identical) issue, some of my enum cases have a trailing underscore which is being omitted causing duplicate enum cases:

case space = "space"
case space = "space_"

Would I be correct in assuming this is related?

@shaps80
Copy link
Collaborator

shaps80 commented Oct 4, 2023

Hey there 👋

I'm having a similar (but not identical) issue, some of my enum cases have a trailing underscore which is being omitted causing duplicate enum cases:

case space = "space"
case space = "space_"

Would I be correct in assuming this is related?


EDIT

I really needed this fix so I forked and created a new PR (#164) that resolves all of the above but perhaps more completely, our Schema had slightly more complex requirements that I believe are still spec-compliant. Please let me know on the PR is I'm wrong here.

@shaps80 shaps80 closed this in #164 Oct 18, 2023
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

Successfully merging this pull request may close these issues.

3 participants