Combine const and enum errors into a single message#155
Combine const and enum errors into a single message#155ShivaGupta-14 wants to merge 1 commit intohyperjump-io:mainfrom
const and enum errors into a single message#155Conversation
7646a94 to
ac73647
Compare
ac73647 to
3b4eaf2
Compare
Hi @jdesrosiers, thanks for pointing out the issue! I understood the problem: Issue: with a schema like above one ->
const passed = normalizedErrors[...][schemaLocation] === true;
if (!passed) hasFailure = true;
// always collect
constraints.push({ allowedValues: [...], schemaLocation });now:
Tests Added:
All tests pass. Is this approach good, or does it need further improvements? |
jdesrosiers
left a comment
There was a problem hiding this comment.
That should work. Just a couple small things I'd like to see.
e7b9dec to
219a62a
Compare
219a62a to
5a31867
Compare
jdesrosiers
left a comment
There was a problem hiding this comment.
The logic to find the most constraining keyword felt off to me and I figured out a case where it doesn't work. Try,
{
"allOf": [
{ "enum": ["a", "b", "c"] },
{ "enum": ["a", "b", "d"] },
{ "enum": ["a", "b", "e"] }
]
}with,
"c"It says, "Expected one of "a" or "b"" with schema location /allOf/0/enum, but that's a passing location. If we exclude passing locations, we get a schema location of /allOf/1/enum, which is better, but it seems contradictory because the message says only "a" and "b" are allowed, but the schema location says that "d" is also allowed. That means we need to include all the failing schema locations to make it clear why only "a" and "b" are allowed. I think the only time we can collapse to a single most constraining location is when the effective allowed set matches exactly one of the keywords.
This is a tough one.
|
Another edge case to consider, {
"allOf": [
{ "enum": ["a", "b"] },
{ "enum": ["a", "b"] }
]
}with, "c"Should one location be selected as the most constraining, or should we return both locations? I think either one is fine. |
5a31867 to
0a80648
Compare
Thanks for pointing it out, I missed it. for such cases, I’ve updated the logic.
this will handle edge case: when intersection is ["a","b"] but no single constraint has exactly ["a","b"], we show all locations so users understand why only "a" and "b" are valid. also, test added for this case: "multiple enums with no exact match" |
return one since both are exactly the same in such cases. also, duplicate identical enums in real schemas are likely unintentional, so returning one seems sufficient. |
Description
When multiple
constorenumconstraints fail (e.g., viaallOf), produce a single message with the most restrictive constraint instead of multiple redundant messages.enummessageChecklist