Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce error diagnostics by aborting execution #607

10 changes: 10 additions & 0 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,16 @@ let package = Package(
swiftSettings: swiftSettings
),

// Test Target for swift-openapi-generator
.testTarget(
name: "OpenAPIGeneratorTests",
dependencies: [
"swift-openapi-generator", .product(name: "ArgumentParser", package: "swift-argument-parser"),
],
resources: [.copy("Resources")],
swiftSettings: swiftSettings
),

// Generator CLI
.executableTarget(
name: "swift-openapi-generator",
Expand Down
63 changes: 49 additions & 14 deletions Sources/_OpenAPIGeneratorCore/Diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,33 @@ public protocol DiagnosticCollector {

/// Submits a diagnostic to the collector.
/// - Parameter diagnostic: The diagnostic to submit.
czechboy0 marked this conversation as resolved.
Show resolved Hide resolved
func emit(_ diagnostic: Diagnostic)
/// - Throws: An error if the implementing type determines that one should be thrown.
func emit(_ diagnostic: Diagnostic) throws
}

/// A type that conforms to the `DiagnosticCollector` protocol.
///
/// It receives diagnostics and forwards them to an upstream `DiagnosticCollector`.
///
/// If a diagnostic with a severity of `.error` is emitted, this collector will throw the diagnostic as an error.
public struct ErrorThrowingDiagnosticCollector: DiagnosticCollector {
let upstream: any DiagnosticCollector

/// Initializes a new `ErrorThrowingDiagnosticCollector` with an upstream `DiagnosticCollector`.
///
/// The upstream collector is where this collector will forward all received diagnostics.
///
/// - Parameter upstream: The `DiagnosticCollector` to which this collector will forward diagnostics.
public init(upstream: any DiagnosticCollector) { self.upstream = upstream }

/// Emits a diagnostic to the collector.
///
/// - Parameter diagnostic: The diagnostic to be submitted.
/// - Throws: The diagnostic itself if its severity is `.error`.
public func emit(_ diagnostic: Diagnostic) throws {
try upstream.emit(diagnostic)
if diagnostic.severity == .error { throw diagnostic }
}
}

extension DiagnosticCollector {
Expand All @@ -180,8 +206,9 @@ extension DiagnosticCollector {
/// feature was detected.
/// - context: A set of key-value pairs that help the user understand
/// where the warning occurred.
func emitUnsupported(_ feature: String, foundIn: String, context: [String: String] = [:]) {
emit(Diagnostic.unsupported(feature, foundIn: foundIn, context: context))
/// - Throws: This method will throw the diagnostic if the severity of the diagnostic is `.error`.
func emitUnsupported(_ feature: String, foundIn: String, context: [String: String] = [:]) throws {
try emit(Diagnostic.unsupported(feature, foundIn: foundIn, context: context))
}

/// Emits a diagnostic for an unsupported schema found in the specified
Expand All @@ -193,9 +220,10 @@ extension DiagnosticCollector {
/// schema was detected.
/// - context: A set of key-value pairs that help the user understand
/// where the warning occurred.
func emitUnsupportedSchema(reason: String, schema: JSONSchema, foundIn: String, context: [String: String] = [:]) {
emit(Diagnostic.unsupportedSchema(reason: reason, schema: schema, foundIn: foundIn, context: context))
}
/// - Throws: This method will throw the diagnostic if the severity of the diagnostic is `.error`.
func emitUnsupportedSchema(reason: String, schema: JSONSchema, foundIn: String, context: [String: String] = [:])
throws
{ try emit(Diagnostic.unsupportedSchema(reason: reason, schema: schema, foundIn: foundIn, context: context)) }

/// Emits a diagnostic for an unsupported feature found in the specified
/// type name.
Expand All @@ -206,8 +234,9 @@ extension DiagnosticCollector {
/// - foundIn: The type name related to where the issue was detected.
/// - context: A set of key-value pairs that help the user understand
/// where the warning occurred.
func emitUnsupported(_ feature: String, foundIn: TypeName, context: [String: String] = [:]) {
emit(Diagnostic.unsupported(feature, foundIn: foundIn.description, context: context))
/// - Throws: This method will throw the diagnostic if the severity of the diagnostic is `.error`.
func emitUnsupported(_ feature: String, foundIn: TypeName, context: [String: String] = [:]) throws {
try emit(Diagnostic.unsupported(feature, foundIn: foundIn.description, context: context))
}

/// Emits a diagnostic for an unsupported feature found in the specified
Expand All @@ -222,9 +251,12 @@ extension DiagnosticCollector {
/// feature was detected.
/// - context: A set of key-value pairs that help the user understand
/// where the warning occurred.
func emitUnsupportedIfNotNil(_ test: Any?, _ feature: String, foundIn: String, context: [String: String] = [:]) {
/// - Throws: This method will throw the diagnostic if the severity of the diagnostic is `.error`.
func emitUnsupportedIfNotNil(_ test: Any?, _ feature: String, foundIn: String, context: [String: String] = [:])
throws
{
if test == nil { return }
emitUnsupported(feature, foundIn: foundIn, context: context)
try emitUnsupported(feature, foundIn: foundIn, context: context)
}

/// Emits a diagnostic for an unsupported feature found in the specified
Expand All @@ -239,14 +271,15 @@ extension DiagnosticCollector {
/// feature was detected.
/// - context: A set of key-value pairs that help the user understand
/// where the warning occurred.
/// - Throws: This method will throw the diagnostic if the severity of the diagnostic is `.error`.
func emitUnsupportedIfNotEmpty<C: Collection>(
_ test: C?,
_ feature: String,
foundIn: String,
context: [String: String] = [:]
) {
) throws {
guard let test = test, !test.isEmpty else { return }
emitUnsupported(feature, foundIn: foundIn, context: context)
try emitUnsupported(feature, foundIn: foundIn, context: context)
}

/// Emits a diagnostic for an unsupported feature found in the specified
Expand All @@ -261,9 +294,11 @@ extension DiagnosticCollector {
/// feature was detected.
/// - context: A set of key-value pairs that help the user understand
/// where the warning occurred.
func emitUnsupportedIfTrue(_ test: Bool, _ feature: String, foundIn: String, context: [String: String] = [:]) {
/// - Throws: This method will throw the diagnostic if the severity of the diagnostic is `.error`.
func emitUnsupportedIfTrue(_ test: Bool, _ feature: String, foundIn: String, context: [String: String] = [:]) throws
{
if !test { return }
emitUnsupported(feature, foundIn: foundIn, context: context)
try emitUnsupported(feature, foundIn: foundIn, context: context)
}
}

Expand Down
36 changes: 36 additions & 0 deletions Sources/_OpenAPIGeneratorCore/DiagnosticsCollectorProvider.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
//===----------------------------------------------------------------------===//
//
// 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 Foundation

/// Prepares a diagnostics collector.
/// - Parameter outputPath: A file path where to persist the YAML file. If `nil`, diagnostics will be printed to stderr.
/// - Returns: A tuple containing:
/// - An instance of `DiagnosticCollector` conforming to `Sendable`.
/// - A closure to finalize the diagnostics collection
public func preparedDiagnosticsCollector(outputPath: URL?) -> (any DiagnosticCollector & Sendable, () throws -> Void) {
let innerDiagnostics: any DiagnosticCollector & Sendable
let finalizeDiagnostics: () throws -> Void

if let outputPath {
let _diagnostics = _YamlFileDiagnosticsCollector(url: outputPath)
finalizeDiagnostics = _diagnostics.finalize
innerDiagnostics = _diagnostics
} else {
innerDiagnostics = StdErrPrintingDiagnosticCollector()
finalizeDiagnostics = {}
}
let diagnostics = ErrorThrowingDiagnosticCollector(upstream: innerDiagnostics)
return (diagnostics, finalizeDiagnostics)
}
2 changes: 1 addition & 1 deletion Sources/_OpenAPIGeneratorCore/GeneratorPipeline.swift
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func makeGeneratorPipeline(
}
let validateDoc = { (doc: OpenAPI.Document) -> OpenAPI.Document in
let validationDiagnostics = try validator(doc, config)
for diagnostic in validationDiagnostics { diagnostics.emit(diagnostic) }
for diagnostic in validationDiagnostics { try diagnostics.emit(diagnostic) }
return doc
}
return .init(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ extension TypesFileTranslator {
objectContext: JSONSchema.ObjectContext,
isDeprecated: Bool
) throws -> Declaration {

let documentedProperties: [PropertyBlueprint] = try objectContext.properties
.filter { key, value in

Expand All @@ -42,7 +41,7 @@ extension TypesFileTranslator {
// have a proper definition in the `properties` map are skipped, as they
// often imply a typo or a mistake in the document. So emit a diagnostic as well.
guard !value.inferred else {
diagnostics.emit(
try diagnostics.emit(
.warning(
message:
"A property name only appears in the required list, but not in the properties map - this is likely a typo; skipping this property.",
Expand All @@ -63,7 +62,7 @@ extension TypesFileTranslator {
// allowed in object properties, explicitly filter these out
// here.
if value.isString && value.formatString == "binary" {
diagnostics.emitUnsupportedSchema(
try diagnostics.emitUnsupportedSchema(
reason: "Binary properties in object schemas.",
schema: value,
foundIn: foundIn
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ extension TypesFileTranslator {

// Attach any warnings from the parsed schema as a diagnostic.
for warning in schema.warnings {
diagnostics.emit(
try diagnostics.emit(
.warning(
message: "Schema warning: \(warning.description)",
context: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ extension FileTranslator {
-> SchemaContent?
{
guard !map.isEmpty else { return nil }
if map.count > 1 { diagnostics.emitUnsupported("Multiple content types", foundIn: foundIn) }
if map.count > 1 { try diagnostics.emitUnsupported("Multiple content types", foundIn: foundIn) }
let mapWithContentTypes = try map.map { key, content in try (type: key.asGeneratorContentType, value: content) }

let chosenContent: (type: ContentType, schema: SchemaContent, content: OpenAPI.Content)?
Expand All @@ -137,15 +137,15 @@ extension FileTranslator {
contentValue
)
} else {
diagnostics.emitUnsupported("Unsupported content", foundIn: foundIn)
try diagnostics.emitUnsupported("Unsupported content", foundIn: foundIn)
chosenContent = nil
}
if let chosenContent {
let contentType = chosenContent.type
if contentType.lowercasedType == "multipart"
|| contentType.lowercasedTypeAndSubtype.contains("application/x-www-form-urlencoded")
{
diagnostics.emitUnsupportedIfNotNil(
try diagnostics.emitUnsupportedIfNotNil(
chosenContent.content.encoding,
"Custom encoding for multipart/formEncoded content",
foundIn: "\(foundIn), content \(contentType.originallyCasedTypeAndSubtype)"
Expand Down Expand Up @@ -181,7 +181,7 @@ extension FileTranslator {
) throws -> SchemaContent? {
let contentType = try contentKey.asGeneratorContentType
if contentType.lowercasedTypeAndSubtype.contains("application/x-www-form-urlencoded") {
diagnostics.emitUnsupportedIfNotNil(
try diagnostics.emitUnsupportedIfNotNil(
contentValue.encoding,
"Custom encoding for formEncoded content",
foundIn: "\(foundIn), content \(contentType.originallyCasedTypeAndSubtype)"
Expand All @@ -191,7 +191,7 @@ extension FileTranslator {
if contentType.isUrlEncodedForm { return .init(contentType: contentType, schema: contentValue.schema) }
if contentType.isMultipart {
guard isRequired else {
diagnostics.emit(
try diagnostics.emit(
.warning(
message:
"Multipart request bodies must always be required, but found an optional one - skipping. Mark as `required: true` to get this body generated.",
Expand All @@ -205,7 +205,7 @@ extension FileTranslator {
if !excludeBinary, contentType.isBinary {
return .init(contentType: contentType, schema: .b(.string(contentEncoding: .binary)))
}
diagnostics.emitUnsupported("Unsupported content", foundIn: foundIn)
try diagnostics.emitUnsupported("Unsupported content", foundIn: foundIn)
return nil
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ extension FileTranslator {
}
let contentType = finalContentTypeSource.contentType
if finalContentTypeSource.contentType.isMultipart {
diagnostics.emitUnsupported("Multipart part cannot nest another multipart content.", foundIn: foundIn)
try diagnostics.emitUnsupported("Multipart part cannot nest another multipart content.", foundIn: foundIn)
return nil
}
let info = MultipartPartInfo(repetition: repetitionKind, contentTypeSource: finalContentTypeSource)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,22 +140,22 @@ extension FileTranslator {
switch location {
case .query:
guard case .form = style else {
diagnostics.emitUnsupported(
try diagnostics.emitUnsupported(
"Query params of style \(style.rawValue), explode: \(explode)",
foundIn: foundIn
)
return nil
}
case .header, .path:
guard case .simple = style else {
diagnostics.emitUnsupported(
try diagnostics.emitUnsupported(
"\(location.rawValue) params of style \(style.rawValue), explode: \(explode)",
foundIn: foundIn
)
return nil
}
case .cookie:
diagnostics.emitUnsupported("Cookie params", foundIn: foundIn)
try diagnostics.emitUnsupported("Cookie params", foundIn: foundIn)
return nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ extension ClientFileTranslator {
containerExpr = .identifierPattern(requestVariableName)
supportsStyleAndExplode = true
default:
diagnostics.emitUnsupported(
try diagnostics.emitUnsupported(
"Parameter of type \(parameter.location.rawValue)",
foundIn: parameter.description
)
Expand Down Expand Up @@ -198,7 +198,7 @@ extension ServerFileTranslator {
])
)
default:
diagnostics.emitUnsupported(
try diagnostics.emitUnsupported(
"Parameter of type \(parameter.location)",
foundIn: "\(typedParameter.description)"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ extension FileTranslator {
switch try isSchemaSupported(schema, referenceStack: &referenceStack) {
case .supported: return true
case .unsupported(reason: let reason, schema: let schema):
diagnostics.emitUnsupportedSchema(reason: reason.description, schema: schema, foundIn: foundIn)
try diagnostics.emitUnsupportedSchema(reason: reason.description, schema: schema, foundIn: foundIn)
return false
}
}
Expand All @@ -82,7 +82,7 @@ extension FileTranslator {
switch try isSchemaSupported(schema, referenceStack: &referenceStack) {
case .supported: return true
case .unsupported(reason: let reason, schema: let schema):
diagnostics.emitUnsupportedSchema(reason: reason.description, schema: schema, foundIn: foundIn)
try diagnostics.emitUnsupportedSchema(reason: reason.description, schema: schema, foundIn: foundIn)
return false
}
}
Expand All @@ -100,7 +100,7 @@ extension FileTranslator {
switch try isObjectOrRefToObjectSchemaAndSupported(schema, referenceStack: &referenceStack) {
case .supported: return true
case .unsupported(reason: let reason, schema: let schema):
diagnostics.emitUnsupportedSchema(reason: reason.description, schema: schema, foundIn: foundIn)
try diagnostics.emitUnsupportedSchema(reason: reason.description, schema: schema, foundIn: foundIn)
return false
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ extension TypesFileTranslator {
var decls = decls
for (index, decl) in decls.enumerated() {
guard let name = decl.name, boxedNames.contains(name) else { continue }
diagnostics.emit(
try diagnostics.emit(
.note(
message: "Detected a recursive type; it will be boxed to break the reference cycle.",
context: ["name": name]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
//===----------------------------------------------------------------------===//
import Foundation
import Yams
import _OpenAPIGeneratorCore

struct _DiagnosticsYamlFileContent: Encodable {
var uniqueMessages: [String]
Expand Down
14 changes: 2 additions & 12 deletions Sources/swift-openapi-generator/GenerateOptions+runGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,7 @@ extension _GenerateOptions {
featureFlags: resolvedFeatureFlags
)
}
let diagnostics: any DiagnosticCollector & Sendable
let finalizeDiagnostics: () throws -> Void
if let diagnosticsOutputPath {
let _diagnostics = _YamlFileDiagnosticsCollector(url: diagnosticsOutputPath)
finalizeDiagnostics = _diagnostics.finalize
diagnostics = _diagnostics
} else {
diagnostics = StdErrPrintingDiagnosticCollector()
finalizeDiagnostics = {}
}

let (diagnostics, finalizeDiagnostics) = preparedDiagnosticsCollector(outputPath: diagnosticsOutputPath)
let doc = self.docPath
print(
"""
Expand Down Expand Up @@ -83,7 +73,7 @@ extension _GenerateOptions {
try finalizeDiagnostics()
} catch let error as Diagnostic {
// Emit our nice Diagnostics message instead of relying on ArgumentParser output.
diagnostics.emit(error)
try diagnostics.emit(error)
try finalizeDiagnostics()
throw ExitCode.failure
} catch {
Expand Down
Loading