From 3d62c0688e7ff996560a95802cc8e7bd6c8019ad Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Sat, 16 Nov 2024 11:10:17 +0100 Subject: [PATCH 1/2] Skip duplicate enum values --- ...tringEnum.swift => translateRawEnum.swift} | 73 ++++++++++++++----- .../SnippetBasedReferenceTests.swift | 27 +++++++ 2 files changed, 80 insertions(+), 20 deletions(-) rename Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/{translateStringEnum.swift => translateRawEnum.swift} (52%) diff --git a/Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateStringEnum.swift b/Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateRawEnum.swift similarity index 52% rename from Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateStringEnum.swift rename to Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateRawEnum.swift index 9add9482..6418def5 100644 --- a/Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateStringEnum.swift +++ b/Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateRawEnum.swift @@ -23,6 +23,23 @@ enum RawEnumBackingType { case integer } +/// The extracted enum value. +private enum EnumValue: Hashable, CustomStringConvertible { + + /// A string value. + case string(String) + + /// An integer value. + case integer(Int) + + var description: String { + switch self { + case .string(let value): return "\"\(value)\"" + case .integer(let value): return String(value) + } + } +} + extension FileTranslator { /// Returns a declaration of the specified raw value-based enum schema. @@ -42,32 +59,48 @@ extension FileTranslator { isNullable: Bool, allowedValues: [AnyCodable] ) throws -> Declaration { - let cases: [(String, LiteralDescription)] = try allowedValues.map(\.value) - .map { anyValue in - switch backingType { - case .string: - // In nullable enum schemas, empty strings are parsed as Void. - // 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 { return (context.asSwiftSafeName(""), .string("")) } + var seen: Set = [] + var cases: [(String, LiteralDescription)] = [] + func shouldAdd(_ value: EnumValue) throws -> Bool { + guard seen.insert(value).inserted else { + try diagnostics.emit( + .warning( + message: "Duplicate enum value, skipping", + context: ["value": "\(value)", "foundIn": typeName.description] + ) + ) + return false + } + return true + } + for anyValue in allowedValues.map(\.value) { + switch backingType { + case .string: + // In nullable enum schemas, empty strings are parsed as Void. + // 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 { + if try shouldAdd(.string("")) { cases.append((context.asSwiftSafeName(""), .string(""))) } + } else { guard let rawValue = anyValue as? String else { throw GenericError(message: "Disallowed value for a string enum '\(typeName)': \(anyValue)") } let caseName = context.asSwiftSafeName(rawValue) - return (caseName, .string(rawValue)) - case .integer: - let rawValue: Int - if let intRawValue = anyValue as? Int { - rawValue = intRawValue - } else if let stringRawValue = anyValue as? String, let intRawValue = Int(stringRawValue) { - rawValue = intRawValue - } else { - throw GenericError(message: "Disallowed value for an integer enum '\(typeName)': \(anyValue)") - } - let caseName = rawValue < 0 ? "_n\(abs(rawValue))" : "_\(rawValue)" - return (caseName, .int(rawValue)) + if try shouldAdd(.string(rawValue)) { cases.append((caseName, .string(rawValue))) } + } + case .integer: + let rawValue: Int + if let intRawValue = anyValue as? Int { + rawValue = intRawValue + } else if let stringRawValue = anyValue as? String, let intRawValue = Int(stringRawValue) { + rawValue = intRawValue + } else { + throw GenericError(message: "Disallowed value for an integer enum '\(typeName)': \(anyValue)") } + let caseName = rawValue < 0 ? "_n\(abs(rawValue))" : "_\(rawValue)" + if try shouldAdd(.integer(rawValue)) { cases.append((caseName, .int(rawValue))) } } + } let baseConformance: String switch backingType { case .string: baseConformance = Constants.RawEnum.baseConformanceString diff --git a/Tests/OpenAPIGeneratorReferenceTests/SnippetBasedReferenceTests.swift b/Tests/OpenAPIGeneratorReferenceTests/SnippetBasedReferenceTests.swift index ba5e5905..05e9c70e 100644 --- a/Tests/OpenAPIGeneratorReferenceTests/SnippetBasedReferenceTests.swift +++ b/Tests/OpenAPIGeneratorReferenceTests/SnippetBasedReferenceTests.swift @@ -1310,6 +1310,33 @@ final class SnippetBasedReferenceTests: XCTestCase { ) } + func testComponentsSchemasStringEnumWithDuplicates() throws { + try self.assertSchemasTranslation( + ignoredDiagnosticMessages: ["Duplicate enum value, skipping"], + """ + schemas: + MyEnum: + type: string + enum: + - one + - two + - three + - two + - four + """, + """ + public enum Schemas { + @frozen public enum MyEnum: String, Codable, Hashable, Sendable, CaseIterable { + case one = "one" + case two = "two" + case three = "three" + case four = "four" + } + } + """ + ) + } + func testComponentsSchemasIntEnum() throws { try self.assertSchemasTranslation( """ From d926a0cbf4096a398a87f4ed45632aa475085bd3 Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Mon, 18 Nov 2024 16:51:57 +0100 Subject: [PATCH 2/2] PR feedback --- Package.swift | 5 +- .../CommonTranslations/translateRawEnum.swift | 47 ++++++++++++++----- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/Package.swift b/Package.swift index 1043f555..36b1d24c 100644 --- a/Package.swift +++ b/Package.swift @@ -49,6 +49,7 @@ let package = Package( // General algorithms .package(url: "https://github.com/apple/swift-algorithms", from: "1.2.0"), + .package(url: "https://github.com/apple/swift-collections", from: "1.1.4"), // Read OpenAPI documents .package(url: "https://github.com/mattpolzin/OpenAPIKit", from: "3.3.0"), @@ -72,7 +73,9 @@ let package = Package( .product(name: "OpenAPIKit", package: "OpenAPIKit"), .product(name: "OpenAPIKit30", package: "OpenAPIKit"), .product(name: "OpenAPIKitCompat", package: "OpenAPIKit"), - .product(name: "Algorithms", package: "swift-algorithms"), .product(name: "Yams", package: "Yams"), + .product(name: "Algorithms", package: "swift-algorithms"), + .product(name: "OrderedCollections", package: "swift-collections"), + .product(name: "Yams", package: "Yams"), ], swiftSettings: swiftSettings ), diff --git a/Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateRawEnum.swift b/Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateRawEnum.swift index 6418def5..ed513551 100644 --- a/Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateRawEnum.swift +++ b/Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateRawEnum.swift @@ -12,6 +12,7 @@ // //===----------------------------------------------------------------------===// import OpenAPIKit +import OrderedCollections /// The backing type of a raw enum. enum RawEnumBackingType { @@ -23,8 +24,8 @@ enum RawEnumBackingType { case integer } -/// The extracted enum value. -private enum EnumValue: Hashable, CustomStringConvertible { +/// The extracted enum value's identifier. +private enum EnumCaseID: Hashable, CustomStringConvertible { /// A string value. case string(String) @@ -40,6 +41,23 @@ private enum EnumValue: Hashable, CustomStringConvertible { } } +/// A wrapper for the metadata about the raw enum case. +private struct EnumCase { + + /// Used for checking uniqueness. + var id: EnumCaseID + + /// The raw Swift-safe name for the case. + var caseName: String + + /// The literal value of the enum case. + var literal: LiteralDescription +} + +extension EnumCase: Equatable { static func == (lhs: EnumCase, rhs: EnumCase) -> Bool { lhs.id == rhs.id } } + +extension EnumCase: Hashable { func hash(into hasher: inout Hasher) { hasher.combine(id) } } + extension FileTranslator { /// Returns a declaration of the specified raw value-based enum schema. @@ -59,19 +77,22 @@ extension FileTranslator { isNullable: Bool, allowedValues: [AnyCodable] ) throws -> Declaration { - var seen: Set = [] - var cases: [(String, LiteralDescription)] = [] - func shouldAdd(_ value: EnumValue) throws -> Bool { - guard seen.insert(value).inserted else { + var cases: OrderedSet = [] + func addIfUnique(id: EnumCaseID, caseName: String) throws { + let literal: LiteralDescription + switch id { + case .string(let string): literal = .string(string) + case .integer(let int): literal = .int(int) + } + guard cases.append(.init(id: id, caseName: caseName, literal: literal)).inserted else { try diagnostics.emit( .warning( message: "Duplicate enum value, skipping", - context: ["value": "\(value)", "foundIn": typeName.description] + context: ["id": "\(id)", "foundIn": typeName.description] ) ) - return false + return } - return true } for anyValue in allowedValues.map(\.value) { switch backingType { @@ -80,13 +101,13 @@ 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 { - if try shouldAdd(.string("")) { cases.append((context.asSwiftSafeName(""), .string(""))) } + try addIfUnique(id: .string(""), caseName: context.asSwiftSafeName("")) } else { guard let rawValue = anyValue as? String else { throw GenericError(message: "Disallowed value for a string enum '\(typeName)': \(anyValue)") } let caseName = context.asSwiftSafeName(rawValue) - if try shouldAdd(.string(rawValue)) { cases.append((caseName, .string(rawValue))) } + try addIfUnique(id: .string(rawValue), caseName: caseName) } case .integer: let rawValue: Int @@ -98,7 +119,7 @@ extension FileTranslator { throw GenericError(message: "Disallowed value for an integer enum '\(typeName)': \(anyValue)") } let caseName = rawValue < 0 ? "_n\(abs(rawValue))" : "_\(rawValue)" - if try shouldAdd(.integer(rawValue)) { cases.append((caseName, .int(rawValue))) } + try addIfUnique(id: .integer(rawValue), caseName: caseName) } } let baseConformance: String @@ -111,7 +132,7 @@ extension FileTranslator { typeName: typeName, conformances: conformances, userDescription: userDescription, - cases: cases, + cases: cases.map { ($0.caseName, $0.literal) }, unknownCaseName: nil, unknownCaseDescription: nil )