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 all 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

This file was deleted.

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.safeNameGenerator.swiftMemberName(for: originalName)
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.safeNameGenerator.swiftMemberName(for: ""))
} 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.safeNameGenerator.swiftMemberName(for: rawValue)
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.safeNameGenerator.swiftMemberName(for: type.rawNames[0])
}
}

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.safeNameGenerator.swiftMemberName(for: originalName) }

/// The JSON path to the property.
///
Expand Down
44 changes: 38 additions & 6 deletions Sources/_OpenAPIGeneratorCore/Translator/FileTranslator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,50 @@ protocol FileTranslator {
func translateFile(parsedOpenAPI: ParsedOpenAPIRepresentation) throws -> StructuredSwiftRepresentation
}

/// A generator that allows overriding the documented name.
struct OverridableSafeNameGenerator: SafeNameGenerator {
czechboy0 marked this conversation as resolved.
Show resolved Hide resolved

/// The upstream name generator for names that aren't overriden.
var upstream: any SafeNameGenerator

/// A set of overrides, where the key is the documented name and the value the desired identifier.
var overrides: [String: String]

func swiftTypeName(for documentedName: String) -> String {
if let override = overrides[documentedName] { return override }
return upstream.swiftTypeName(for: documentedName)
}

func swiftMemberName(for documentedName: String) -> String {
if let override = overrides[documentedName] { return override }
return upstream.swiftMemberName(for: documentedName)
}

func swiftContentTypeName(for contentType: ContentType) -> String {
upstream.swiftContentTypeName(for: contentType)
}
}

extension FileTranslator {

/// A new context from the file translator.
var context: TranslatorContext { TranslatorContext(asSwiftSafeName: { $0.safeForSwiftCode }) }
var context: TranslatorContext {
let safeNameGenerator: any SafeNameGenerator
switch config.namingStrategy {
case .defensive: safeNameGenerator = .defensive
case .idiomatic: safeNameGenerator = .idiomatic
}
let overridingGenerator = OverridableSafeNameGenerator(
upstream: safeNameGenerator,
overrides: config.nameOverrides
)
return TranslatorContext(safeNameGenerator: overridingGenerator)
}
}

/// A set of configuration values for concrete file translators.
struct TranslatorContext {

/// A closure that returns a copy of the string modified to be a valid Swift identifier.
///
/// - Parameter string: The string to convert to be safe for Swift.
/// - Returns: A Swift-safe version of the input string.
var asSwiftSafeName: (String) -> String
/// A type that generates safe names for use as Swift identifiers.
var safeNameGenerator: any SafeNameGenerator
}
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.safeNameGenerator.swiftTypeName(for: key)
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.safeNameGenerator.swiftMemberName(for: documentedPart.originalName),
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.safeNameGenerator.swiftMemberName(for: originalName)
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.safeNameGenerator.swiftMemberName(for: originalName)
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.safeNameGenerator.swiftMemberName(for: operationID) }

/// 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.safeNameGenerator.swiftTypeName(for: operationID) }

/// 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.safeNameGenerator.swiftMemberName(for: param))
}
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.safeNameGenerator.swiftMemberName(for: name) }

/// 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 @@ -52,7 +52,7 @@ extension TypesFileTranslator {
bodyMembers.append(contentsOf: inlineTypeDecls)
}
let contentType = content.content.contentType
let identifier = typeAssigner.contentSwiftName(contentType)
let identifier = context.safeNameGenerator.swiftContentTypeName(for: contentType)
let associatedType = content.resolvedTypeUsage.withOptional(false)
let contentCase: Declaration = .commentable(
contentType.docComment(typeName: contentTypeName),
Expand Down Expand Up @@ -148,7 +148,7 @@ extension ClientFileTranslator {
var cases: [SwitchCaseDescription] = try contents.map { typedContent in
let content = typedContent.content
let contentType = content.contentType
let contentTypeIdentifier = typeAssigner.contentSwiftName(contentType)
let contentTypeIdentifier = context.safeNameGenerator.swiftContentTypeName(for: contentType)
let contentTypeHeaderValue = contentType.headerValueForSending

let extraBodyAssignArgs: [FunctionArgumentDescription]
Expand Down Expand Up @@ -251,7 +251,7 @@ extension ServerFileTranslator {
argumentNames: ["value"],
body: [
.expression(
.dot(typeAssigner.contentSwiftName(typedContent.content.contentType))
.dot(context.safeNameGenerator.swiftContentTypeName(for: typedContent.content.contentType))
.call([.init(label: nil, expression: .identifierPattern("value"))])
)
]
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.safeNameGenerator.swiftMemberName(for: name) }

/// 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 @@ -141,7 +141,7 @@ extension TypesFileTranslator {
) throws -> [Declaration] {
var bodyCases: [Declaration] = []
let contentType = typedContent.content.contentType
let identifier = typeAssigner.contentSwiftName(contentType)
let identifier = context.safeNameGenerator.swiftContentTypeName(for: contentType)
let associatedType = typedContent.resolvedTypeUsage
let content = typedContent.content
let schema = content.schema
Expand Down
Loading
Loading