diff --git a/package-lock.json b/package-lock.json index 92b24db..a4a4483 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,6 +11,7 @@ "dependencies": { "@fluent/bundle": "^0.19.1", "@hyperjump/browser": "^1.3.1", + "@hyperjump/json-pointer": "^1.1.1", "@hyperjump/json-schema": "^1.16.0", "@hyperjump/pact": "^1.4.0", "leven": "^4.0.0" diff --git a/package.json b/package.json index 8f0987c..7de5e51 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,7 @@ "dependencies": { "@fluent/bundle": "^0.19.1", "@hyperjump/browser": "^1.3.1", + "@hyperjump/json-pointer": "^1.1.1", "@hyperjump/json-schema": "^1.16.0", "@hyperjump/pact": "^1.4.0", "leven": "^4.0.0" diff --git a/src/error-handlers/anyOf.js b/src/error-handlers/anyOf.js index 1f8a559..77244a9 100644 --- a/src/error-handlers/anyOf.js +++ b/src/error-handlers/anyOf.js @@ -1,6 +1,7 @@ import { getSchema } from "@hyperjump/json-schema/experimental"; -import * as Schema from "@hyperjump/browser"; import * as Instance from "@hyperjump/json-schema/instance/experimental"; +import * as Schema from "@hyperjump/browser"; +import * as JsonPointer from "@hyperjump/json-pointer"; import { getErrors } from "../error-handling.js"; /** @@ -8,24 +9,30 @@ import { getErrors } from "../error-handling.js"; */ /** @type ErrorHandler */ -const anyOf = async (normalizedErrors, instance, localization) => { +const anyOfErrorHandler = async (normalizedErrors, instance, localization) => { /** @type ErrorObject[] */ const errors = []; if (normalizedErrors["https://json-schema.org/keyword/anyOf"]) { for (const schemaLocation in normalizedErrors["https://json-schema.org/keyword/anyOf"]) { + const allAlternatives = normalizedErrors["https://json-schema.org/keyword/anyOf"][schemaLocation]; + if (typeof allAlternatives === "boolean") { + continue; + } + /** @type NormalizedOutput[] */ const alternatives = []; - const allAlternatives = /** @type NormalizedOutput[] */ (normalizedErrors["https://json-schema.org/keyword/anyOf"][schemaLocation]); for (const alternative of allAlternatives) { - if (Object.values(alternative[Instance.uri(instance)]["https://json-schema.org/keyword/type"]).every((valid) => valid)) { + if (Object.values(alternative[Instance.uri(instance)]["https://json-schema.org/keyword/type"] ?? {}).every((valid) => valid)) { alternatives.push(alternative); } } - // case 1 where no. alternative matched the type of the instance. + + // No alternative matched the type of the instance. if (alternatives.length === 0) { /** @type Set */ const expectedTypes = new Set(); + for (const alternative of allAlternatives) { for (const instanceLocation in alternative) { if (instanceLocation === Instance.uri(instance)) { @@ -37,28 +44,114 @@ const anyOf = async (normalizedErrors, instance, localization) => { } } } + errors.push({ message: localization.getTypeErrorMessage([...expectedTypes], Instance.typeOf(instance)), instanceLocation: Instance.uri(instance), schemaLocation: schemaLocation }); - } else if (alternatives.length === 1) { // case 2 when only one type match - return getErrors(alternatives[0], instance, localization); - } else if (instance.type === "object") { - let targetAlternativeIndex = -1; - for (const alternative of alternatives) { - targetAlternativeIndex++; + continue; + } + + // Only one alternative matches the type of the instance + if (alternatives.length === 1) { + errors.push(...await getErrors(alternatives[0], instance, localization)); + continue; + } + + if (instance.type === "object") { + const definedProperties = allAlternatives.map((alternative) => { + /** @type Set */ + const alternativeProperties = new Set(); + for (const instanceLocation in alternative) { - if (instanceLocation !== "#") { - return getErrors(alternatives[targetAlternativeIndex], instance, localization); + const pointer = instanceLocation.slice(Instance.uri(instance).length + 1); + if (pointer.length > 0) { + const position = pointer.indexOf("/"); + const propertyName = pointer.slice(0, position === -1 ? undefined : position); + const location = JsonPointer.append(propertyName, Instance.uri(instance)); + alternativeProperties.add(location); } } + + return alternativeProperties; + }); + + const discriminator = definedProperties.reduce((acc, properties) => { + return acc.intersection(properties); + }, definedProperties[0]); + + const discriminatedAlternatives = alternatives.filter((alternative) => { + for (const instanceLocation in alternative) { + if (!discriminator.has(instanceLocation)) { + continue; + } + + let valid = true; + for (const keyword in alternative[instanceLocation]) { + for (const schemaLocation in alternative[instanceLocation][keyword]) { + if (alternative[instanceLocation][keyword][schemaLocation] !== true) { + valid = false; + break; + } + } + } + if (valid) { + return true; + } + } + return false; + }); + + // Discriminator match + if (discriminatedAlternatives.length === 1) { + errors.push(...await getErrors(discriminatedAlternatives[0], instance, localization)); + continue; } + + // Discriminator identified, but none of the alternatives match + if (discriminatedAlternatives.length === 0) { + // TODO: How do we handle this case? + } + + // Last resort, select the alternative with the most properties matching the instance + // TODO: We shouldn't use this strategy if alternatives have the same number of matching instances + const instanceProperties = new Set(Instance.values(instance) + .map((node) => Instance.uri(node))); + let maxMatches = -1; + let selectedIndex = 0; + let index = -1; + for (const alternativeProperties of definedProperties) { + index++; + const matches = alternativeProperties.intersection(instanceProperties).size; + if (matches > maxMatches) { + selectedIndex = index; + } + } + + errors.push(...await getErrors(alternatives[selectedIndex], instance, localization)); + continue; } + + // TODO: Handle alternatives with const + // TODO: Handle alternatives with enum + // TODO: Handle null alternatives + // TODO: Handle boolean alternatives + // TODO: Handle string alternatives + // TODO: Handle array alternatives + // TODO: Handle alternatives without a type + + // TODO: If we get here, we don't know what else to do and give a very generic message + // Ideally this should be replace by something that can handle whatever case is missing. + errors.push({ + message: localization.getAnyOfErrorMessage(), + instanceLocation: Instance.uri(instance), + schemaLocation: schemaLocation + }); } } return errors; }; -export default anyOf; +export default anyOfErrorHandler; diff --git a/src/keyword-error-message.test.js b/src/keyword-error-message.test.js index 9a30f5d..c79a0b6 100644 --- a/src/keyword-error-message.test.js +++ b/src/keyword-error-message.test.js @@ -961,7 +961,7 @@ describe("Error messages", async () => { ]); }); - test.skip("anyOf - const-based discriminator mismatch", async () => { + test.skip("anyOf - discriminator with no matches", async () => { registerSchema({ $schema: "https://json-schema.org/draft/2020-12/schema", anyOf: [ @@ -1022,6 +1022,65 @@ describe("Error messages", async () => { ]); }); + test("anyOf - discriminator with one match", async () => { + registerSchema({ + $schema: "https://json-schema.org/draft/2020-12/schema", + anyOf: [ + { + type: "object", + properties: { + type: { const: "a" }, + apple: { type: "string" } + }, + required: ["type"] + }, + { + type: "object", + properties: { + type: { const: "b" }, + banana: { type: "string" } + }, + required: ["type"] + } + ] + }, schemaUri); + + const instance = { + type: "a", + apple: 42, + banana: "yellow" + }; + + /** @type OutputFormat */ + const output = { + valid: false, + errors: [ + { + absoluteKeywordLocation: `https://example.com/main#/anyOf/0/properties/apple/type`, + instanceLocation: "#/apple" + }, + { + absoluteKeywordLocation: `https://example.com/main#/anyOf/1/properties/type/const`, + instanceLocation: "#/type" + }, + { + absoluteKeywordLocation: `https://example.com/main#/anyOf`, + instanceLocation: "#" + } + ] + }; + + const result = await betterJsonSchemaErrors(output, schemaUri, instance); + + expect(result.errors).to.eql([ + { + schemaLocation: `https://example.com/main#/anyOf/0/properties/apple/type`, + instanceLocation: "#/apple", + message: localization.getTypeErrorMessage("string", "number") + } + ]); + }); + test("anyOf - using $ref in alternatives", async () => { const subjectUri = "https://example.com/main"; diff --git a/src/localization.js b/src/localization.js index 746d635..9bad051 100644 --- a/src/localization.js +++ b/src/localization.js @@ -259,4 +259,9 @@ export class Localization { return this._formatMessage("enum-error", formattedArgs); } + + /** @type () => string */ + getAnyOfErrorMessage() { + return this._formatMessage("anyOf-error"); + } } diff --git a/src/normalization-handlers/properties.js b/src/normalization-handlers/properties.js index 8adcc09..20af8f2 100644 --- a/src/normalization-handlers/properties.js +++ b/src/normalization-handlers/properties.js @@ -1,5 +1,6 @@ -import { evaluateSchema } from "../normalized-output.js"; import * as Instance from "@hyperjump/json-schema/instance/experimental"; +import * as JsonPointer from "@hyperjump/json-pointer"; +import { evaluateSchema } from "../normalized-output.js"; /** * @import { KeywordHandler, NormalizedOutput } from "../index.d.ts" @@ -15,10 +16,13 @@ const properties = { for (const propertyName in properties) { const propertyNode = Instance.step(propertyName, instance); if (!propertyNode) { - continue; + errors.push({ + [JsonPointer.append(propertyName, Instance.uri(instance))]: {} + }); + } else { + errors.push(evaluateSchema(properties[propertyName], propertyNode, context)); + context.evaluatedProperties?.add(propertyName); } - errors.push(evaluateSchema(properties[propertyName], propertyNode, context)); - context.evaluatedProperties?.add(propertyName); } return errors; diff --git a/src/normalized-output.js b/src/normalized-output.js index 01afaa8..ca2c9e1 100644 --- a/src/normalized-output.js +++ b/src/normalized-output.js @@ -85,8 +85,8 @@ export const evaluateSchema = (schemaLocation, instance, context) => { /** @type (a: API.NormalizedOutput, b: API.NormalizedOutput) => void */ const mergeOutput = (a, b) => { for (const instanceLocation in b) { + a[instanceLocation] ??= {}; for (const keywordUri in b[instanceLocation]) { - a[instanceLocation] ??= {}; a[instanceLocation][keywordUri] ??= {}; Object.assign(a[instanceLocation][keywordUri], b[instanceLocation][keywordUri]); diff --git a/src/normalized-output.test.js b/src/normalized-output.test.js index 7ca583b..1dade8b 100644 --- a/src/normalized-output.test.js +++ b/src/normalized-output.test.js @@ -138,6 +138,7 @@ describe("Error Output Normalization", async () => { "https://example.com/main#/type": true } }, + "#/name": {}, "#/age": { "https://json-schema.org/keyword/type": { "https://example.com/main#/properties/age/type": false @@ -213,7 +214,8 @@ describe("Error Output Normalization", async () => { "https://json-schema.org/keyword/type": { "https://example.com/main#/$defs/profile/properties/name/type": false } - } + }, + "#/profile/age": {} }); }); @@ -313,7 +315,8 @@ describe("Error Output Normalization", async () => { "https://json-schema.org/keyword/type": { "https://example.com/main#/$defs/profile/properties/name/type": false } - } + }, + "#/profile/age": {} }); }); diff --git a/src/translations/en-US.ftl b/src/translations/en-US.ftl index 057d26b..3545455 100644 --- a/src/translations/en-US.ftl +++ b/src/translations/en-US.ftl @@ -1,32 +1,38 @@ +# Non-type specific messages type-error = The instance should be of type {$expected} but found {$actual}. +const-error = The instance should be equal to {$expectedValue}. +enum-error = { $variant -> + [suggestion] Unexpected value {$instanceValue}. Did you mean {$suggestion}? + *[fallback] Unexpected value {$instanceValue}. Expected one of: {$allowedValues}. +} +# String messages string-error = Expected a string {$constraints}. string-error-minLength = at least {$minLength} characters long string-error-maxLength = at most {$maxLength} characters long +pattern-error = The instance should match the pattern: {$pattern}. +format-error = The instance should match the format: {$format}. +# Number messages number-error = Expected a number {$constraints}. number-error-minimum = greater than {$minimum} number-error-exclusive-minimum = greater than or equal to {$minimum} number-error-maximum = less than {$maximum} number-error-exclusive-maximum = less than or equal to {$maximum} - -required-error = This instance is missing required property(s): {$missingProperties}. multiple-of-error = The instance should be a multiple of {$divisor}. +# Object messages properties-error = Expected object to have {$constraints} properties-error-max = at most {$maxProperties} properties. properties-error-min = at least {$minProperties} properties. +required-error = This instance is missing required property(s): {$missingProperties}. +additional-properties-error = The property "{$propertyName}" is not allowed. -const-error = The instance should be equal to {$expectedValue}. - +# Array messages array-error = Expected the array to have {$constraints}. array-error-min = at least {$minItems} items array-error-max = at most {$maxItems} items - unique-items-error = The instance should have unique items in the array. -format-error = The instance should match the format: {$format}. -pattern-error = The instance should match the pattern: {$pattern}. - contains-error-min = The array must contain at least {$minContains -> [one] item that passes *[other] items that pass @@ -36,9 +42,6 @@ contains-error-min-max = The array must contain at least {$minContains} and at m *[other] items that pass } the 'contains' schema. +# Conditional messages +anyOf-error = The instance must pass at least one of the given schemas. not-error = The instance is not allowed to be used in this schema. -additional-properties-error = The property "{$propertyName}" is not allowed. -enum-error = { $variant -> - [suggestion] Unexpected value {$instanceValue}. Did you mean {$suggestion}? - *[fallback] Unexpected value {$instanceValue}. Expected one of: {$allowedValues}. -}