Skip to content

Trimming alternatives for anyOf/oneOf based on type#170

Open
srivastava-diya wants to merge 3 commits intohyperjump-io:mainfrom
srivastava-diya:main
Open

Trimming alternatives for anyOf/oneOf based on type#170
srivastava-diya wants to merge 3 commits intohyperjump-io:mainfrom
srivastava-diya:main

Conversation

@srivastava-diya
Copy link
Contributor

Description

This PR improves the error reporting for anyOf and oneOf keywords by filtering out irrelevant schema alternatives based on the instance's type. This results in significantly cleaner error messages.

Fixes : #165

Changes

  1. The error handlers now check the type keyword of each alternative. If an alternative expects a type different from the instance it is excluded from the error report.

  2. If filtering results in only one alternative, then anyOf / oneOf wrapper error is removed entirely.

  3. If no alternatives match the instance type (or if types aren't specified), the original behavior is preserved all alternatives are reported.

Comment on lines +27 to +30
const anyOfSchema = await getSchema(schemaLocation);
const alternativeSchema = await Schema.step(String(i), anyOfSchema);
const typeSchema = await Schema.step("type", alternativeSchema);
const type = /** @type {string | string[]} */ (Schema.value(typeSchema));
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will only work in simple schemas. Consider an alternative schema that is just a $ref to another schema that provides defines type. You can use normalizedErrors instead to get the type information.

It occurs to me that we might have some duplication with the type error handler. Let's try to consider how to avoid that duplication.

Comment on lines +130 to +143
"anyOf": [
{
"type": "string",
"maxLength": 2
},
{
"type": "string",
"maxLength": 4
},
{
"type": "number",
"minimum": 2
}
]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an interesting case because this could be collapsed into a single message: "Expected a string with no more than 4 characters". Let's change this test to something that can't be collapsed so the test doesn't break if we find a way to collapse these kinds of messages in the future.

Also, please open an issue with this case so we can discuss and think about how we might handle cases like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Discriminate between instance alternatives based on type

2 participants

Comments