Skip to content

fix type error handler to handle allOf conflicting type #101

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.

Sign up for GitHub

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

Open
srivastava-diya wants to merge 3 commits into
base: main
Choose a base branch
from
+171 −6
Open

fix type error handler to handle allOf conflicting type #101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9755206
fixed type error handler to handle allOf
srivastava-diya Dec 1, 2025
330c783
Add more tests for schemas with mulitple types
jdesrosiers Dec 2, 2025
c79d9cd
refactor code in type handler
srivastava-diya Dec 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 52 additions & 4 deletions src/error-handlers/type.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,73 @@ import * as Instance from "@hyperjump/json-schema/instance/experimental";
* @import { ErrorHandler, ErrorObject } from "../index.d.ts"
*/

const ALL_TYPES = ["null", "boolean", "number", "string", "array", "object", "integer"];

/** @type ErrorHandler */
const type = async (normalizedErrors, instance, localization) => {
/** @type ErrorObject[] */
const errors = [];

if (normalizedErrors["https://json-schema.org/keyword/type"]) {
let allowedTypes = new Set(ALL_TYPES);
Copy link
Collaborator

@jdesrosiers jdesrosiers Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no reason to create a new set every time you initialize allowedTypes. ALL_TYPES can be a set.

const failedTypeLocations = [];

for (const schemaLocation in normalizedErrors["https://json-schema.org/keyword/type"]) {
if (!normalizedErrors["https://json-schema.org/keyword/type"][schemaLocation]) {
const keyword = await getSchema(schemaLocation);
const isValid = normalizedErrors["https://json-schema.org/keyword/type"][schemaLocation];
if (!isValid) {
failedTypeLocations.push(schemaLocation);
}

const keyword = await getSchema(schemaLocation);
/** @type {string|string[]} */
const value = Schema.value(keyword);
const types = Array.isArray(value) ? value : [value];
/** @type {Set<string>} */
const keywordTypes = new Set(types);
allowedTypes = intersectTypeSets(allowedTypes, keywordTypes);
Comment on lines +31 to +32
Copy link
Collaborator

@jdesrosiers jdesrosiers Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need to write custom set intersection logic. Conceptually, if a schema allows numbers, it also allows integers. If you code for that relationship, the rest should all just work.

Suggested change
const keywordTypes = new Set(types);
allowedTypes = intersectTypeSets(allowedTypes, keywordTypes);
const keywordTypes = new Set(types);
if (keywordTypes.has("number")) {
keywordTypes.add("integer");
}
allowedTypes = allowedTypes.intersection(keywordTypes);

}

if (allowedTypes.size === 0) {
if (failedTypeLocations.length > 0) {
Copy link
Collaborator

@jdesrosiers jdesrosiers Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's impossible for failedTypeLocations to be empty if allowedTypes is empty, right? If allowedTypes is empty, there must be at least two occurrences of type that don't overlap. That means one must fail.

Copy link
Contributor Author

@srivastava-diya srivastava-diya Dec 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are absolutely right here. One must fail. I will refine that .

Regarding integer vs number, I wasn’t fully clear on how to treat them either. I saw the patternProperties test using type:"integer", so I included "integer" in the initial allowedTypes set, . I initially did not include integer in the set which caused this particulr test to fail so i included it,but I now understand the problem: "integer" isn’t a standalone JSON type, and it has a subtype relationship with "number" .I also saw your note about needing to eliminate duplicate type messages so I'll try to incorporate that .

Copy link
Collaborator

@jdesrosiers jdesrosiers Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you forget to remove this if, or did you come up with a reason why it needs to be there?

errors.push({
message: localization.getTypeErrorMessage(Schema.value(keyword), Instance.typeOf(instance)),
message: localization.getConflictingTypeMessage(),
instanceLocation: Instance.uri(instance),
schemaLocation: schemaLocation
schemaLocation: failedTypeLocations
});
}
} else if (failedTypeLocations.length > 0) {
if (allowedTypes.has("number")) {
allowedTypes.delete("integer");
}
Comment on lines +44 to +46
Copy link
Collaborator

@jdesrosiers jdesrosiers Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a great place to put this code. It would be more cohesive to put it after the for loop that builds allowedTypes. That way all the code that builds allowedTypes is in one place. Then all the code that reads allowedTypes comes after.

errors.push({
message: localization.getTypeErrorMessage([...allowedTypes], Instance.typeOf(instance)),
instanceLocation: Instance.uri(instance),
schemaLocation: failedTypeLocations.length === 1 ? failedTypeLocations[0] : failedTypeLocations
});
}
}

return errors;
};

/**
* @param {Set<string>} a
* @param {Set<string>} b
* @returns {Set<string>}
*/
const intersectTypeSets = (a, b) => {
/** @type {Set<string>} */
const intersection = new Set();
for (const type of a) {
if (b.has(type)) {
intersection.add(type);
} else if (type === "integer" && b.has("number")) {
intersection.add("integer");
} else if (type === "number" && b.has("integer")) {
intersection.add("integer");
}
}
return intersection;
};

export default type;
121 changes: 119 additions & 2 deletions src/keyword-error-message.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1719,7 +1719,7 @@ describe("Error messages", async () => {
]);
});

