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

Emit YAML processing errors that Xcode can display #81

4 changes: 2 additions & 2 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ let package = Package(
.product(name: "SwiftSyntaxBuilder", package: "swift-syntax"),
.product(name: "SwiftFormat", package: "swift-format"),
.product(name: "SwiftFormatConfiguration", package: "swift-format"),
.product(name: "ArgumentParser", package: "swift-argument-parser"),
]
),

Expand Down Expand Up @@ -116,8 +117,7 @@ let package = Package(
.executableTarget(
name: "swift-openapi-generator",
dependencies: [
"_OpenAPIGeneratorCore",
.product(name: "ArgumentParser", package: "swift-argument-parser"),
"_OpenAPIGeneratorCore"
czechboy0 marked this conversation as resolved.
Show resolved Hide resolved
]
),

Expand Down
20 changes: 19 additions & 1 deletion Sources/_OpenAPIGeneratorCore/Diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ public struct Diagnostic: Error, Codable {
/// A user-friendly description of the diagnostic.
public var message: String

/// The absolute path to a specific source file that triggered the diagnostic.
public var absoluteFilePath: URL?

/// The line number within the specific source file that triggered the diagnostic.
public var lineNumber: Int?
czechboy0 marked this conversation as resolved.
Show resolved Hide resolved

/// Additional information about where the issue occurred.
public var context: [String: String] = [:]

Expand Down Expand Up @@ -101,8 +107,20 @@ extension Diagnostic.Severity: CustomStringConvertible {

extension Diagnostic: CustomStringConvertible {
public var description: String {
var prefix = ""
if let filePath = absoluteFilePath {
if #available(macOS 13.0, *) {
prefix = "\(filePath.path(percentEncoded: false)):"
} else {
prefix = "\(filePath.path):"
czechboy0 marked this conversation as resolved.
Show resolved Hide resolved
}
if let line = lineNumber {
prefix += "\(line):"
}
prefix += " "
}
let contextString = context.map { "\($0)=\($1)" }.sorted().joined(separator: ", ")
return "\(severity): \(message) [\(contextString.isEmpty ? "" : "context: \(contextString)")]"
return "\(prefix)\(severity): \(message)\(contextString.isEmpty ? "" : " [context: \(contextString)]")"
}
}

Expand Down
81 changes: 71 additions & 10 deletions Sources/_OpenAPIGeneratorCore/Parser/YamsParser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
import Yams
import OpenAPIKit30
import ArgumentParser
import Foundation
import OpenAPIKit30
import Yams

/// A parser that uses the Yams library to parse the provided
/// raw file into an OpenAPI document.
Expand Down Expand Up @@ -43,10 +44,16 @@ struct YamsParser: ParserProtocol {
}
}

let versionedDocument = try decoder.decode(
OpenAPIVersionedDocument.self,
from: openapiData
)
let versionedDocument: OpenAPIVersionedDocument
do {
versionedDocument = try decoder.decode(
OpenAPIVersionedDocument.self,
from: openapiData
)
} catch DecodingError.dataCorrupted(let errorContext) {
try possibly_throw_parsing_error(context: errorContext, input: input, diagnostics: diagnostics)
throw DecodingError.dataCorrupted(errorContext)
}

guard let openAPIVersion = versionedDocument.openapi else {
throw OpenAPIMissingVersionError()
Expand All @@ -58,9 +65,63 @@ struct YamsParser: ParserProtocol {
throw OpenAPIVersionError(versionString: "openapi: \(openAPIVersion)")
}

return try decoder.decode(
OpenAPI.Document.self,
from: input.contents
)
do {
return try decoder.decode(
OpenAPI.Document.self,
from: input.contents
)
} catch DecodingError.dataCorrupted(let errorContext) {
try possibly_throw_parsing_error(context: errorContext, input: input, diagnostics: diagnostics)
throw DecodingError.dataCorrupted(errorContext)
}
}

/// Detect specific YAML parsing errors to emit nicely formatted errors for IDEs
private func possibly_throw_parsing_error(
czechboy0 marked this conversation as resolved.
Show resolved Hide resolved
context: DecodingError.Context,
input: InMemoryInputFile,
diagnostics: DiagnosticCollector
) throws {
if let yamlError = context.underlyingError as? YamlError {
if case .parser(let yamlContext, let yamlProblem, let yamlMark, _) = yamlError {
try throw_parsing_error(
context: yamlContext,
problem: yamlProblem,
lineNumber: yamlMark.line - 1,
input: input,
diagnostics
)
} else if case .scanner(let yamlContext, let yamlProblem, let yamlMark, _) = yamlError {
try throw_parsing_error(
context: yamlContext,
problem: yamlProblem,
lineNumber: yamlMark.line - 1,
input: input,
diagnostics
)
}
} else if let openAPIError = context.underlyingError as? OpenAPIError {
var problem = Diagnostic.error(message: openAPIError.localizedDescription)
czechboy0 marked this conversation as resolved.
Show resolved Hide resolved
problem.absoluteFilePath = input.absolutePath
diagnostics.emit(problem)
throw ExitCode.failure
czechboy0 marked this conversation as resolved.
Show resolved Hide resolved
}
}

private func throw_parsing_error(
czechboy0 marked this conversation as resolved.
Show resolved Hide resolved
simonjbeaumont marked this conversation as resolved.
Show resolved Hide resolved
context: YamlError.Context?,
problem: String,
lineNumber: Int,
input: InMemoryInputFile,
_ diagnostics: DiagnosticCollector
) throws {
let text = "\(problem) \(context?.description ?? "")"

var problem = Diagnostic.error(message: text)
problem.absoluteFilePath = input.absolutePath
problem.lineNumber = lineNumber
diagnostics.emit(problem)

throw ExitCode.failure
}
}
152 changes: 140 additions & 12 deletions Tests/OpenAPIGeneratorCoreTests/Parser/Test_YamsParser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
//===----------------------------------------------------------------------===//
import XCTest
@testable import _OpenAPIGeneratorCore
import ArgumentParser

