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

public enum NamingStrategy: String, Sendable, Codable, Equatable {
case defensive
case optimistic
}

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

public var namingStrategy: NamingStrategy?

public var nameOverrides: [String: String]?

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

Expand All @@ -50,12 +59,16 @@ public struct Config: Sendable {
access: AccessModifier,
additionalImports: [String] = [],
filter: DocumentFilter? = nil,
namingStrategy: NamingStrategy? = nil,
nameOverrides: [String: String]? = nil,
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,16 @@
//===----------------------------------------------------------------------===//
import Foundation

struct SwiftNameOptions: OptionSet {
let rawValue: Int32

static let none = SwiftNameOptions([])

static let capitalize = SwiftNameOptions(rawValue: 1 << 0)

static let all: SwiftNameOptions = [.capitalize]
}

extension String {

/// Returns a string sanitized to be usable as a Swift identifier.
Expand All @@ -26,7 +36,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 @@ -67,6 +77,110 @@ extension String {
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`.
func safeForSwiftCode_optimistic(options: SwiftNameOptions) -> String {
let capitalize = options.contains(.capitalize)
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 "_"

var buffer: [Character] = []
buffer.reserveCapacity(count)

enum State {
case modifying
case preFirstWord
case accumulatingWord
case waitingForWordStarter
}
var state: State = .preFirstWord
for char in self {
let _state = state
state = .modifying
switch _state {
case .preFirstWord:
if char == "_" {
// Leading underscores are kept.
buffer.append(char)
state = .preFirstWord
} else if char.isNumber {
// Prefix with an underscore if the first character is a number.
buffer.append("_")
buffer.append(char)
state = .accumulatingWord
} else if char.isLetter {
// First character in the identifier.
buffer.append(contentsOf: capitalize ? char.uppercased() : char.lowercased())
state = .accumulatingWord
} else {
// Illegal character, fall back to the defensive strategy.
return safeForSwiftCode_defensive(options: options)
}
case .accumulatingWord:
if char.isLetter || char.isNumber {
if isAllUppercase {
buffer.append(contentsOf: char.lowercased())
} else {
buffer.append(char)
}
state = .accumulatingWord
} else if char == "_" || char == "-" || char == " " {
// In the middle of an identifier, dashes, underscores, and spaces are considered
// word separators, so we remove the character and end the current word.
state = .waitingForWordStarter
} else if char == "." {
// In the middle of an identifier, a period gets replaced with an underscore, but continues
// the current word.
buffer.append("_")
state = .accumulatingWord
} else {
// Illegal character, fall back to the defensive strategy.
return safeForSwiftCode_defensive(options: options)
}
case .waitingForWordStarter:
if char == "_" || char == "-" {
// Between words, just drop dashes, underscores, and spaces, 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, fall back to the defensive strategy.
return safeForSwiftCode_defensive(options: options)
}
case .modifying:
preconditionFailure("Logic error in \(#function), string: '\(self)'")
}
precondition(state != .modifying, "Logic error in \(#function), string: '\(self)'")
}
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
}

/// A list of Swift keywords.
///
/// Copied from SwiftSyntax/TokenKind.swift
Expand Down
Original file line number Diff line number Diff line change
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, .none)
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("", .none))
} 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, .none)
try addIfUnique(id: .string(rawValue), caseName: caseName)
}
case .integer:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ extension FileTranslator {
/// - 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])
context.asSwiftSafeName(type.rawNames[0], .capitalize)
}
}

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, .none) }

/// 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,24 @@ 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, .none:
asSwiftSafeName = { $0.safeForSwiftCode_defensive(options: $1) }
case .optimistic:
asSwiftSafeName = { $0.safeForSwiftCode_optimistic(options: $1) }
}
let overrides = config.nameOverrides ?? [:]
czechboy0 marked this conversation as resolved.
Show resolved Hide resolved
return TranslatorContext(
asSwiftSafeName: { name, options in
if let override = overrides[name] {
return override
}
return asSwiftSafeName(name, options)
}
)
}
}

/// A set of configuration values for concrete file translators.
Expand All @@ -57,5 +74,5 @@ 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
}
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, .capitalize)
let typeName = typeName.appending(
swiftComponent: swiftSafeName + Constants.Global.inlineTypeSuffix,
jsonComponent: key
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ extension TypesFileTranslator {
switch part {
case .documentedTyped(let documentedPart):
let caseDecl: Declaration = .enumCase(
name: context.asSwiftSafeName(documentedPart.originalName),
name: context.asSwiftSafeName(documentedPart.originalName, .none),
kind: .nameWithAssociatedValues([.init(type: .init(part.wrapperTypeUsage))])
)
let decl = try translateMultipartPartContent(
Expand Down Expand Up @@ -404,7 +404,7 @@ extension FileTranslator {
switch part {
case .documentedTyped(let part):
let originalName = part.originalName
let identifier = context.asSwiftSafeName(originalName)
let identifier = context.asSwiftSafeName(originalName, .none)
let contentType = part.partInfo.contentType
let partTypeName = part.typeName
let schema = part.schema
Expand Down Expand Up @@ -613,7 +613,7 @@ extension FileTranslator {
switch part {
case .documentedTyped(let part):
let originalName = part.originalName
let identifier = context.asSwiftSafeName(originalName)
let identifier = context.asSwiftSafeName(originalName, .none)
let contentType = part.partInfo.contentType
let headersTypeName = part.typeName.appending(
swiftComponent: Constants.Operation.Output.Payload.Headers.typeName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,14 @@ extension OperationDescription {
/// Uses the `operationID` value in the OpenAPI operation, if one was
/// specified. Otherwise, computes a unique name from the operation's
/// path and HTTP method.
var methodName: String { context.asSwiftSafeName(operationID) }
var methodName: String { context.asSwiftSafeName(operationID, .none) }

/// Returns a Swift-safe type name for the operation.
///
/// Uses the `operationID` value in the OpenAPI operation, if one was
/// specified. Otherwise, computes a unique name from the operation's
/// path and HTTP method.
var operationTypeName: String { context.asSwiftSafeName(operationID, .capitalize) }

/// Returns the identifier for the operation.
///
Expand All @@ -103,7 +110,7 @@ extension OperationDescription {
.init(
components: [.root, .init(swift: Constants.Operations.namespace, json: "paths")]
+ path.components.map { .init(swift: nil, json: $0) } + [
.init(swift: methodName, json: httpMethod.rawValue)
.init(swift: operationTypeName, json: httpMethod.rawValue)
]
)
}
Expand Down Expand Up @@ -292,7 +299,7 @@ extension OperationDescription {
}
let newPath = OpenAPI.Path(newComponents, trailingSlash: path.trailingSlash)
let names: [Expression] = orderedPathParameters.map { param in
.identifierPattern("input").dot("path").dot(context.asSwiftSafeName(param))
.identifierPattern("input").dot("path").dot(context.asSwiftSafeName(param, .none))
}
let arrayExpr: Expression = .literal(.array(names))
return (newPath.rawValue, arrayExpr)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ extension TypedParameter {
var name: String { parameter.name }

/// The name of the parameter sanitized to be a valid Swift identifier.
var variableName: String { context.asSwiftSafeName(name) }
var variableName: String { context.asSwiftSafeName(name, .none) }

/// A Boolean value that indicates whether the parameter must be specified
/// when performing the OpenAPI operation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ struct TypedResponseHeader {
extension TypedResponseHeader {

/// The name of the header sanitized to be a valid Swift identifier.
var variableName: String { context.asSwiftSafeName(name) }
var variableName: String { context.asSwiftSafeName(name, .none) }

/// A Boolean value that indicates whether the response header can
/// be omitted in the HTTP response.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,8 @@ extension ServerFileTranslator {
func translateServerMethod(_ description: OperationDescription, serverUrlVariableName: String) throws -> (
registerCall: Expression, functionDecl: 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 requestArg = FunctionArgumentDescription(label: "request", expression: .identifierPattern("request"))
Expand Down
Loading
Loading