test.skip("allOf conflicting type", async () => {
test("allOf conflicting type", async () => {
registerSchema({
$schema: "https://json-schema.org/draft/2020-12/schema",
allOf: [
Expand Down Expand Up @@ -1749,13 +1749,130 @@ describe("Error messages", async () => {

expect(result.errors).to.eql([
{
schemaLocation: "https://example.com/main#/allOf/0/type",
schemaLocation: [
"https://example.com/main#/allOf/0/type",
"https://example.com/main#/allOf/1/type"
],
instanceLocation: "#",
message: localization.getConflictingTypeMessage()
}
]);
});

test("can be a number and integer at the same time", async () => {
registerSchema({
$schema: "https://json-schema.org/draft/2020-12/schema",
allOf: [
{ type: "number" },
{ type: "integer" }
]
}, schemaUri);

const instance = "foo";

/** @type OutputFormat */
const output = {
valid: false,
errors: [
{
absoluteKeywordLocation: "https://example.com/main#/allOf/0/type",
instanceLocation: "#"
},
{
absoluteKeywordLocation: "https://example.com/main#/allOf/1/type",
instanceLocation: "#"
}
]
};

const result = await betterJsonSchemaErrors(output, schemaUri, instance);

expect(result.errors).to.eql([
{
message: localization.getTypeErrorMessage("integer", "string"),
instanceLocation: "#",
schemaLocation: [
"https://example.com/main#/allOf/0/type",
"https://example.com/main#/allOf/1/type"
]
}
]);
});

test("can be a number and integer at the same time - pass", async () => {
registerSchema({
$schema: "https://json-schema.org/draft/2020-12/schema",
allOf: [
{ type: "number" },
{ type: "integer" }
],
maximum: 5
}, schemaUri);

const instance = 15;

/** @type OutputFormat */
const output = {
valid: false,
errors: [
{
absoluteKeywordLocation: "https://example.com/main#/maximum",
instanceLocation: "#"
}
]
};

const result = await betterJsonSchemaErrors(output, schemaUri, instance);

expect(result.errors).to.eql([
{
message: localization.getNumberErrorMessage({ maximum: 5 }),
instanceLocation: "#",
schemaLocation: "https://example.com/main#/maximum"
}
]);
});

test("there should be one type message per schema", async () => {
registerSchema({
$schema: "https://json-schema.org/draft/2020-12/schema",
allOf: [
{ type: "number" },
{ type: "number" }
]
}, schemaUri);

const instance = "foo";

/** @type OutputFormat */
const output = {
valid: false,
errors: [
{
absoluteKeywordLocation: "https://example.com/main#/allOf/0/type",
instanceLocation: "#"
},
{
absoluteKeywordLocation: "https://example.com/main#/allOf/1/type",
instanceLocation: "#"
}
]
};

const result = await betterJsonSchemaErrors(output, schemaUri, instance);

expect(result.errors).to.eql([
{
message: localization.getTypeErrorMessage(["number"], "string"),
instanceLocation: "#",
schemaLocation: [
"https://example.com/main#/allOf/0/type",
"https://example.com/main#/allOf/1/type"
]
}
]);
});

test("normalized output for a failing 'contains' keyword", async () => {
registerSchema({
$schema: "https://json-schema.org/draft/2020-12/schema",
Expand Down