final class Test_YamsParser: Test_Core {

Expand All @@ -25,24 +26,151 @@ final class Test_YamsParser: Test_Core {
XCTAssertThrowsError(try _test("2.0"))
}

func _test(_ openAPIVersionString: String) throws -> ParsedOpenAPIRepresentation {
private func _test(_ openAPIVersionString: String) throws -> ParsedOpenAPIRepresentation {
try _test(
"""
openapi: "\(openAPIVersionString)"
info:
title: "Test"
version: "1.0.0"
paths: {}
""",
diagnostics: PrintingDiagnosticCollector()
)
}

func testEmitsYamsParsingError() throws {
let collector = TestingDiagnosticsCollector()
// The `title: "Test"` line is indented the wrong amount to make the YAML invalid for the parser
let yaml = """
openapi: "3.0.0"
info:
title: "Test"
version: 1.0.0
paths: {}
"""

XCTAssertThrowsError(try _test(yaml, diagnostics: collector)) { error in
if let exitError = error as? ExitCode {
czechboy0 marked this conversation as resolved.
Show resolved Hide resolved
XCTAssertEqual(exitError, ExitCode.failure)
} else {
XCTFail("Thrown error is \(type(of: error)) but should be ExitCode.failure")
}

XCTAssertEqual(collector.allOutput.count, 1)
if let actualDiagnostic = collector.allOutput.first {
let expectedDiagnostic =
"/foo.yaml:3: error: did not find expected key while parsing a block mapping in line 3, column 2\n"
XCTAssertEqual(actualDiagnostic, expectedDiagnostic)
}
}
}

func testEmitsYamsScanningError() throws {
let collector = TestingDiagnosticsCollector()
// The `version:"1.0.0"` line is missing a space after the colon to make it invalid YAML for the scanner
let yaml = """
openapi: "3.0.0"
info:
title: "Test"
version:"1.0.0"
paths: {}
"""

XCTAssertThrowsError(try _test(yaml, diagnostics: collector)) { error in
if let exitError = error as? ExitCode {
XCTAssertEqual(exitError, ExitCode.failure)
} else {
XCTFail("Thrown error is \(type(of: error)) but should be ExitCode.failure")
}

XCTAssertEqual(collector.allOutput.count, 1)
if let actualDiagnostic = collector.allOutput.first {
let expectedDiagnostic =
"/foo.yaml:4: error: could not find expected ':' while scanning a simple key in line 4, column 3\n"
XCTAssertEqual(actualDiagnostic, expectedDiagnostic)
}
}
}

func testEmitsMissingInfoKeyOpenAPIParsingError() throws {
let collector = TestingDiagnosticsCollector()
// The `smurf` line should be `info` in a real OpenAPI document.
let yaml = """
openapi: "3.0.0"
smurf:
title: "Test"
version: "1.0.0"
paths: {}
"""

XCTAssertThrowsError(try _test(yaml, diagnostics: collector)) { error in
if let exitError = error as? ExitCode {
XCTAssertEqual(exitError, ExitCode.failure)
} else {
XCTFail("Thrown error is \(type(of: error)) but should be ExitCode.failure")
}

XCTAssertEqual(collector.allOutput.count, 1)
if let actualDiagnostic = collector.allOutput.first {
let expectedDiagnostic =
"/foo.yaml: error: Expected to find `info` key in the root Document object but it is missing."
XCTAssertEqual(actualDiagnostic, expectedDiagnostic)
}
}
}

func testEmitsComplexOpenAPIParsingError() throws {
let collector = TestingDiagnosticsCollector()
// The `resonance` line should be `response` in a real OpenAPI document.
let yaml = """
openapi: "3.0.0"
info:
title: "Test"
version: "1.0.0"
paths:
/system:
get:
description: This is a unit test.
resonance:
'200':
description: Success
"""

XCTAssertThrowsError(try _test(yaml, diagnostics: collector)) { error in
if let exitError = error as? ExitCode {
XCTAssertEqual(exitError, ExitCode.failure)
} else {
XCTFail("Thrown error is \(type(of: error)) but should be ExitCode.failure")
}

XCTAssertEqual(collector.allOutput.count, 1)
if let actualDiagnostic = collector.allOutput.first {
let expectedDiagnostic =
"/foo.yaml: error: Expected to find `responses` key for the **GET** endpoint under `/system` but it is missing."
XCTAssertEqual(actualDiagnostic, expectedDiagnostic)
}
}
}

private func _test(_ yaml: String, diagnostics: DiagnosticCollector) throws -> ParsedOpenAPIRepresentation {
try YamsParser()
.parseOpenAPI(
.init(
absolutePath: URL(fileURLWithPath: "/foo.yaml"),
contents: Data(
"""
openapi: "\(openAPIVersionString)"
info:
title: "Test"
version: "1.0.0"
paths: {}
"""
.utf8
)
contents: Data(yaml.utf8)
),
config: .init(mode: .types),
diagnostics: PrintingDiagnosticCollector()
diagnostics: diagnostics
)
}
}

/// Collect all of the diagnostic descriptions for later assertion checks.
class TestingDiagnosticsCollector: DiagnosticCollector {
var allOutput: [String] = []

func emit(_ diagnostic: Diagnostic) {
allOutput.append(diagnostic.description)
}
}