Skip to content

Commit

Permalink
Improve the diagnostic for when a schema is unsupported (#164)
Browse files Browse the repository at this point in the history
Improve the diagnostic for when a schema is unsupported

### Motivation

Previously, when a schema that was, or contained a subschema that was, unsupported, adopter would get a pretty opaque warning diagnostic telling them the "schema is unsupported", without more details of why, and which part of it.

### Modifications

This PR moves from a simple Boolean status on the `isSchemaSupported` class of utility functions, and moves to a richer enum, which includes the reason and the schema itself when unsupported, leading to much more informative diagnostics.

### Result

Diagnostics emitted for unsupported schemas will be a lot more meaningful.

### Test Plan

Adapted the unit tests to make sure all the reasons for unsupported schemas are now tested, and the reason matches.


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. 

#164
  • Loading branch information
czechboy0 authored Aug 2, 2023
1 parent c181ed7 commit 3c0fcab
Show file tree
Hide file tree
Showing 3 changed files with 192 additions and 48 deletions.
51 changes: 51 additions & 0 deletions Sources/_OpenAPIGeneratorCore/Diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,32 @@ public struct Diagnostic: Error, Codable {
context: context
)
}

/// Creates a diagnostic for an unsupported schema.
/// - Parameters:
/// - reason: A human-readable reason.
/// - schema: The unsupported JSON schema.
/// - foundIn: A description of the location in which the unsupported
/// schema was detected.
/// - location: Describe the source file that triggered the diagnostic (if known).
/// - context: A set of key-value pairs that help the user understand
/// where the warning occurred.
/// - Returns: A warning diagnostic.
public static func unsupportedSchema(
reason: String,
schema: JSONSchema,
foundIn: String,
location: Location? = nil,
context: [String: String] = [:]
) -> Diagnostic {
var context = context
context["foundIn"] = foundIn
return warning(
message: "Schema \"\(schema.prettyDescription)\" is not supported, reason: \"\(reason)\", skipping",
location: location,
context: context
)
}
}

extension Diagnostic.Severity: CustomStringConvertible {
Expand Down Expand Up @@ -174,6 +200,31 @@ extension DiagnosticCollector {
emit(Diagnostic.unsupported(feature, foundIn: foundIn, context: context))
}

/// Emits a diagnostic for an unsupported schema found in the specified
/// string location.
/// - Parameters:
/// - reason: A human-readable reason.
/// - schema: The unsupported JSON schema.
/// - foundIn: A description of the location in which the unsupported
/// 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
)
)
}

/// Emits a diagnostic for an unsupported feature found in the specified
/// type name.
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,46 @@
//===----------------------------------------------------------------------===//
import OpenAPIKit30

/// A result of checking whether a schema is supported.
enum IsSchemaSupportedResult: Equatable {

/// The schema is supported and can be generated.
case supported

/// The reason a schema is unsupported.
enum UnsupportedReason: Equatable, CustomStringConvertible {

/// Describes when no subschemas are found in an allOf, oneOf, or anyOf.
case noSubschemas

/// Describes when the schema is not object-ish, in other words isn't
/// an object, a ref, or an allOf.
case notObjectish

/// Describes when the schema is not a reference.
case notRef

/// Describes when the schema is of an unsupported schema type.
case schemaType

var description: String {
switch self {
case .noSubschemas:
return "no subschemas"
case .notObjectish:
return "not an object-ish schema (object, ref, allOf)"
case .notRef:
return "not a reference"
case .schemaType:
return "schema type"
}
}
}

/// The schema is unsupported for the provided reason.
case unsupported(reason: UnsupportedReason, schema: JSONSchema)
}

