-
-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Add swift5 option for generating frozen enums #11013
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
Changes from 4 commits
0a6d4da
bd8cba2
96a8b4b
1d6efb3
9280e55
2950d0d
219e3d5
8113ed9
869499e
0e873a9
6eab5c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| generatorName: swift5 | ||
| outputDir: samples/client/petstore/swift5/frozenEnums | ||
| inputSpec: modules/openapi-generator/src/test/resources/2_0/swift/petstore-with-fake-endpoints-models-for-testing.yaml | ||
| templateDir: modules/openapi-generator/src/main/resources/swift5 | ||
| generateAliasAsModel: true | ||
| additionalProperties: | ||
| podAuthors: "" | ||
| podSummary: PetstoreClient | ||
| sortParamsByRequiredFlag: false | ||
| generateFrozenEnums: false | ||
| projectName: PetstoreClient | ||
| podHomepage: https://github.com/openapitools/openapi-generator |
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||||||||||||
| // Protocols.swift | ||||||||||||||||
| // | ||||||||||||||||
| // Generated by openapi-generator | ||||||||||||||||
| // https://openapi-generator.tech | ||||||||||||||||
| // | ||||||||||||||||
|
|
||||||||||||||||
| /// An enum where the last case value can be used as a default catch-all. | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are some protocols already defined in the models.mustache files, I think, so I'm not sure if we should create a new file or put this one with the others that already exist.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is an example openapi-generator/modules/openapi-generator/src/main/resources/swift5/Models.mustache Lines 9 to 15 in 9280e55
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's fair — I wasn't really sure what was recommended or preferred. I don't have a preference, so I'm fine to move it — will take care of shortly |
||||||||||||||||
| public protocol CaseIterableDefaultsLast: Decodable & CaseIterable & RawRepresentable | ||||||||||||||||
| where RawValue: Decodable, AllCases: BidirectionalCollection {} | ||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,12 @@ | ||
| {{#nonPublicApi}}internal{{/nonPublicApi}}{{^nonPublicApi}}public{{/nonPublicApi}} enum {{classname}}: {{dataType}}, {{#useVapor}}Content, Hashable{{/useVapor}}{{^useVapor}}Codable{{/useVapor}}, CaseIterable { | ||
| {{#nonPublicApi}}internal{{/nonPublicApi}}{{^nonPublicApi}}public{{/nonPublicApi}} enum {{classname}}: {{dataType}}, {{#useVapor}}Content, Hashable{{/useVapor}}{{^useVapor}}Codable{{/useVapor}}, CaseIterable{{^generateFrozenEnums}}{{#vendorExtensions.x-non-frozen-enum-capable}}, CaseIterableDefaultsLast{{/vendorExtensions.x-non-frozen-enum-capable}}{{/generateFrozenEnums}} { | ||
| {{#allowableValues}} | ||
| {{#enumVars}} | ||
| case {{{name}}} = {{{value}}} | ||
| {{/enumVars}} | ||
| {{/allowableValues}} | ||
| {{^generateFrozenEnums}} | ||
| {{#vendorExtensions.x-non-frozen-enum-capable}} | ||
| case unknownDefaultOpenApi{{#vendorExtensions.x-non-frozen-enum-int}} = -152915291529{{/vendorExtensions.x-non-frozen-enum-int}}{{#vendorExtensions.x-non-frozen-enum-string}} = "unknown_default_open_api"{{/vendorExtensions.x-non-frozen-enum-string}} | ||
| {{/vendorExtensions.x-non-frozen-enum-capable}} | ||
| {{/generateFrozenEnums}} | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,12 @@ | ||
| {{#nonPublicApi}}internal{{/nonPublicApi}}{{^nonPublicApi}}public{{/nonPublicApi}} enum {{enumName}}: {{^isContainer}}{{dataType}}{{/isContainer}}{{#isContainer}}String{{/isContainer}}, {{#useVapor}}Content, Hashable{{/useVapor}}{{^useVapor}}Codable{{/useVapor}}, CaseIterable { | ||
| {{#nonPublicApi}}internal{{/nonPublicApi}}{{^nonPublicApi}}public{{/nonPublicApi}} enum {{enumName}}: {{^isContainer}}{{dataType}}{{/isContainer}}{{#isContainer}}String{{/isContainer}}, {{#useVapor}}Content, Hashable{{/useVapor}}{{^useVapor}}Codable{{/useVapor}}, CaseIterable{{^generateFrozenEnums}}{{#vendorExtensions.x-non-frozen-enum-capable}}, CaseIterableDefaultsLast{{/vendorExtensions.x-non-frozen-enum-capable}}{{/generateFrozenEnums}} { | ||
| {{#allowableValues}} | ||
| {{#enumVars}} | ||
| case {{{name}}} = {{{value}}} | ||
| {{/enumVars}} | ||
| {{/allowableValues}} | ||
| {{^generateFrozenEnums}} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jarrodparkes please apply all the feedback from |
||
| {{#vendorExtensions.x-non-frozen-enum-capable}} | ||
| case unknownDefaultOpenApi{{#vendorExtensions.x-non-frozen-enum-int}} = -11184809 // -(Int.min / 192){{/vendorExtensions.x-non-frozen-enum-int}}{{#vendorExtensions.x-non-frozen-enum-string}} = "unknown_default_open_api"{{/vendorExtensions.x-non-frozen-enum-string}} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that you put the comment here, and I think we should expand it a bit so that it's explicite on why this case is here. This enum case is a fallback generated by OpenAPI, in case the server adds new enum cases that are unknown by old versions of the client. The raw value of this case is a dummy and it tries to use a value that avoids collisions. And if its a number, put the precious message and also add the following: The formula used to generate it is Int.min/192 (the Swift Evolution issue number for frozen/non-frozen enums) Note: English is not my main language, and this message ended up being very big, so fell free to improve it, shorten it or write a completely different one. Or maybe this is overkill and unnecessary? What do you think?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like it, will add shortly 😄 |
||
| {{/vendorExtensions.x-non-frozen-enum-capable}} | ||
| {{/generateFrozenEnums}} | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| // Protocols.swift | ||
| // | ||
| // Generated by openapi-generator | ||
| // https://openapi-generator.tech | ||
| // | ||
|
|
||
| /// An enum where the last case value can be used as a default catch-all. | ||
| public protocol CaseIterableDefaultsLast: Decodable & CaseIterable & RawRepresentable | ||
| where RawValue: Decodable, AllCases: BidirectionalCollection {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| // Protocols.swift | ||
| // | ||
| // Generated by openapi-generator | ||
| // https://openapi-generator.tech | ||
| // | ||
|
|
||
| /// An enum where the last case value can be used as a default catch-all. | ||
| public protocol CaseIterableDefaultsLast: Decodable & CaseIterable & RawRepresentable | ||
| where RawValue: Decodable, AllCases: BidirectionalCollection {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| // Protocols.swift | ||
| // | ||
| // Generated by openapi-generator | ||
| // https://openapi-generator.tech | ||
| // | ||
|
|
||
| /// An enum where the last case value can be used as a default catch-all. | ||
| public protocol CaseIterableDefaultsLast: Decodable & CaseIterable & RawRepresentable | ||
| where RawValue: Decodable, AllCases: BidirectionalCollection {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| // Protocols.swift | ||
| // | ||
| // Generated by openapi-generator | ||
| // https://openapi-generator.tech | ||
| // | ||
|
|
||
| /// An enum where the last case value can be used as a default catch-all. | ||
| public protocol CaseIterableDefaultsLast: Decodable & CaseIterable & RawRepresentable | ||
| where RawValue: Decodable, AllCases: BidirectionalCollection {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since now we are going to assign values to the enums, can we remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe. Let me push the next update. I think we'll want to know the enum's type in order to assign a default value that is correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any enum type that we don't support?
I think there is an easier way to do this, by removing this block of code and do this type check in the mustache files.
I think there are checks like isString, isInt, isDouble, isNumber, but I need to check tomorrow, because I'm not 100% sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@4brunu You are correct. I had tried this earlier, but was using the mustache clauses incorrectly. I made the change to remove the custom vendor extensions and use the built-in type checks. I think we should be good now