Stop treating schema warnings as errors (#178)

Stop treating schema warnings as errors

### Motivation

In #130, we introduced extra validation by calling into OpenAPIKit's `validate` method. (It's hidden behind a feature flag, but I've been testing with it enabled.)

It looks for structural issues, such as non-unique operation ids, which would result in us generating non-compiling code anyway, so the validation helps catch these early and emit descriptive errors that adopters can use to fix their doc.

However, the method also takes a parameter `strict`, which defaults to `true`, and when enabled, it turns warnings emitted during schema parsing into errors. This part is _too_ strict for us and was rejecting OpenAPI documents that were valid enough for our needs.

An example of a schema warning is a schema having `minItems: 1` on a non-array schema. While it's not technically correct, it also doesn't impede our understanding of the non-array schema, as we never actually check what the value of `minItems` is. That's why these are just warnings, not errors, so we should stop promoting them to fatal errors that block an adopter from generating code.

### Modifications

This PR flips the `strict` parameter to `false`. This doesn't make us miss out on these warnings, as recently (in #174), we started forwarding these schema warnings into generator diagnostics, so the adopter will see them still, and can address them on their own schedule.

### Result

Now documents with only schema warnings aren't rejected by the generator anymore.

### Test Plan

Added a unit test of the broken-out `validateDoc` function, ensures that a schema with warnings doesn't trip it up anymore.


Reviewed by: 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. 

#178

Author: czechboy0

Sources/_OpenAPIGeneratorCore/GeneratorPipeline.swift

@@ -106,6 +106,7 @@ public func runGenerator(
 /// ``GeneratorPipeline/run(_:)``.
 func makeGeneratorPipeline(
     parser: any ParserProtocol = YamsParser(),
+    validator: @escaping (ParsedOpenAPIRepresentation, Config) throws -> [Diagnostic] = validateDoc,
     translator: any TranslatorProtocol = MultiplexTranslator(),
     renderer: any RendererProtocol = TextBasedRenderer(),
     formatter: @escaping (InMemoryOutputFile) throws -> InMemoryOutputFile = { try $0.swiftFormatted },
@@ -124,36 +125,10 @@ func makeGeneratorPipeline(
             },
             postTransitionHooks: [
                 { doc in
-
-                    if config.featureFlags.contains(.strictOpenAPIValidation) {
-                        // Run OpenAPIKit's built-in validation.
-                        try doc.validate()
-
-                        // Validate that the document is dereferenceable, which
-                        // catches reference cycles, which we don't yet support.
-                        _ = try doc.locallyDereferenced()
-
-                        // Also explicitly dereference the parts of components
-                        // that the generator uses. `locallyDereferenced()` above
-                        // only dereferences paths/operations, but not components.
-                        let components = doc.components
-                        try components.schemas.forEach { schema in
-                            _ = try schema.value.dereferenced(in: components)
-                        }
-                        try components.parameters.forEach { schema in
-                            _ = try schema.value.dereferenced(in: components)
-                        }
-                        try components.headers.forEach { schema in
-                            _ = try schema.value.dereferenced(in: components)
-                        }
-                        try components.requestBodies.forEach { schema in
-                            _ = try schema.value.dereferenced(in: components)
-                        }
-                        try components.responses.forEach { schema in
-                            _ = try schema.value.dereferenced(in: components)
-                        }
+                    let validationDiagnostics = try validator(doc, config)
+                    for diagnostic in validationDiagnostics {
+                        diagnostics.emit(diagnostic)
                     }
-
                     return doc
                 }
             ]

Sources/_OpenAPIGeneratorCore/Parser/validateDoc.swift

@@ -0,0 +1,69 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+/// Runs validation steps on the incoming OpenAPI document.
+/// - Parameters:
+///   - doc: The OpenAPI document to validate.
+///   - config: The generator config.
+/// - Throws: An error if a fatal issue is found.
+func validateDoc(_ doc: ParsedOpenAPIRepresentation, config: Config) throws -> [Diagnostic] {
+    guard config.featureFlags.contains(.strictOpenAPIValidation) else {
+        return []
+    }
+    // Run OpenAPIKit's built-in validation.
+    // Pass `false` to `strict`, however, because we don't
+    // want to turn schema loading warnings into errors.
+    // We already propagate the warnings to the generator's
+    // diagnostics, so they get surfaced to the user.
+    // But the warnings are often too strict and should not
+    // block the generator from running.
+    // Validation errors continue to be fatal, such as
+    // structural issues, like non-unique operationIds, etc.
+    let warnings = try doc.validate(strict: false)
+    let diagnostics: [Diagnostic] = warnings.map { warning in
+        .warning(
+            message: "Validation warning: \(warning.description)",
+            context: [
+                "codingPath": warning.codingPathString ?? "<none>",
+                "contextString": warning.contextString ?? "<none>",
+                "subjectName": warning.subjectName ?? "<none>",
+            ]
+        )
+    }
+
+    // Validate that the document is dereferenceable, which
+    // catches reference cycles, which we don't yet support.
+    _ = try doc.locallyDereferenced()
+
+    // Also explicitly dereference the parts of components
+    // that the generator uses. `locallyDereferenced()` above
+    // only dereferences paths/operations, but not components.
+    let components = doc.components
+    try components.schemas.forEach { schema in
+        _ = try schema.value.dereferenced(in: components)
+    }
+    try components.parameters.forEach { schema in
+        _ = try schema.value.dereferenced(in: components)
+    }
+    try components.headers.forEach { schema in
+        _ = try schema.value.dereferenced(in: components)
+    }
+    try components.requestBodies.forEach { schema in
+        _ = try schema.value.dereferenced(in: components)
+    }
+    try components.responses.forEach { schema in
+        _ = try schema.value.dereferenced(in: components)
+    }
+    return diagnostics
+}

Tests/OpenAPIGeneratorCoreTests/Parser/Test_validateDoc.swift

@@ -0,0 +1,79 @@
+//===----------------------------------------------------------------------===//
+//
+// 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_validateDoc: Test_Core {
+
+    func testSchemaWarningIsNotFatal() throws {
+        let schemaWithWarnings = try loadSchemaFromYAML(
+            #"""
+            type: string
+            items:
+              type: integer
+            """#
+        )
+        let doc = OpenAPI.Document(
+            info: .init(title: "Test", version: "1.0.0"),
+            servers: [],
+            paths: [:],
+            components: .init(schemas: [
+                "myImperfectSchema": schemaWithWarnings
+            ])
+        )
+        let diagnostics = try validateDoc(
+            doc,
+            config: .init(
+                mode: .types,
+                featureFlags: [
+                    .strictOpenAPIValidation
+                ]
+            )
+        )
+        XCTAssertEqual(diagnostics.count, 1)
+    }
+
+    func testStructuralWarningIsFatal() throws {
+        let doc = OpenAPI.Document(
+            info: .init(title: "Test", version: "1.0.0"),
+            servers: [],
+            paths: [
+                "/foo": .b(
+                    .init(
+                        get: .init(
+                            requestBody: nil,
+
+                            // Fatal error: missing at least one response.
+                            responses: [:]
+                        )
+                    )
+                )
+            ],
+            components: .noComponents
+        )
+        XCTAssertThrowsError(
+            try validateDoc(
+                doc,
+                config: .init(
+                    mode: .types,
+                    featureFlags: [
+                        .strictOpenAPIValidation
+                    ]
+                )
+            )
+        )
+    }
+
+}

Tests/OpenAPIGeneratorCoreTests/TestUtilities.swift

@@ -54,6 +54,10 @@ class Test_Core: XCTestCase {
         )
     }
 
+    func loadSchemaFromYAML(_ yamlString: String) throws -> JSONSchema {
+        try YAMLDecoder().decode(JSONSchema.self, from: yamlString)
+    }
+
     static var testTypeName: TypeName {
         .init(swiftKeyPath: ["Foo"])
     }

Tests/OpenAPIGeneratorCoreTests/Translator/TypesTranslator/Test_translateSchemas.swift

@@ -21,15 +21,13 @@ class Test_translateSchemas: Test_Core {
     func testSchemaWarningsForwardedToGeneratorDiagnostics() throws {
         let typeName = TypeName(swiftKeyPath: ["Foo"])
 
-        let schemaWithWarnings = try YAMLDecoder()
-            .decode(
-                JSONSchema.self,
-                from: #"""
-                    type: string
-                    items:
-                      type: integer
-                    """#
-            )
+        let schemaWithWarnings = try loadSchemaFromYAML(
+            #"""
+            type: string
+            items:
+              type: integer
+            """#
+        )
 
         let cases: [(JSONSchema, [String])] = [
             (.string, []),