extension FileTranslator {

/// Validates that the schema is supported by the generator.
Expand All @@ -26,11 +66,17 @@ extension FileTranslator {
_ schema: JSONSchema,
foundIn: String
) throws -> Bool {
guard try isSchemaSupported(schema) else {
diagnostics.emitUnsupported("Schema", foundIn: foundIn)
switch try isSchemaSupported(schema) {
case .supported:
return true
case .unsupported(reason: let reason, schema: let schema):
diagnostics.emitUnsupportedSchema(
reason: reason.description,
schema: schema,
foundIn: foundIn
)
return false
}
return true
}

/// Validates that the schema is supported by the generator.
Expand All @@ -44,22 +90,27 @@ extension FileTranslator {
_ schema: UnresolvedSchema?,
foundIn: String
) throws -> Bool {
guard try isSchemaSupported(schema) else {
diagnostics.emitUnsupported("Schema", foundIn: foundIn)
switch try isSchemaSupported(schema) {
case .supported:
return true
case .unsupported(reason: let reason, schema: let schema):
diagnostics.emitUnsupportedSchema(
reason: reason.description,
schema: schema,
foundIn: foundIn
)
return false
}
return true
}

/// Returns a Boolean value that indicates whether the schema is supported.
/// Returns whether the schema is supported.
///
/// If a schema is not supported, no references to it should be emitted.
/// - Parameters:
/// - schema: The schema to validate.
/// - Returns: `true` if the schema is supported; `false` otherwise.
func isSchemaSupported(
_ schema: JSONSchema
) throws -> Bool {
) throws -> IsSchemaSupportedResult {
switch schema.value {
case .string,
.integer,
Expand All @@ -71,31 +122,40 @@ extension FileTranslator {
// responsible for picking only supported properties.
.object,
.fragment:
return true
return .supported
case .reference(let ref, _):
// reference is supported iff the existing type is supported
let existingSchema = try components.lookup(ref)
return try isSchemaSupported(existingSchema)
case .array(_, let array):
guard let items = array.items else {
// an array of fragments is supported
return true
return .supported
}
// an array is supported iff its element schema is supported
return try isSchemaSupported(items)
case .all(of: let schemas, _):
guard !schemas.isEmpty else {
return false
return .unsupported(
reason: .noSubschemas,
schema: schema
)
}
return try areObjectishSchemasAndSupported(schemas)
case .any(of: let schemas, _):
guard !schemas.isEmpty else {
return false
return .unsupported(
reason: .noSubschemas,
schema: schema
)
}
return try areObjectishSchemasAndSupported(schemas)
case .one(of: let schemas, let context):
guard !schemas.isEmpty else {
return false
return .unsupported(
reason: .noSubschemas,
schema: schema
)
}
// If a discriminator is provided, only refs to object/allOf of
// object schemas are allowed.
Expand All @@ -105,81 +165,104 @@ extension FileTranslator {
}
return try areRefsToObjectishSchemaAndSupported(schemas)
case .not:
return false
return .unsupported(
reason: .schemaType,
schema: schema
)
}
}

/// Returns a Boolean value that indicates whether the schema is supported.
/// Returns a result indicating whether the schema is supported.
///
/// If a schema is not supported, no references to it should be emitted.
/// - Parameters:
/// - schema: The schema to validate.
/// - Returns: `true` if the schema is supported; `false` otherwise.
func isSchemaSupported(
_ schema: UnresolvedSchema?
) throws -> Bool {
) throws -> IsSchemaSupportedResult {
guard let schema else {
// fragment type is supported
return true
return .supported
}
switch schema {
case .a:
// references are supported
return true
return .supported
case let .b(schema):
return try isSchemaSupported(schema)
}
}

/// Returns a Boolean value that indicates whether the provided schemas
/// Returns a result indicating whether the provided schemas
/// are supported.
/// - Parameter schemas: Schemas to check.
/// - Returns: `true` if all schemas are supported; `false` otherwise.
func areSchemasSupported(_ schemas: [JSONSchema]) throws -> Bool {
try schemas.allSatisfy(isSchemaSupported)
func areSchemasSupported(_ schemas: [JSONSchema]) throws -> IsSchemaSupportedResult {
for schema in schemas {
let result = try isSchemaSupported(schema)
guard result == .supported else {
return result
}
}
return .supported
}

/// Returns a Boolean value that indicates whether the provided schemas
/// Returns a result indicating whether the provided schemas
/// are reference, object, or allOf schemas and supported.
/// - Parameter schemas: Schemas to check.
/// - Returns: `true` if all schemas match; `false` otherwise.
func areObjectishSchemasAndSupported(_ schemas: [JSONSchema]) throws -> Bool {
try schemas.allSatisfy(isObjectishSchemaAndSupported)
/// - Returns: `.supported` if all schemas match; `.unsupported` otherwise.
func areObjectishSchemasAndSupported(_ schemas: [JSONSchema]) throws -> IsSchemaSupportedResult {
for schema in schemas {
let result = try isObjectishSchemaAndSupported(schema)
guard result == .supported else {
return result
}
}
return .supported
}

/// Returns a Boolean value that indicates whether the provided schema
/// Returns a result indicating whether the provided schema
/// is an reference, object, or allOf (object-ish) schema and is supported.
/// - Parameter schema: A schemas to check.
/// - Returns: `true` if the schema matches; `false` otherwise.
func isObjectishSchemaAndSupported(_ schema: JSONSchema) throws -> Bool {
func isObjectishSchemaAndSupported(_ schema: JSONSchema) throws -> IsSchemaSupportedResult {
switch schema.value {
case .object, .reference:
return try isSchemaSupported(schema)
case .all(of: let schemas, _):
return try areObjectishSchemasAndSupported(schemas)
default:
return false
return .unsupported(
reason: .notObjectish,
schema: schema
)
}
}

/// Returns a Boolean value that indicates whether the provided schemas
/// Returns a result indicating whether the provided schemas
/// are reference schemas that point to object-ish schemas and supported.
/// - Parameter schemas: Schemas to check.
/// - Returns: `true` if all schemas match; `false` otherwise.
func areRefsToObjectishSchemaAndSupported(_ schemas: [JSONSchema]) throws -> Bool {
try schemas.allSatisfy(isRefToObjectishSchemaAndSupported)
/// - Returns: `.supported` if all schemas match; `.unsupported` otherwise.
func areRefsToObjectishSchemaAndSupported(_ schemas: [JSONSchema]) throws -> IsSchemaSupportedResult {
for schema in schemas {
let result = try isRefToObjectishSchemaAndSupported(schema)
guard result == .supported else {
return result
}
}
return .supported
}

/// Returns a Boolean value that indicates whether the provided schema
/// Returns a result indicating whether the provided schema
/// is a reference schema that points to an object-ish schema and is supported.
/// - Parameter schema: A schema to check.
/// - Returns: `true` if the schema matches; `false` otherwise.
func isRefToObjectishSchemaAndSupported(_ schema: JSONSchema) throws -> Bool {
func isRefToObjectishSchemaAndSupported(_ schema: JSONSchema) throws -> IsSchemaSupportedResult {
switch schema.value {
case .reference:
return try isObjectishSchemaAndSupported(schema)
default:
return false
return .unsupported(
reason: .notRef,
schema: schema
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,23 +88,33 @@ class Test_isSchemaSupported: XCTestCase {
let translator = self.translator
for schema in Self.supportedTypes {
XCTAssertTrue(
try translator.isSchemaSupported(schema),
try translator.isSchemaSupported(schema) == .supported,
"Expected schema to be supported: \(schema)"
)
}
}

static let unsupportedTypes: [JSONSchema] = [
static let unsupportedTypes: [(JSONSchema, IsSchemaSupportedResult.UnsupportedReason)] = [
// a not
.not(.string)
(.not(.string), .schemaType),

// an allOf without any subschemas
(.all(of: []), .noSubschemas),

// an allOf with non-object-ish schemas
(.all(of: [.string, .integer]), .notObjectish),

// a oneOf with a discriminator with an inline subschema
(.one(of: .object, discriminator: .init(propertyName: "foo")), .notRef),
]
func testUnsupportedTypes() throws {
let translator = self.translator
for schema in Self.unsupportedTypes {
XCTAssertFalse(
try translator.isSchemaSupported(schema),
"Expected schema to be unsupported: \(schema)"
)
for (schema, expectedReason) in Self.unsupportedTypes {
guard case let .unsupported(reason, _) = try translator.isSchemaSupported(schema) else {
XCTFail("Expected schema to be unsupported: \(schema)")
return
}
XCTAssertEqual(reason, expectedReason)
}
}
}

0 comments on commit 3c0fcab

Please sign in to comment.