Skip to content

Commit

Permalink
Idiomatic naming strategy as opt-in (#679)
Browse files Browse the repository at this point in the history
### Motivation

Implementation of #683.

### Modifications

- Implemented the SOAR-0013 idiomatic naming strategy
- Added name overrides
- Switched most reference tests to use the new strategy
- Updated some docs (I'll update the rest after this lands and gets
released, then I can update Examples and IntegrationTest)

### Result

SOAR-0013 implemented.

### Test Plan

Updated and added new unit tests.

---------

Co-authored-by: Si Beaumont <simonjbeaumont@gmail.com>
Co-authored-by: Si Beaumont <beaumont@apple.com>
  • Loading branch information
3 people authored Dec 18, 2024
1 parent 4d0c02d commit ad7f0e7
Show file tree
Hide file tree
Showing 61 changed files with 1,263 additions and 727 deletions.
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 {

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

0 comments on commit ad7f0e7

Please sign in to comment.