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 36 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
30 changes: 30 additions & 0 deletions Sources/_OpenAPIGeneratorCore/Config.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,20 @@
//
//===----------------------------------------------------------------------===//

/// A strategy for turning OpenAPI identifiers into Swift identifiers.
public enum NamingStrategy: String, Sendable, Codable, Equatable {

/// A defensive strategy that can handle any OpenAPI identifier and produce a non-conflicting Swift identifier.
///
/// 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.
///
/// Introduced in [SOAR-0013](https://swiftpackageindex.com/apple/swift-openapi-generator/documentation/swift-openapi-generator/soar-0013).
case idiomatic
}

/// A structure that contains configuration options for a single execution
/// of the generator pipeline run.
///
Expand All @@ -35,6 +49,14 @@ public struct Config: Sendable {
/// Filter to apply to the OpenAPI document before generation.
public var filter: DocumentFilter?

/// The naming strategy to use for deriving Swift identifiers from OpenAPI identifiers.
///
/// Defaults to `defensive`.
public var namingStrategy: NamingStrategy

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

/// Additional pre-release features to enable.
public var featureFlags: FeatureFlags

Expand All @@ -44,18 +66,26 @@ public struct Config: Sendable {
/// - access: The access modifier to use for generated declarations.
/// - additionalImports: Additional imports to add to each generated file.
/// - filter: Filter to apply to the OpenAPI document before generation.
/// - namingStrategy: The naming strategy to use for deriving Swift identifiers from OpenAPI identifiers.
/// Defaults to `defensive`.
/// - nameOverrides: A map of OpenAPI identifiers to desired Swift identifiers, used instead
/// of the naming strategy.
/// - featureFlags: Additional pre-release features to enable.
public init(
mode: GeneratorMode,
access: AccessModifier,
additionalImports: [String] = [],
filter: DocumentFilter? = nil,
namingStrategy: NamingStrategy = .defensive,
nameOverrides: [String: String] = [:],
featureFlags: FeatureFlags = []
) {
self.mode = mode
self.access = access
self.additionalImports = additionalImports
self.filter = filter
self.namingStrategy = namingStrategy
self.nameOverrides = nameOverrides
self.featureFlags = featureFlags
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ extension ClientFileTranslator {
func translateClientMethod(_ description: OperationDescription) throws -> Declaration {

let operationTypeExpr = Expression.identifierType(.member(Constants.Operations.namespace))
.dot(description.methodName)
.dot(description.operationTypeName)

let operationArg = FunctionArgumentDescription(label: "forOperation", expression: operationTypeExpr.dot("id"))
let inputArg = FunctionArgumentDescription(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,19 @@
//===----------------------------------------------------------------------===//
import Foundation

/// Extra context for the `safeForSwiftCode_` family of functions to produce more appropriate Swift identifiers.
struct SwiftNameOptions {

/// The capitalization option.
var isCapitalized: Bool

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

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

extension String {

/// Returns a string sanitized to be usable as a Swift identifier.
Expand All @@ -26,7 +39,7 @@ extension String {
///
/// In addition to replacing illegal characters, it also
/// ensures that the identifier starts with a letter and not a number.
var safeForSwiftCode: String {
func safeForSwiftCode_defensive(options: SwiftNameOptions) -> String {
guard !isEmpty else { return "_empty" }

let firstCharSet: CharacterSet = .letters.union(.init(charactersIn: "_"))
Expand Down Expand Up @@ -59,14 +72,185 @@ 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 }
return "_\(validString)"
}

/// Returns a string sanitized to be usable as a Swift identifier, and tries to produce UpperCamelCase
/// or lowerCamelCase string, the casing is controlled using the provided options.
///
/// If the string contains any illegal characters, falls back to the behavior
/// matching `safeForSwiftCode_defensive`.
///
/// 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.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
// don't return `true` to `isUppercase`.
!$0.isLowercase
}

// 1. Leave leading underscores as-are
// 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 ""

var buffer: [Character] = []
buffer.reserveCapacity(count)
enum State: Equatable {
case modifying
case preFirstWord
struct AccumulatingFirstWordContext: Equatable { var isAccumulatingInitialUppercase: Bool }
case accumulatingFirstWord(AccumulatingFirstWordContext)
case accumulatingWord
case waitingForWordStarter
}
var state: State = .preFirstWord
for index in self[...].indices {
let char = self[index]
let _state = state
state = .modifying
switch _state {
case .preFirstWord:
if char == "_" {
// Leading underscores are kept.
buffer.append(char)
state = .preFirstWord
} else if char.isNumber {
// The underscore will be added by the defensive strategy.
buffer.append(char)
state = .accumulatingFirstWord(.init(isAccumulatingInitialUppercase: false))
} else if char.isLetter {
// First character in the identifier.
buffer.append(contentsOf: capitalize ? char.uppercased() : char.lowercased())
state = .accumulatingFirstWord(
.init(isAccumulatingInitialUppercase: !capitalize && char.isUppercase)
)
} else {
// 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 {
if isAllUppercase {
buffer.append(contentsOf: char.lowercased())
} else if context.isAccumulatingInitialUppercase {
// Example: "HTTPProxy"/"HTTP_Proxy"/"HTTP_proxy"" should all
// become "httpProxy" when capitalize == false.
// This means treating the first word differently.
// Here we are on the second or later character of the first word (the first
// character is handled in `.preFirstWord`.
// If the first character was uppercase, and we're in lowercasing mode,
// we need to lowercase every consequtive uppercase character while there's
// another uppercase character after it.
if char.isLowercase {
// No accumulating anymore, just append it and turn off accumulation.
buffer.append(char)
context.isAccumulatingInitialUppercase = false
} else {
let suffix = suffix(from: self.index(after: index))
if suffix.count >= 2 {
let next = suffix.first!
let secondNext = suffix.dropFirst().first!
if next.isUppercase && secondNext.isLowercase {
// Finished lowercasing.
context.isAccumulatingInitialUppercase = false
buffer.append(contentsOf: char.lowercased())
} else if Self.wordSeparators.contains(next) {
// Finished lowercasing.
context.isAccumulatingInitialUppercase = false
buffer.append(contentsOf: char.lowercased())
} else if next.isUppercase {
// Keep lowercasing.
buffer.append(contentsOf: char.lowercased())
} else {
// Append as-is, stop accumulating.
context.isAccumulatingInitialUppercase = false
buffer.append(char)
}
} else {
// This is the last or second to last character,
// since we were accumulating capitals, lowercase it.
buffer.append(contentsOf: char.lowercased())
context.isAccumulatingInitialUppercase = false
}
czechboy0 marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
buffer.append(char)
}
state = .accumulatingFirstWord(context)
} 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
} else if ["."].contains(char) {
// In the middle of an identifier, these get replaced with
// an underscore, but continue the current word.
buffer.append("_")
state = .accumulatingFirstWord(.init(isAccumulatingInitialUppercase: false))
} else if ["{", "}"].contains(char) {
// In the middle of an identifier, curly braces are dropped.
state = .accumulatingFirstWord(.init(isAccumulatingInitialUppercase: false))
} else {
// 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 {
if isAllUppercase { buffer.append(contentsOf: char.lowercased()) } else { buffer.append(char) }
state = .accumulatingWord
} else if Self.wordSeparators.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
} else if ["."].contains(char) {
// In the middle of an identifier, these get replaced with
// an underscore, but continue the current word.
buffer.append("_")
state = .accumulatingWord
} else if ["{", "}"].contains(char) {
// In the middle of an identifier, these are dropped.
state = .accumulatingWord
} else {
// Illegal character, keep and let the defensive strategy deal with it.
state = .accumulatingWord
buffer.append(char)
}
case .waitingForWordStarter:
if ["_", "-", ".", "/", "+", "{", "}"].contains(char) {
// Between words, just drop allowed special characters, since
// we're already between words anyway.
state = .waitingForWordStarter
} else if char.isLetter || char.isNumber {
// Starting a new word in the middle of the identifier.
buffer.append(contentsOf: char.uppercased())
state = .accumulatingWord
} else {
// Illegal character, keep and let the defensive strategy deal with it.
state = .waitingForWordStarter
buffer.append(char)
}
case .modifying: preconditionFailure("Logic error in \(#function), string: '\(self)'")
}
precondition(state != .modifying, "Logic error in \(#function), string: '\(self)'")
}
return String(buffer).safeForSwiftCode_defensive(options: options)
}

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

/// A list of Swift keywords.
///
/// Copied from SwiftSyntax/TokenKind.swift
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ extension TypesFileTranslator {
userDescription: nil,
parent: typeName
)
let caseName = safeSwiftNameForOneOfMappedType(mappedType)
let caseName = safeSwiftNameForOneOfMappedCase(mappedType)
return (caseName, mappedType.rawNames, true, comment, mappedType.typeName.asUsage, [])
}
} else {
Expand Down Expand Up @@ -209,7 +209,7 @@ extension TypesFileTranslator {
let decoder: Declaration
if let discriminator {
let originalName = discriminator.propertyName
let swiftName = context.asSwiftSafeName(originalName)
let swiftName = context.asSwiftSafeName(originalName, .noncapitalized)
codingKeysDecls = [
.enum(
accessModifier: config.access,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,12 @@ extension FileTranslator {
// This is unlikely to be fixed, so handling that case here.
// https://github.com/apple/swift-openapi-generator/issues/118
if isNullable && anyValue is Void {
try addIfUnique(id: .string(""), caseName: context.asSwiftSafeName(""))
try addIfUnique(id: .string(""), caseName: context.asSwiftSafeName("", .noncapitalized))
} else {
guard let rawValue = anyValue as? String else {
throw GenericError(message: "Disallowed value for a string enum '\(typeName)': \(anyValue)")
}
let caseName = context.asSwiftSafeName(rawValue)
let caseName = context.asSwiftSafeName(rawValue, .noncapitalized)
try addIfUnique(id: .string(rawValue), caseName: caseName)
}
case .integer:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ extension FileTranslator {
/// component.
/// - Parameter type: The `OneOfMappedType` for which to determine the case name.
/// - Returns: A string representing the safe Swift name for the specified `OneOfMappedType`.
func safeSwiftNameForOneOfMappedType(_ type: OneOfMappedType) -> String {
context.asSwiftSafeName(type.rawNames[0])
func safeSwiftNameForOneOfMappedCase(_ type: OneOfMappedType) -> String {
context.asSwiftSafeName(type.rawNames[0], .noncapitalized)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ struct PropertyBlueprint {
extension PropertyBlueprint {

/// A name that is verified to be a valid Swift identifier.
var swiftSafeName: String { context.asSwiftSafeName(originalName) }
var swiftSafeName: String { context.asSwiftSafeName(originalName, .noncapitalized) }

/// The JSON path to the property.
///
Expand Down
21 changes: 19 additions & 2 deletions Sources/_OpenAPIGeneratorCore/Translator/FileTranslator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,21 @@ protocol FileTranslator {
extension FileTranslator {

/// A new context from the file translator.
var context: TranslatorContext { TranslatorContext(asSwiftSafeName: { $0.safeForSwiftCode }) }
var context: TranslatorContext {
let asSwiftSafeName: (String, SwiftNameOptions) -> String
switch config.namingStrategy {
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)
},
namingStrategy: config.namingStrategy
)
}
}

/// A set of configuration values for concrete file translators.
Expand All @@ -57,5 +71,8 @@ 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) -> String
var asSwiftSafeName: (String, SwiftNameOptions) -> String

/// The naming strategy.
var namingStrategy: NamingStrategy
}
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ extension FileTranslator {
}
var parts: [MultipartSchemaTypedContent] = try topLevelObject.properties.compactMap {
(key, value) -> MultipartSchemaTypedContent? in
let swiftSafeName = context.asSwiftSafeName(key)
let swiftSafeName = context.asSwiftSafeName(key, .capitalized)
let typeName = typeName.appending(
swiftComponent: swiftSafeName + Constants.Global.inlineTypeSuffix,
jsonComponent: key
Expand Down
Loading
Loading