Skip to content

Commit 86ae4f6

Browse files
authored
Enforce error diagnostics by aborting execution (#607)
### Motivation - Fixes #180 ### Modifications - Create `ErrorThrowingDiagnosticCollector` wrapper collector. ### Result - The `ErrorThrowingDiagnosticCollector` throws an error when a diagnostic with severity `.error` is emited. ### Test Plan - Make sure all tests pass and add additional test.
1 parent 7c36ba9 commit 86ae4f6

20 files changed

+217
-49
lines changed

Package.swift

+10
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,16 @@ let package = Package(
114114
swiftSettings: swiftSettings
115115
),
116116

117+
// Test Target for swift-openapi-generator
118+
.testTarget(
119+
name: "OpenAPIGeneratorTests",
120+
dependencies: [
121+
"swift-openapi-generator", .product(name: "ArgumentParser", package: "swift-argument-parser"),
122+
],
123+
resources: [.copy("Resources")],
124+
swiftSettings: swiftSettings
125+
),
126+
117127
// Generator CLI
118128
.executableTarget(
119129
name: "swift-openapi-generator",

Sources/_OpenAPIGeneratorCore/Diagnostics.swift

+49-14
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,33 @@ public protocol DiagnosticCollector {
165165

166166
/// Submits a diagnostic to the collector.
167167
/// - Parameter diagnostic: The diagnostic to submit.
168-
func emit(_ diagnostic: Diagnostic)
168+
/// - Throws: An error if the implementing type determines that one should be thrown.
169+
func emit(_ diagnostic: Diagnostic) throws
170+
}
171+
172+
/// A type that conforms to the `DiagnosticCollector` protocol.
173+
///
174+
/// It receives diagnostics and forwards them to an upstream `DiagnosticCollector`.
175+
///
176+
/// If a diagnostic with a severity of `.error` is emitted, this collector will throw the diagnostic as an error.
177+
public struct ErrorThrowingDiagnosticCollector: DiagnosticCollector {
178+
let upstream: any DiagnosticCollector
179+
180+
/// Initializes a new `ErrorThrowingDiagnosticCollector` with an upstream `DiagnosticCollector`.
181+
///
182+
/// The upstream collector is where this collector will forward all received diagnostics.
183+
///
184+
/// - Parameter upstream: The `DiagnosticCollector` to which this collector will forward diagnostics.
185+
public init(upstream: any DiagnosticCollector) { self.upstream = upstream }
186+
187+
/// Emits a diagnostic to the collector.
188+
///
189+
/// - Parameter diagnostic: The diagnostic to be submitted.
190+
/// - Throws: The diagnostic itself if its severity is `.error`.
191+
public func emit(_ diagnostic: Diagnostic) throws {
192+
try upstream.emit(diagnostic)
193+
if diagnostic.severity == .error { throw diagnostic }
194+
}
169195
}
170196

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

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

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

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

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

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

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the SwiftOpenAPIGenerator open source project
4+
//
5+
// Copyright (c) 2023 Apple Inc. and the SwiftOpenAPIGenerator project authors
6+
// Licensed under Apache License v2.0
7+
//
8+
// See LICENSE.txt for license information
9+
// See CONTRIBUTORS.txt for the list of SwiftOpenAPIGenerator project authors
10+
//
11+
// SPDX-License-Identifier: Apache-2.0
12+
//
13+
//===----------------------------------------------------------------------===//
14+
15+
import Foundation
16+
17+
/// Prepares a diagnostics collector.
18+
/// - Parameter outputPath: A file path where to persist the YAML file. If `nil`, diagnostics will be printed to stderr.
19+
/// - Returns: A tuple containing:
20+
/// - An instance of `DiagnosticCollector` conforming to `Sendable`.
21+
/// - A closure to finalize the diagnostics collection
22+
public func preparedDiagnosticsCollector(outputPath: URL?) -> (any DiagnosticCollector & Sendable, () throws -> Void) {
23+
let innerDiagnostics: any DiagnosticCollector & Sendable
24+
let finalizeDiagnostics: () throws -> Void
25+
26+
if let outputPath {
27+
let _diagnostics = _YamlFileDiagnosticsCollector(url: outputPath)
28+
finalizeDiagnostics = _diagnostics.finalize
29+
innerDiagnostics = _diagnostics
30+
} else {
31+
innerDiagnostics = StdErrPrintingDiagnosticCollector()
32+
finalizeDiagnostics = {}
33+
}
34+
let diagnostics = ErrorThrowingDiagnosticCollector(upstream: innerDiagnostics)
35+
return (diagnostics, finalizeDiagnostics)
36+
}

Sources/_OpenAPIGeneratorCore/GeneratorPipeline.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func makeGeneratorPipeline(
110110
}
111111
let validateDoc = { (doc: OpenAPI.Document) -> OpenAPI.Document in
112112
let validationDiagnostics = try validator(doc, config)
113-
for diagnostic in validationDiagnostics { diagnostics.emit(diagnostic) }
113+
for diagnostic in validationDiagnostics { try diagnostics.emit(diagnostic) }
114114
return doc
115115
}
116116
return .init(

Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateObjectStruct.swift

+2-3
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ extension TypesFileTranslator {
3232
objectContext: JSONSchema.ObjectContext,
3333
isDeprecated: Bool
3434
) throws -> Declaration {
35-
3635
let documentedProperties: [PropertyBlueprint] = try objectContext.properties
3736
.filter { key, value in
3837

@@ -42,7 +41,7 @@ extension TypesFileTranslator {
4241
// have a proper definition in the `properties` map are skipped, as they
4342
// often imply a typo or a mistake in the document. So emit a diagnostic as well.
4443
guard !value.inferred else {
45-
diagnostics.emit(
44+
try diagnostics.emit(
4645
.warning(
4746
message:
4847
"A property name only appears in the required list, but not in the properties map - this is likely a typo; skipping this property.",
@@ -63,7 +62,7 @@ extension TypesFileTranslator {
6362
// allowed in object properties, explicitly filter these out
6463
// here.
6564
if value.isString && value.formatString == "binary" {
66-
diagnostics.emitUnsupportedSchema(
65+
try diagnostics.emitUnsupportedSchema(
6766
reason: "Binary properties in object schemas.",
6867
schema: value,
6968
foundIn: foundIn

Sources/_OpenAPIGeneratorCore/Translator/CommonTranslations/translateSchema.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ extension TypesFileTranslator {
7676

7777
// Attach any warnings from the parsed schema as a diagnostic.
7878
for warning in schema.warnings {
79-
diagnostics.emit(
79+
try diagnostics.emit(
8080
.warning(
8181
message: "Schema warning: \(warning.description)",
8282
context: [

Sources/_OpenAPIGeneratorCore/Translator/Content/ContentInspector.swift

+6-6
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ extension FileTranslator {
123123
-> SchemaContent?
124124
{
125125
guard !map.isEmpty else { return nil }
126-
if map.count > 1 { diagnostics.emitUnsupported("Multiple content types", foundIn: foundIn) }
126+
if map.count > 1 { try diagnostics.emitUnsupported("Multiple content types", foundIn: foundIn) }
127127
let mapWithContentTypes = try map.map { key, content in try (type: key.asGeneratorContentType, value: content) }
128128

129129
let chosenContent: (type: ContentType, schema: SchemaContent, content: OpenAPI.Content)?
@@ -137,15 +137,15 @@ extension FileTranslator {
137137
contentValue
138138
)
139139
} else {
140-
diagnostics.emitUnsupported("Unsupported content", foundIn: foundIn)
140+
try diagnostics.emitUnsupported("Unsupported content", foundIn: foundIn)
141141
chosenContent = nil
142142
}
143143
if let chosenContent {
144144
let contentType = chosenContent.type
145145
if contentType.lowercasedType == "multipart"
146146
|| contentType.lowercasedTypeAndSubtype.contains("application/x-www-form-urlencoded")
147147
{
148-
diagnostics.emitUnsupportedIfNotNil(
148+
try diagnostics.emitUnsupportedIfNotNil(
149149
chosenContent.content.encoding,
150150
"Custom encoding for multipart/formEncoded content",
151151
foundIn: "\(foundIn), content \(contentType.originallyCasedTypeAndSubtype)"
@@ -181,7 +181,7 @@ extension FileTranslator {
181181
) throws -> SchemaContent? {
182182
let contentType = try contentKey.asGeneratorContentType
183183
if contentType.lowercasedTypeAndSubtype.contains("application/x-www-form-urlencoded") {
184-
diagnostics.emitUnsupportedIfNotNil(
184+
try diagnostics.emitUnsupportedIfNotNil(
185185
contentValue.encoding,
186186
"Custom encoding for formEncoded content",
187187
foundIn: "\(foundIn), content \(contentType.originallyCasedTypeAndSubtype)"
@@ -191,7 +191,7 @@ extension FileTranslator {
191191
if contentType.isUrlEncodedForm { return .init(contentType: contentType, schema: contentValue.schema) }
192192
if contentType.isMultipart {
193193
guard isRequired else {
194-
diagnostics.emit(
194+
try diagnostics.emit(
195195
.warning(
196196
message:
197197
"Multipart request bodies must always be required, but found an optional one - skipping. Mark as `required: true` to get this body generated.",
@@ -205,7 +205,7 @@ extension FileTranslator {
205205
if !excludeBinary, contentType.isBinary {
206206
return .init(contentType: contentType, schema: .b(.string(contentEncoding: .binary)))
207207
}
208-
diagnostics.emitUnsupported("Unsupported content", foundIn: foundIn)
208+
try diagnostics.emitUnsupported("Unsupported content", foundIn: foundIn)
209209
return nil
210210
}
211211
}

Sources/_OpenAPIGeneratorCore/Translator/Multipart/MultipartContentInspector.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ extension FileTranslator {
312312
}
313313
let contentType = finalContentTypeSource.contentType
314314
if finalContentTypeSource.contentType.isMultipart {
315-
diagnostics.emitUnsupported("Multipart part cannot nest another multipart content.", foundIn: foundIn)
315+
try diagnostics.emitUnsupported("Multipart part cannot nest another multipart content.", foundIn: foundIn)
316316
return nil
317317
}
318318
let info = MultipartPartInfo(repetition: repetitionKind, contentTypeSource: finalContentTypeSource)

Sources/_OpenAPIGeneratorCore/Translator/Parameters/TypedParameter.swift

+3-3
Original file line numberDiff line numberDiff line change
@@ -140,22 +140,22 @@ extension FileTranslator {
140140
switch location {
141141
case .query:
142142
guard case .form = style else {
143-
diagnostics.emitUnsupported(
143+
try diagnostics.emitUnsupported(
144144
"Query params of style \(style.rawValue), explode: \(explode)",
145145
foundIn: foundIn
146146
)
147147
return nil
148148
}
149149
case .header, .path:
150150
guard case .simple = style else {
151-
diagnostics.emitUnsupported(
151+
try diagnostics.emitUnsupported(
152152
"\(location.rawValue) params of style \(style.rawValue), explode: \(explode)",
153153
foundIn: foundIn
154154
)
155155
return nil
156156
}
157157
case .cookie:
158-
diagnostics.emitUnsupported("Cookie params", foundIn: foundIn)
158+
try diagnostics.emitUnsupported("Cookie params", foundIn: foundIn)
159159
return nil
160160
}
161161

Sources/_OpenAPIGeneratorCore/Translator/Parameters/translateParameter.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ extension ClientFileTranslator {
114114
containerExpr = .identifierPattern(requestVariableName)
115115
supportsStyleAndExplode = true
116116
default:
117-
diagnostics.emitUnsupported(
117+
try diagnostics.emitUnsupported(
118118
"Parameter of type \(parameter.location.rawValue)",
119119
foundIn: parameter.description
120120
)
@@ -198,7 +198,7 @@ extension ServerFileTranslator {
198198
])
199199
)
200200
default:
201-
diagnostics.emitUnsupported(
201+
try diagnostics.emitUnsupported(
202202
"Parameter of type \(parameter.location)",
203203
foundIn: "\(typedParameter.description)"
204204
)

Sources/_OpenAPIGeneratorCore/Translator/TypeAssignment/isSchemaSupported.swift

+3-3
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ extension FileTranslator {
6464
switch try isSchemaSupported(schema, referenceStack: &referenceStack) {
6565
case .supported: return true
6666
case .unsupported(reason: let reason, schema: let schema):
67-
diagnostics.emitUnsupportedSchema(reason: reason.description, schema: schema, foundIn: foundIn)
67+
try diagnostics.emitUnsupportedSchema(reason: reason.description, schema: schema, foundIn: foundIn)
6868
return false
6969
}
7070
}
@@ -82,7 +82,7 @@ extension FileTranslator {
8282
switch try isSchemaSupported(schema, referenceStack: &referenceStack) {
8383
case .supported: return true
8484
case .unsupported(reason: let reason, schema: let schema):
85-
diagnostics.emitUnsupportedSchema(reason: reason.description, schema: schema, foundIn: foundIn)
85+
try diagnostics.emitUnsupportedSchema(reason: reason.description, schema: schema, foundIn: foundIn)
8686
return false
8787
}
8888
}
@@ -100,7 +100,7 @@ extension FileTranslator {
100100
switch try isObjectOrRefToObjectSchemaAndSupported(schema, referenceStack: &referenceStack) {
101101
case .supported: return true
102102
case .unsupported(reason: let reason, schema: let schema):
103-
diagnostics.emitUnsupportedSchema(reason: reason.description, schema: schema, foundIn: foundIn)
103+
try diagnostics.emitUnsupportedSchema(reason: reason.description, schema: schema, foundIn: foundIn)
104104
return false
105105
}
106106
}

Sources/_OpenAPIGeneratorCore/Translator/TypesTranslator/translateBoxedTypes.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ extension TypesFileTranslator {
3333
var decls = decls
3434
for (index, decl) in decls.enumerated() {
3535
guard let name = decl.name, boxedNames.contains(name) else { continue }
36-
diagnostics.emit(
36+
try diagnostics.emit(
3737
.note(
3838
message: "Detected a recursive type; it will be boxed to break the reference cycle.",
3939
context: ["name": name]

Sources/swift-openapi-generator/YamlFileDiagnosticsCollector.swift Sources/_OpenAPIGeneratorCore/YamlFileDiagnosticsCollector.swift

-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
//===----------------------------------------------------------------------===//
1414
import Foundation
1515
import Yams
16-
import _OpenAPIGeneratorCore
1716

1817
struct _DiagnosticsYamlFileContent: Encodable {
1918
var uniqueMessages: [String]

Sources/swift-openapi-generator/GenerateOptions+runGenerator.swift

+2-12
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,7 @@ extension _GenerateOptions {
4242
featureFlags: resolvedFeatureFlags
4343
)
4444
}
45-
let diagnostics: any DiagnosticCollector & Sendable
46-
let finalizeDiagnostics: () throws -> Void
47-
if let diagnosticsOutputPath {
48-
let _diagnostics = _YamlFileDiagnosticsCollector(url: diagnosticsOutputPath)
49-
finalizeDiagnostics = _diagnostics.finalize
50-
diagnostics = _diagnostics
51-
} else {
52-
diagnostics = StdErrPrintingDiagnosticCollector()
53-
finalizeDiagnostics = {}
54-
}
55-
45+
let (diagnostics, finalizeDiagnostics) = preparedDiagnosticsCollector(outputPath: diagnosticsOutputPath)
5646
let doc = self.docPath
5747
print(
5848
"""
@@ -83,7 +73,7 @@ extension _GenerateOptions {
8373
try finalizeDiagnostics()
8474
} catch let error as Diagnostic {
8575
// Emit our nice Diagnostics message instead of relying on ArgumentParser output.
86-
diagnostics.emit(error)
76+
try diagnostics.emit(error)
8777
try finalizeDiagnostics()
8878
throw ExitCode.failure
8979
} catch {

0 commit comments

Comments
 (0)