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

Idiomatic naming strategy as opt-in #679

Merged
merged 39 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
140f5c0
[Experiment] Optimistic naming strategy as opt-in
czechboy0 Nov 13, 2024
24a97e6
Merge branch 'main' into hd-naming-strategy-optimistic
czechboy0 Nov 22, 2024
d9b901e
Remove unneeded code
czechboy0 Nov 22, 2024
3ded3fe
Updated draft proposal
czechboy0 Nov 22, 2024
4303842
Update SOAR-0013.md
czechboy0 Nov 25, 2024
6c0f05c
PR feedback
czechboy0 Nov 26, 2024
33ef5ea
Merge branch 'hd-naming-strategy-optimistic' of github.com:czechboy0/…
czechboy0 Nov 26, 2024
d6c037c
Proposal updates
simonjbeaumont Nov 27, 2024
8f51c49
Remove proposal, broken out into #683
czechboy0 Nov 27, 2024
7cbca17
Update Petstore fixtures to use idiomatic naming
czechboy0 Nov 27, 2024
cdcaed7
Merge branch 'main' into hd-naming-strategy-optimistic
czechboy0 Nov 27, 2024
c41d3de
Updated Petstore tests
czechboy0 Nov 27, 2024
bda0eb3
Updating tests
czechboy0 Nov 27, 2024
14b090a
Update the implementation with the latest feedback on the proposal
czechboy0 Nov 28, 2024
013f463
Merge branch 'main' into hd-naming-strategy-optimistic
czechboy0 Dec 2, 2024
acb471e
Add test cases for acronyms
czechboy0 Dec 2, 2024
4bd4904
PR feedback
czechboy0 Dec 10, 2024
94173c7
Merge branch 'main' into hd-naming-strategy-optimistic
czechboy0 Dec 10, 2024
6d98aa4
Clarify SwiftNameOptions
czechboy0 Dec 10, 2024
fa1b0fa
Merge branch 'main' into hd-naming-strategy-optimistic
czechboy0 Dec 11, 2024
a098cd1
Update docs
czechboy0 Dec 11, 2024
fd62811
Bump the runtime version in test
czechboy0 Dec 11, 2024
d5ace32
Merge branch 'main' into hd-naming-strategy-optimistic
czechboy0 Dec 16, 2024
21e5f05
Reformatted
czechboy0 Dec 16, 2024
3da75b8
Apply suggestions from code review
czechboy0 Dec 16, 2024
cda4a4c
Additional test cases
czechboy0 Dec 16, 2024
69de944
Some PR feedback, some temporary updates to snippet reference tests (…
czechboy0 Dec 16, 2024
dbcd035
Remove SwiftNameOptions.Capitalization
czechboy0 Dec 16, 2024
8567ad3
Undo changes to tutorial Package.swift
czechboy0 Dec 16, 2024
96f90be
Undo changes to the filter command
czechboy0 Dec 16, 2024
bd28e53
Added + as a word separator
czechboy0 Dec 16, 2024
4cb7b14
Revert snippet tests changes, add only specific new tests
czechboy0 Dec 17, 2024
837e6ed
Formatting
czechboy0 Dec 17, 2024
af9b5ff
Update test now that we also handle plus signs in the idiomatic strategy
czechboy0 Dec 17, 2024
9c9f657
Always run the string through the defensive strategy after finishing …
czechboy0 Dec 17, 2024
4d4e9b6
Test fixes
czechboy0 Dec 17, 2024
603970c
Start of refactor of SafeNameGenerator
czechboy0 Dec 18, 2024
762925d
Finished refactoring
czechboy0 Dec 18, 2024
6763372
Merge branch 'main' into hd-naming-strategy-optimistic
simonjbeaumont Dec 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions Sources/_OpenAPIGeneratorCore/Config.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,11 @@ public enum NamingStrategy: String, Sendable, Codable, Equatable {

/// A defensive strategy that can handle any OpenAPI identifier and produce a non-conflicting Swift identifier.
///
/// This strategy is the default in Swift OpenAPI Generator 1.x.
///
/// Introduced in [SOAR-0001](https://swiftpackageindex.com/apple/swift-openapi-generator/documentation/swift-openapi-generator/soar-0001).
case defensive

/// An idiomatic strategy that produces Swift identifiers that more likely conform to Swift conventions.
///
/// Opt-in since Swift OpenAPI Generator 1.6.0.
///
/// Introduced in [SOAR-0013](https://swiftpackageindex.com/apple/swift-openapi-generator/documentation/swift-openapi-generator/soar-0013).
case idiomatic
}
Expand Down Expand Up @@ -56,10 +52,10 @@ public struct Config: Sendable {
/// The naming strategy to use for deriving Swift identifiers from OpenAPI identifiers.
///
/// Defaults to `defensive`.
public var namingStrategy: NamingStrategy?
public var namingStrategy: NamingStrategy

/// A map of OpenAPI identifiers to desired Swift identifiers, used instead of the naming strategy.
public var nameOverrides: [String: String]?
public var nameOverrides: [String: String]

/// Additional pre-release features to enable.
public var featureFlags: FeatureFlags
Expand All @@ -80,8 +76,8 @@ public struct Config: Sendable {
access: AccessModifier,
additionalImports: [String] = [],
filter: DocumentFilter? = nil,
namingStrategy: NamingStrategy? = nil,
nameOverrides: [String: String]? = nil,
namingStrategy: NamingStrategy = .defensive,
nameOverrides: [String: String] = [:],
featureFlags: FeatureFlags = []
) {
self.mode = mode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,27 +16,14 @@ import Foundation
/// Extra context for the `safeForSwiftCode_` family of functions to produce more appropriate Swift identifiers.
struct SwiftNameOptions {

/// An option for controlling capitalization.
///
/// Generally, type names are capitalized, for example: `Foo`.
/// And member names are not capitalized, for example: `foo`.
enum Capitalization {

/// Capitalize the name, used for type names, for example: `Foo`.
case capitalized

/// Don't capitalize the name, used for member names, for example: `foo`.
case noncapitalized
}

/// The capitalization option.
var capitalization: Capitalization
var isCapitalized: Bool

/// Preset options for capitalized names.
static let capitalized = SwiftNameOptions(capitalization: .capitalized)
static let capitalized = SwiftNameOptions(isCapitalized: true)

/// Preset options for non-capitalized names.
static let noncapitalized = SwiftNameOptions(capitalization: .noncapitalized)
static let noncapitalized = SwiftNameOptions(isCapitalized: false)
}

extension String {
Expand Down Expand Up @@ -85,8 +72,8 @@ extension String {

let validString = String(UnicodeScalarView(sanitizedScalars))

//Special case for a single underscore.
//We can't add it to the map as its a valid swift identifier in other cases.
// Special case for a single underscore.
// We can't add it to the map as its a valid swift identifier in other cases.
if validString == "_" { return "_underscore_" }

guard Self.keywords.contains(validString) else { return validString }
Expand All @@ -101,8 +88,9 @@ extension String {
///
/// Check out [SOAR-0013](https://swiftpackageindex.com/apple/swift-openapi-generator/documentation/swift-openapi-generator/soar-0013) for details.
func safeForSwiftCode_idiomatic(options: SwiftNameOptions) -> String {
let capitalize = options.capitalization == .capitalized
let capitalize = options.isCapitalized
if isEmpty { return capitalize ? "_Empty_" : "_empty_" }

// Detect cases like HELLO_WORLD, sometimes used for constants.
let isAllUppercase = allSatisfy {
// Must check that no characters are lowercased, as non-letter characters
Expand All @@ -111,7 +99,8 @@ extension String {
}

// 1. Leave leading underscores as-are
// 2. In the middle: word separators: ["_", "-", "/", <space>] -> remove and capitalize next word
// 2. In the middle: word separators: ["_", "-", "/", "+", <space>] -> remove and capitalize
// next word
// 3. In the middle: period: ["."] -> replace with "_"
// 4. In the middle: drop ["{", "}"] -> replace with ""

Expand All @@ -124,7 +113,6 @@ extension String {
case accumulatingFirstWord(AccumulatingFirstWordContext)
case accumulatingWord
case waitingForWordStarter
case fallback
}
var state: State = .preFirstWord
for index in self[...].indices {
Expand All @@ -138,8 +126,7 @@ extension String {
buffer.append(char)
state = .preFirstWord
} else if char.isNumber {
// Prefix with an underscore if the first character is a number.
buffer.append("_")
// The underscore will be added by the defensive strategy.
buffer.append(char)
state = .accumulatingFirstWord(.init(isAccumulatingInitialUppercase: false))
} else if char.isLetter {
Expand All @@ -149,8 +136,9 @@ extension String {
.init(isAccumulatingInitialUppercase: !capitalize && char.isUppercase)
)
} else {
// Illegal character, fall back to the defensive strategy.
state = .fallback
// Illegal character, keep and let the defensive strategy deal with it.
state = .accumulatingFirstWord(.init(isAccumulatingInitialUppercase: false))
buffer.append(char)
}
case .accumulatingFirstWord(var context):
if char.isLetter || char.isNumber {
Expand Down Expand Up @@ -201,7 +189,7 @@ extension String {
buffer.append(char)
}
state = .accumulatingFirstWord(context)
} else if ["_", "-", " ", "/"].contains(char) {
} else if ["_", "-", " ", "/", "+"].contains(char) {
// In the middle of an identifier, these are considered
// word separators, so we remove the character and end the current word.
state = .waitingForWordStarter
Expand All @@ -214,8 +202,9 @@ extension String {
// In the middle of an identifier, curly braces are dropped.
state = .accumulatingFirstWord(.init(isAccumulatingInitialUppercase: false))
} else {
// Illegal character, fall back to the defensive strategy.
state = .fallback
// Illegal character, keep and let the defensive strategy deal with it.
state = .accumulatingFirstWord(.init(isAccumulatingInitialUppercase: false))
buffer.append(char)
}
case .accumulatingWord:
if char.isLetter || char.isNumber {
Expand All @@ -231,14 +220,15 @@ extension String {
buffer.append("_")
state = .accumulatingWord
} else if ["{", "}"].contains(char) {
// In the middle of an identifier, curly braces are dropped.
// In the middle of an identifier, these are dropped.
state = .accumulatingWord
} else {
// Illegal character, fall back to the defensive strategy.
state = .fallback
// Illegal character, keep and let the defensive strategy deal with it.
state = .accumulatingWord
buffer.append(char)
}
case .waitingForWordStarter:
if ["_", "-", ".", "/", "{", "}"].contains(char) {
if ["_", "-", ".", "/", "+", "{", "}"].contains(char) {
// Between words, just drop allowed special characters, since
// we're already between words anyway.
state = .waitingForWordStarter
Expand All @@ -247,23 +237,19 @@ extension String {
buffer.append(contentsOf: char.uppercased())
state = .accumulatingWord
} else {
// Illegal character, fall back to the defensive strategy.
state = .fallback
// Illegal character, keep and let the defensive strategy deal with it.
state = .waitingForWordStarter
buffer.append(char)
}
case .modifying, .fallback: preconditionFailure("Logic error in \(#function), string: '\(self)'")
case .modifying: preconditionFailure("Logic error in \(#function), string: '\(self)'")
}
precondition(state != .modifying, "Logic error in \(#function), string: '\(self)'")
if case .fallback = state { return safeForSwiftCode_defensive(options: options) }
}
if buffer.isEmpty || state == .preFirstWord { return safeForSwiftCode_defensive(options: options) }
// Check for keywords
let newString = String(buffer)
if Self.keywords.contains(newString) { return "_\(newString)" }
return newString
return String(buffer).safeForSwiftCode_defensive(options: options)
}

/// A list of word separator characters for the idiomatic naming strategy.
private static let wordSeparators: Set<Character> = ["_", "-", " ", "/"]
private static let wordSeparators: Set<Character> = ["_", "-", " ", "/", "+"]

/// A list of Swift keywords.
///
Expand Down
18 changes: 12 additions & 6 deletions Sources/_OpenAPIGeneratorCore/Translator/FileTranslator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,17 @@ extension FileTranslator {
var context: TranslatorContext {
let asSwiftSafeName: (String, SwiftNameOptions) -> String
switch config.namingStrategy {
case .defensive, .none: asSwiftSafeName = { $0.safeForSwiftCode_defensive(options: $1) }
case .defensive: asSwiftSafeName = { $0.safeForSwiftCode_defensive(options: $1) }
case .idiomatic: asSwiftSafeName = { $0.safeForSwiftCode_idiomatic(options: $1) }
}
let overrides = config.nameOverrides ?? [:]
return TranslatorContext(asSwiftSafeName: { name, options in
if let override = overrides[name] { return override }
return asSwiftSafeName(name, options)
})
let overrides = config.nameOverrides
return TranslatorContext(
asSwiftSafeName: { name, options in
if let override = overrides[name] { return override }
return asSwiftSafeName(name, options)
},
namingStrategy: config.namingStrategy
)
}
}

Expand All @@ -69,4 +72,7 @@ struct TranslatorContext {
/// - Parameter string: The string to convert to be safe for Swift.
/// - Returns: A Swift-safe version of the input string.
var asSwiftSafeName: (String, SwiftNameOptions) -> String

/// The naming strategy.
var namingStrategy: NamingStrategy
}
Original file line number Diff line number Diff line change
Expand Up @@ -531,19 +531,33 @@ struct TypeAssigner {
default:
let safedType = context.asSwiftSafeName(contentType.originallyCasedType, .noncapitalized)
let safedSubtype = context.asSwiftSafeName(contentType.originallyCasedSubtype, .noncapitalized)
let prefix = "\(safedType)_\(safedSubtype)"
let componentSeparator: String
let capitalizeNonFirstWords: Bool
switch context.namingStrategy {
case .defensive:
componentSeparator = "_"
capitalizeNonFirstWords = false
case .idiomatic:
componentSeparator = ""
capitalizeNonFirstWords = true
}
let prettifiedSubtype = capitalizeNonFirstWords ? safedSubtype.uppercasingFirstLetter : safedSubtype
let prefix = "\(safedType)\(componentSeparator)\(prettifiedSubtype)"
czechboy0 marked this conversation as resolved.
Show resolved Hide resolved
let params = contentType.lowercasedParameterPairs
guard !params.isEmpty else { return prefix }
let safedParams =
params.map { pair in
pair.split(separator: "=").map { context.asSwiftSafeName(String($0), .noncapitalized) }
.joined(separator: "_")
pair.split(separator: "=")
.map { component in
let safedComponent = context.asSwiftSafeName(String(component), .noncapitalized)
return capitalizeNonFirstWords ? safedComponent.uppercasingFirstLetter : safedComponent
}
.joined(separator: componentSeparator)
}
.joined(separator: "_")
return prefix + "_" + safedParams
.joined(separator: componentSeparator)
return prefix + componentSeparator + safedParams
}
}

}

extension FileTranslator {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ The configuration file has the following keys:
- `tags`: Operations tagged with these tags will be included in the filter.
- `paths`: Operations for these paths will be included in the filter.
- `schemas`: These (additional) schemas will be included in the filter.
- `namingStrategy` (optional): a string. Customizes the strategy of converting OpenAPI identifiers into Swift identifiers.
- `namingStrategy` (optional): a string. The strategy of converting OpenAPI identifiers into Swift identifiers.
- `defensive` (default): Produces non-conflicting Swift identifiers for any OpenAPI identifiers. Check out [SOAR-0001](https://swiftpackageindex.com/apple/swift-openapi-generator/documentation/swift-openapi-generator/soar-0001) for details.
- `idiomatic`: Produces more idiomatic Swift identifiers for OpenAPI identifiers, might produce name conflicts (in that case, switch back to `defensive`.) Check out [SOAR-0013](https://swiftpackageindex.com/apple/swift-openapi-generator/documentation/swift-openapi-generator/soar-0013) for details.
- `idiomatic`: Produces more idiomatic Swift identifiers for OpenAPI identifiers. Might produce name conflicts (in that case, switch back to `defensive`). Check out [SOAR-0013](https://swiftpackageindex.com/apple/swift-openapi-generator/documentation/swift-openapi-generator/soar-0013) for details.
- `nameOverrides` (optional): a string to string dictionary. Allows customizing how individual OpenAPI identifiers get converted to Swift identifiers.
- `featureFlags` (optional): array of strings. Each string must be a valid feature flag to enable. For a list of currently supported feature flags, check out [FeatureFlags.swift](https://github.com/apple/swift-openapi-generator/blob/main/Sources/_OpenAPIGeneratorCore/FeatureFlags.swift).

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version: 6.0
// swift-tools-version: 5.9
import PackageDescription

let package = Package(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version: 6.0
// swift-tools-version: 5.9
import PackageDescription

let package = Package(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version: 6.0
// swift-tools-version: 5.9
import PackageDescription

let package = Package(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version: 6.0
// swift-tools-version: 5.9
import PackageDescription

let package = Package(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version: 6.0
// swift-tools-version: 5.9
import PackageDescription

let package = Package(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version: 6.0
// swift-tools-version: 5.9
import PackageDescription

let package = Package(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version: 6.0
// swift-tools-version: 5.9
import PackageDescription

let package = Package(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version: 6.0
// swift-tools-version: 5.9
import PackageDescription

let package = Package(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version: 6.0
// swift-tools-version: 5.9
import PackageDescription

let package = Package(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version: 6.0
// swift-tools-version: 5.9
import PackageDescription

let package = Package(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version: 6.0
// swift-tools-version: 5.9
import PackageDescription

let package = Package(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version: 6.0
// swift-tools-version: 5.9
import PackageDescription

let package = Package(
Expand Down
6 changes: 2 additions & 4 deletions Sources/swift-openapi-generator/FilterCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,13 @@ private func timing<Output>(_ title: String, _ operation: @autoclosure () throws
}

private let sampleConfig = _UserConfig(
generate: [.types, .client],
generate: [],
filter: DocumentFilter(
operations: ["getGreeting"],
tags: ["greetings"],
paths: ["/greeting"],
schemas: ["Greeting"]
),
namingStrategy: .idiomatic,
nameOverrides: ["SPECIAL_NAME": "SpecialName"]
)
)

enum OutputFormat: String, ExpressibleByArgument {
Expand Down
8 changes: 8 additions & 0 deletions Sources/swift-openapi-generator/GenerateOptions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,16 @@ extension _GenerateOptions {
return []
}

/// Returns the naming strategy requested by the user.
/// - Parameter config: The configuration specified by the user.
/// - Returns: The naming strategy requestd by the user.
func resolvedNamingStrategy(_ config: _UserConfig?) -> NamingStrategy { config?.namingStrategy ?? .defensive }

/// Returns the name overrides requested by the user.
/// - Parameter config: The configuration specified by the user.
/// - Returns: The name overrides requested by the user
func resolvedNameOverrides(_ config: _UserConfig?) -> [String: String] { config?.nameOverrides ?? [:] }

/// Returns a list of the feature flags requested by the user.
/// - Parameter config: The configuration specified by the user.
/// - Returns: A set of feature flags requested by the user.
Expand Down
Loading
Loading