Propagate nullability through references (#300)

Propagate nullability through references

### Motivation

Fixes #288.

### Modifications

Refactors how we decide whether to mark a type as optional, and adds the ability to "peek" through references to decide the nullability of the reference schema.

Also added an article describing the overall logic as a starting point, will be expanded with a few more changes coming in this area in future PRs.

### Result

We now correctly mark an object property as optional if it points at an optional schema through a reference. Previously, it was always marked as required, which caused decoding to fail e.g. in the GitHub OpenAPI doc.

### Test Plan

Added and adjusted unit tests.


Reviewed by: simonjbeaumont

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

#300

Author: czechboy0

Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateAllAnyOneOf.swift

@@ -49,6 +49,7 @@ extension FileTranslator {
                 let rawPropertyType = try typeAssigner.typeUsage(
                     forAllOrAnyOrOneOfChildSchemaNamed: key,
                     withSchema: schema,
+                    components: components,
                     inParent: typeName
                 )
                 let propertyType: TypeUsage
@@ -163,6 +164,7 @@ extension FileTranslator {
                     let childType = try typeAssigner.typeUsage(
                         forAllOrAnyOrOneOfChildSchemaNamed: key,
                         withSchema: schema,
+                        components: components,
                         inParent: typeName
                     )
                     let caseName: String

Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateArray.swift

@@ -37,11 +37,15 @@ extension FileTranslator {
         // An OpenAPI array is represented as a Swift array with an element type
         let elementType: TypeUsage
         if let items = arrayContext.items {
-            if let builtinType = try typeMatcher.tryMatchReferenceableType(for: items) {
+            if let builtinType = try typeMatcher.tryMatchReferenceableType(
+                for: items,
+                components: components
+            ) {
                 elementType = builtinType
             } else {
                 elementType = try typeAssigner.typeUsage(
                     forArrayElementWithSchema: items,
+                    components: components,
                     inParent: typeName
                 )
                 let nestedDecls = try translateSchema(

Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateObjectStruct.swift

@@ -43,6 +43,7 @@ extension FileTranslator {
                 let propertyType = try typeAssigner.typeUsage(
                     forObjectPropertyNamed: key,
                     withSchema: value,
+                    components: components,
                     inParent: typeName
                 )
                 let comment: Comment? = .property(
@@ -127,6 +128,7 @@ extension FileTranslator {
             let valueTypeUsage = try typeAssigner.typeUsage(
                 forObjectPropertyNamed: "additionalProperties",
                 withSchema: schema,
+                components: components,
                 inParent: parent
             )
             if TypeMatcher.isInlinable(schema) {

Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateSchema.swift

@@ -89,11 +89,16 @@ extension FileTranslator {
         }
 
         // If this type maps to a referenceable schema, define a typealias
-        if let builtinType = try typeMatcher.tryMatchReferenceableType(for: schema) {
+        if let builtinType = try typeMatcher.tryMatchReferenceableType(
+            for: schema,
+            components: components
+        ) {
             let typealiasDecl = try translateTypealias(
                 named: typeName,
                 userDescription: overrides.userDescription ?? schema.description,
-                to: builtinType.withOptional(overrides.isOptional ?? !schema.required)
+                to: builtinType.withOptional(
+                    overrides.isOptional ?? typeMatcher.isOptional(schema, components: components)
+                )
             )
             return [typealiasDecl]
         }

Sources/_OpenAPIGeneratorCore/Translator/Content/ContentInspector.swift

@@ -56,6 +56,7 @@ extension FileTranslator {
         let associatedType = try typeAssigner.typeUsage(
             usingNamingHint: identifier,
             withSchema: content.schema,
+            components: components,
             inParent: parent
         )
         return .init(content: content, typeUsage: associatedType)
@@ -91,6 +92,7 @@ extension FileTranslator {
             let associatedType = try typeAssigner.typeUsage(
                 usingNamingHint: identifier,
                 withSchema: content.schema,
+                components: components,
                 inParent: parent
             )
             return .init(content: content, typeUsage: associatedType)

Sources/_OpenAPIGeneratorCore/Translator/Parameters/TypedParameter.swift

@@ -238,6 +238,7 @@ extension FileTranslator {
                 type = try typeAssigner.typeUsage(
                     forParameterNamed: _parameter.name,
                     withSchema: schema,
+                    components: components,
                     inParent: locationTypeName
                 )
             }

Sources/_OpenAPIGeneratorCore/Translator/Responses/TypedResponseHeader.swift

@@ -154,6 +154,7 @@ extension FileTranslator {
                 type = try typeAssigner.typeUsage(
                     forParameterNamed: name,
                     withSchema: schema,
+                    components: components,
                     inParent: parent
                 )
             }

Sources/_OpenAPIGeneratorCore/Translator/TypeAssignment/TypeAssigner.swift

@@ -83,11 +83,13 @@ struct TypeAssigner {
     /// - Parameters:
     ///   - hint: A hint string used when computing a name for an inline type.
     ///   - schema: The OpenAPI schema.
+    ///   - components: The components in which to look up references.
     ///   - parent: The parent type in which to name the type.
     /// - Returns: A type usage; or nil if the schema is nil or unsupported.
     func typeUsage(
         usingNamingHint hint: String,
         withSchema schema: UnresolvedSchema?,
+        components: OpenAPI.Components,
         inParent parent: TypeName
     ) throws -> TypeUsage? {
         let associatedType: TypeUsage?
@@ -99,6 +101,7 @@ struct TypeAssigner {
                 associatedType = try _typeUsage(
                     forPotentiallyInlinedValueNamed: hint,
                     withSchema: schema,
+                    components: components,
                     inParent: parent,
                     subtype: .appendScope
                 )
@@ -113,16 +116,19 @@ struct TypeAssigner {
     /// - Parameters:
     ///   - originalName: The name of the property in the OpenAPI document.
     ///   - schema: The OpenAPI schema provided for the property.
+    ///   - components: The components in which to look up references.
     ///   - parent: The parent type in which to name the type.
     /// - Returns: A type usage.
     func typeUsage(
         forObjectPropertyNamed originalName: String,
         withSchema schema: JSONSchema,
+        components: OpenAPI.Components,
         inParent parent: TypeName
     ) throws -> TypeUsage {
         try _typeUsage(
             forPotentiallyInlinedValueNamed: originalName,
             withSchema: schema,
+            components: components,
             inParent: parent,
             subtype: .appendScope
         )
@@ -132,17 +138,20 @@ struct TypeAssigner {
     /// - Parameters:
     ///   - originalName: A hint for naming.
     ///   - schema: The OpenAPI schema provided for the property.
+    ///   - components: The components in which to look up references.
     ///   - parent: The parent type in which to name the type.
     /// - Returns: A type usage.
     func typeUsage(
         forAllOrAnyOrOneOfChildSchemaNamed originalName: String,
         withSchema schema: JSONSchema,
+        components: OpenAPI.Components,
         inParent parent: TypeName
     ) throws -> TypeUsage {
         try _typeUsage(
             forPotentiallyInlinedValueNamed: originalName.uppercasingFirstLetter,
             jsonReferenceComponentOverride: originalName,
             withSchema: schema,
+            components: components,
             inParent: parent,
             subtype: .appendScope
         )
@@ -151,15 +160,18 @@ struct TypeAssigner {
     /// Returns a type usage for an element schema of an array.
     /// - Parameters:
     ///   - schema: The OpenAPI schema provided for the array element type.
+    ///   - components: The components in which to look up references.
     ///   - parent: The parent type in which to name the type.
     /// - Returns: A type usage.
     func typeUsage(
         forArrayElementWithSchema schema: JSONSchema,
+        components: OpenAPI.Components,
         inParent parent: TypeName
     ) throws -> TypeUsage {
         try _typeUsage(
             forPotentiallyInlinedValueNamed: parent.shortSwiftName,
             withSchema: schema,
+            components: components,
             inParent: parent,
             subtype: .appendToLastPathComponent
         )
@@ -169,16 +181,19 @@ struct TypeAssigner {
     /// - Parameters:
     ///   - originalName: The name of the parameter in the OpenAPI document.
     ///   - schema: The OpenAPI schema provided for the parameter.
+    ///   - components: The components in which to look up references.
     ///   - parent: The parent type in which to name the type.
     /// - Returns: A type usage.
     func typeUsage(
         forParameterNamed originalName: String,
         withSchema schema: JSONSchema,
+        components: OpenAPI.Components,
         inParent parent: TypeName
     ) throws -> TypeUsage {
         try _typeUsage(
             forPotentiallyInlinedValueNamed: originalName,
             withSchema: schema,
+            components: components,
             inParent: parent,
             subtype: .appendScope
         )
@@ -250,17 +265,19 @@ struct TypeAssigner {
         jsonReferenceComponentOverride: String? = nil,
         suffix: String = Constants.Global.inlineTypeSuffix,
         withSchema schema: JSONSchema,
+        components: OpenAPI.Components,
         inParent parent: TypeName,
         subtype: SubtypeNamingMethod
     ) throws -> TypeUsage {
+        let typeMatcher = TypeMatcher(
+            asSwiftSafeName: asSwiftSafeName
+        )
         // Check if this type can be simply referenced without
         // creating a new inline type.
-        if let referenceableType =
-            try TypeMatcher(
-                asSwiftSafeName: asSwiftSafeName
-            )
-            .tryMatchReferenceableType(for: schema)
-        {
+        if let referenceableType = try typeMatcher.tryMatchReferenceableType(
+            for: schema,
+            components: components
+        ) {
             return referenceableType
         }
         // Otherwise it's an inline, non-referenceable type
@@ -277,7 +294,7 @@ struct TypeAssigner {
                 jsonComponent: jsonReferenceComponentOverride ?? originalName
             )
             .asUsage
-            .withOptional(!schema.required)
+            .withOptional(try typeMatcher.isOptional(schema, components: components))
     }
 
     /// Returns a type name for a reusable component.

Sources/_OpenAPIGeneratorCore/Translator/TypeAssignment/TypeMatcher.swift

@@ -63,11 +63,14 @@ struct TypeMatcher {
     /// - A reference
     ///
     /// - Note: Optionality from the `JSONSchema` is applied.
-    /// - Parameter schema: The schema to match a referenceable type for.
+    /// - Parameters:
+    ///   - schema: The schema to match a referenceable type for.
+    ///   - components: The components in which to look up references.
     /// - Returns: A type usage for the schema if the schema is supported.
     /// Otherwise, returns nil.
     func tryMatchReferenceableType(
-        for schema: JSONSchema
+        for schema: JSONSchema,
+        components: OpenAPI.Components
     ) throws -> TypeUsage? {
         try Self._tryMatchRecursive(
             for: schema.value,
@@ -90,7 +93,7 @@ struct TypeMatcher {
                 TypeName.arrayContainer.asUsage
             }
         )?
-        .withOptional(!schema.required || schema.nullable)
+        .withOptional(isOptional(schema, components: components))
     }
 
     /// Returns a Boolean value that indicates whether the schema
@@ -240,6 +243,47 @@ struct TypeMatcher {
         return try isKeyValuePair(schemaToCheck, components: components)
     }
 
+    /// Returns a Boolean value indicating whether the schema is optional.
+    /// - Parameters:
+    ///   - schema: The schema to check.
+    ///   - components: The OpenAPI components for looking up references.
+    /// - Returns: `true` if the schema is optional, `false` otherwise.
+    func isOptional(
+        _ schema: JSONSchema,
+        components: OpenAPI.Components
+    ) throws -> Bool {
+        if schema.nullable || !schema.required {
+            return true
+        }
+        guard case .reference(let ref, _) = schema.value else {
+            return false
+        }
+        let targetSchema = try components.lookup(ref)
+        return try isOptional(targetSchema, components: components)
+    }
+
+    /// Returns a Boolean value indicating whether the schema is optional.
+    /// - Parameters:
+    ///   - schema: The schema to check.
+    ///   - components: The OpenAPI components for looking up references.
+    /// - Returns: `true` if the schema is optional, `false` otherwise.
+    func isOptional(
+        _ schema: UnresolvedSchema?,
+        components: OpenAPI.Components
+    ) throws -> Bool {
+        guard let schema else {
+            // A nil unresolved schema represents a non-optional fragment.
+            return false
+        }
+        switch schema {
+        case .a(let ref):
+            let targetSchema = try components.lookup(ref)
+            return try isOptional(targetSchema, components: components)
+        case .b(let schema):
+            return try isOptional(schema, components: components)
+        }
+    }
+
     // MARK: - Private
 
     /// Returns the type name of a built-in type that matches the specified

Sources/swift-openapi-generator/Documentation.docc/Development/Documentation-for-maintainers.md

@@ -12,3 +12,4 @@ Use the resources below if you'd like to learn more about how the generator work
 
 - <doc:Converting-between-data-and-Swift-types>
 - <doc:Generating-custom-Codable-conformance-methods>
+- <doc:Handling-nullable-schemas>