From 6c0f05cc402b7a606148a462fe401fad45a7be3d Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Tue, 26 Nov 2024 11:56:45 +0100 Subject: [PATCH] PR feedback --- .../Documentation.docc/Proposals/SOAR-0013.md | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0013.md b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0013.md index 44832c25..b77b73c9 100644 --- a/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0013.md +++ b/Sources/swift-openapi-generator/Documentation.docc/Proposals/SOAR-0013.md @@ -1,4 +1,4 @@ -# SOAR-0013: Optimistic naming strategy +# SOAR-0013: Idiomatic naming strategy Introduce an alternative naming strategy for more idiomatic Swift identifiers, including a way to provide custom name overrides. @@ -33,6 +33,8 @@ When Swift OpenAPI Generator 0.1.0 went open-source in May 2023, it had a simple The root cause of conflicts are the different allowed character sets for OpenAPI names and Swift identifiers. OpenAPI has a more flexible allowed character set than Swift identifiers. +The existing naming strategy also avoids changing the character casing, as we discovered OpenAPI documents with properties within an object schema that only differred by case. + In response to the findings on the test corpus, the proposal [SOAR-0001: Improved mapping of identifiers][soar0001], which shipped in 0.2.0, changed the naming strategy to avoid conflicts and resulted in no conflicts produced in the test corpus, allowing hundreds of additional OpenAPI documents to be correctly handled by Swift OpenAPI Generator. The way the conflicts are avoided in the naming strategy from SOAR-0001 is by turning any special characters (any characters that aren't letters, numbers, or an underscore) into words, resulting in identifiers like: @@ -44,31 +46,27 @@ user-name -> user_hyphen_name my.org.User -> my_period_org_period_User ``` -The existing naming strategy also avoids changing the character casing, as we discovered OpenAPI documents with properties within an object schema that only differred by case. - The decision to rely on a naming strategy that can handle all the tested OpenAPI documents was the right one, and it has allowed more developers to get value from Swift OpenAPI Generator since then. However, we've also [heard][issue1] [from][issue2] [adopters][issue3] [who][issue4] don't use special characters in their OpenAPI documents, and how some of the generated Swift names are still difficult to read and are simply unpleasant to look at. -We originally believed that a fully generalized solution, such as a ["naming extension"][issuePlugin], would be the right way to go, however after further consideration, we believe we can offer a solution that solves the majority of the problem without needing to invent an extension architecture for Swift OpenAPI Generator. - ### Proposed solution We propose to introduce a second, opt-in naming strategy, which produces idiomatic Swift identifiers from arbitrary OpenAPI names, and a way to fully customize the conversion from an OpenAPI name to a Swift identifier using a string -> string map. -For clarity, we'll refer to the existing naming strategy as the "defensive" naming strategy, and to the new proposed strategy as the "optimistic" naming strategy. The names reflect the strengths of each strategy - the defensive strategy can handle any OpenAPI document and produce compiling Swift code, the optimistic naming strategy produces prettier names, but does not work for all documents, and falls back to the defensive strategy when needed on a per-name basis. +For clarity, we'll refer to the existing naming strategy as the "defensive" naming strategy, and to the new proposed strategy as the "idiomatic" naming strategy. The names reflect the strengths of each strategy - the defensive strategy can handle any OpenAPI document and produce compiling Swift code, the idiomatic naming strategy produces prettier names, but does not work for all documents, and falls back to the defensive strategy when needed on a per-name basis. Part of the new strategy is adjusting the capitalization, and producing `UpperCamelCase` names for types, and `lowerCamelCase` names for members, as is common in hand-written Swift code. -> Warning: Due to the optimistic naming strategy changing capitalization, it is possible to get non-compiling Swift code from more OpenAPI documents than with the defensive naming strategy. We recommend you try to use the optimistic naming strategy on your OpenAPI document, and if it produces conflicts, switch back to the defensive naming strategy, which avoids conflicts. However, the number of documents that result in conflicts with the optimistic naming strategy is estimated to be very small (<1%). +> Warning: Due to the idiomatic naming strategy changing capitalization, it is possible to get non-compiling Swift code from more OpenAPI documents than with the defensive naming strategy. We recommend you try to use the idiomatic naming strategy on your OpenAPI document, and if it produces conflicts, switch back to the defensive naming strategy, which avoids conflicts. However, the number of documents that result in conflicts with the idiomatic naming strategy is estimated to be very small (<1%). The second feature introduced as part of this proposal is a way to provide a string -> string map to fully override only specific OpenAPI names and provide their exact Swift identifiers. This is the ultimate escape hatch when both naming strategies fail to provide the desired result for the adopter. #### Examples -To get a sense for the proposed change, check out the table below that compares the existing defensive strategy against the proposed optimistic strategy on a set of examples: +To get a sense for the proposed change, check out the table below that compares the existing defensive strategy against the proposed idiomatic strategy on a set of examples: -| OpenAPI name | Defensive | Optimistic (capitalized) | Optimistic (non-capitalized) | +| OpenAPI name | Defensive | Idiomatic (capitalized) | Idiomatic (non-capitalized) | | ------------ | --------- | ------------------------ | ---------------------------- | | `foo` | `foo` | `Foo` | `foo` | | `Hello world` | `Hello_space_world` | `HelloWorld` | `helloWorld` | @@ -80,7 +78,7 @@ To get a sense for the proposed change, check out the table below that compares | `__user` | `__user` | `__User` | `__user` | | `order#123` | `order_num_123` | `order_num_123` | `order_num_123` | -Notice that in the last example, since the OpenAPI name contains the pound (`#`) character, the optimistic naming strategy falls back to the defensive naming strategy. In all the other cases, however, the resulting names are more idiomatic Swift identifiers. +Notice that in the last example, since the OpenAPI name contains the pound (`#`) character, the idiomatic naming strategy falls back to the defensive naming strategy. In all the other cases, however, the resulting names are more idiomatic Swift identifiers. > Tip: For more examples, check out the updated [test suite](https://github.com/czechboy0/swift-openapi-generator/blob/hd-naming-strategy-optimistic/Tests/OpenAPIGeneratorCoreTests/Extensions/Test_SwiftSafeNames.swift). @@ -88,11 +86,11 @@ Notice that in the last example, since the OpenAPI name contains the pound (`#`) This section goes into detail of the [draft implementation][pr] that you can already check out and try to run on your OpenAPI document. -> Note: To enable it, you'll need to add `namingStrategy: optimistic` to your `openapi-generator-config.yaml` file. +> Note: To enable it, you'll need to add `namingStrategy: idiomatic` to your `openapi-generator-config.yaml` file. #### Naming logic -The optimistic naming strategy (check out the current code [here][impl], look for the method `safeForSwiftCode_optimistic`) is built around the decision to _only_ optimize for names that include the following: +The idiomatic naming strategy (check out the current code [here][impl], look for the method `safeForSwiftCode_idiomatic`) is built around the decision to _only_ optimize for names that include the following: - letters - numbers @@ -103,9 +101,9 @@ The optimistic naming strategy (check out the current code [here][impl], look fo > Note: We let [`Swift.String.isLetter`](https://developer.apple.com/documentation/swift/character/isletter) decide whether a character is a letter, which has the advantage of including letters in the non-ASCII range. Swift identifiers also support a [wide range](https://docs.swift.org/swift-book/documentation/the-swift-programming-language/lexicalstructure/#Identifiers) of alphanumeric characters. -If the OpenAPI name includes any _other_ characters, the optimistic naming strategy _falls back_ to the defensive naming strategy for that input string only. +If the OpenAPI name includes any _other_ characters, the idiomatic naming strategy _falls back_ to the defensive naming strategy for that input string only. -There's a second special case for handling all uppercased names, such as `NOT_AVAILABLE` - if this situation is detected, the optimistic naming strategy turns it into `NotAvailable` for types and `notAvailable` for members. +There's a second special case for handling all uppercased names, such as `NOT_AVAILABLE` - if this situation is detected, the idiomatic naming strategy turns it into `NotAvailable` for types and `notAvailable` for members. The best way to understand the detailed logic is to check out the [code][impl], feel free to leave comments on the pull request. @@ -114,13 +112,13 @@ The best way to understand the detailed logic is to check out the [code][impl], Since Swift OpenAPI Generator is on a stable 1.x version, we cannot change the naming strategy for everyone, as it would be considered an API break. So this new naming strategy is fully opt-in using a new configuration key called `namingStrategy`, with the following allowed values: - `defensive`: the existing naming strategy introduced in 0.2.0 -- `optimistic`: the new naming strategy proposed here +- `idiomatic`: the new naming strategy proposed here - not specified: defaults to `defensive` for backwards compatibility Enabling this feature in the configuration file would look like this: ```yaml -namingStrategy: optimistic +namingStrategy: idiomatic ``` #### Name overrides