Skip to content

Commit

Permalink
Nullable enums with an empty string fail to get generated (#176)
Browse files Browse the repository at this point in the history
Nullable enums with an empty string fail to get generated

### Motivation

Fixes #118.

The issue is that when a `type: string, enum: ...` schema is marked as `nullable: true`, _empty cases are parsed as Void_ instead of _nil_. See the issue for the investigation and identifying the root cause, but the TL;DR is that we need a workaround in the generator in the short term, and it's uncertain if the issue will ever be addressed in our dependency Yams (might be unlikely, as it'd probably be a breaking change for them at this point).

### Modifications

This PR adds code that catches this case and converts the `Void` into an empty string, which is what the OpenAPI document author actually put into their document in the first place.

### Result

Now `nullable: true` string enums with an empty case can be generated correctly. One example of such use is the [GitHub API](https://github.com/github/rest-api-description).

Note that this PR isn't really adding support for nullable schemas in general, that's tracked by #82. We're just working around one specific scenario in which a relatively common input trips up the generator.

### Test Plan

Added a unit test for each of: non-nullable and nullable variants.


Reviewed by: gjcairo, simonjbeaumont

Builds:
     ✔︎ pull request validation (5.8) - Build finished. 
     ✔︎ pull request validation (5.9) - Build finished. 
     ✔︎ pull request validation (docc test) - Build finished. 
     ✔︎ pull request validation (integration test) - Build finished. 
     ✔︎ pull request validation (nightly) - Build finished. 
     ✔︎ pull request validation (soundness) - Build finished. 

#176
  • Loading branch information
czechboy0 authored Aug 8, 2023
1 parent 0ae57b9 commit 4cf4891
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ extension FileTranslator {
let enumDecl = try translateStringEnum(
typeName: typeName,
userDescription: overrides.userDescription ?? coreContext.description,
isNullable: coreContext.nullable,
allowedValues: allowedValues
)
return [enumDecl]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,23 @@ extension FileTranslator {
/// - typeName: The name of the type to give to the declared enum.
/// - openAPIDescription: A user-specified description from the OpenAPI
/// document.
/// - isNullable: Whether the enum schema is nullable.
/// - allowedValues: The enumerated allowed values.
func translateStringEnum(
typeName: TypeName,
userDescription: String?,
isNullable: Bool,
allowedValues: [AnyCodable]
) throws -> Declaration {

let rawValues = try allowedValues.map(\.value)
.map { anyValue in
// 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 ""
}
guard let string = anyValue as? String else {
throw GenericError(message: "Disallowed value for a string enum '\(typeName)': \(anyValue)")
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftOpenAPIGenerator open source project
//
// Copyright (c) 2023 Apple Inc. and the SwiftOpenAPIGenerator project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftOpenAPIGenerator project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
import XCTest
import OpenAPIKit30
@testable import _OpenAPIGeneratorCore

final class Test_translateStringEnum: Test_Core {

func testCaseValues() throws {
let names = try _caseValues(
.string(
allowedValues: "a",
""
)
)
XCTAssertEqual(names, ["a", "_empty", "undocumented"])
}

func testCaseValuesForNullableSchema() throws {
let names = try _caseValues(
.string(
nullable: true,
allowedValues: "a",
nil
)
)
XCTAssertEqual(names, ["a", "_empty", "undocumented"])
}

func _caseValues(_ schema: JSONSchema) throws -> [String] {
self.continueAfterFailure = false
let translator = makeTypesTranslator()
let decls = try translator.translateSchema(
typeName: .init(swiftKeyPath: ["FooEnum"]),
schema: schema,
overrides: .none
)
XCTAssertEqual(decls.count, 1)
let decl = decls[0]
guard case .enum(let enumDesc) = decl.strippingTopComment else {
throw UnexpectedDeclError(actual: decl.info.kind, expected: .enum)
}
XCTAssertEqual(enumDesc.name, "FooEnum")
let names: [String] = enumDesc.members.compactMap { memberDecl in
guard case .enumCase(let caseDesc) = memberDecl.strippingTopComment else {
return nil
}
return caseDesc.name
}
return names
}
}

0 comments on commit 4cf4891

Please sign in to comment.