-
-
Notifications
You must be signed in to change notification settings - Fork 216
Test for unevaluatedProperties peering inside an allOf #310
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.
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
Comments
Awesome, looks reasonable certainly -- we don't yet have any unevaluatedProperties tests :/ but yeah definitely looks worth adding. |
added unevaluatedProperties test file; resolves #310
Step 4.i.a. certainly passes, but I disagree on the reason. I thought |
@ssilverman You're correct. In the provided schema, the It's a tricky one to reason about because |
@ssilverman yeah in this case, there aren't any other properties defined in the same schema object as |
This question was brought up in the Slack workspace.
(The schema was originally in YAML, but I've translated to JSON for the issue.)
Given a schema
should the instance
pass validation?
The thing to note here is that
additionalProperties
, regardless of the subschema that's in it, visits all of the properties of any object instance that's given to it. This means thatunevaluatedProperties
is effectively a no-op.The evaluation proceeds as follows:
/unevaluatedProperties
must be run last, so we save it for later./allOf/properties/foo
isn't found. Pass ✔️/allOf/properties/bar
is found, and it's a string. Pass ✔️/additionalProperties
evaluates all properties not defined byproperties
as:/additionalProperties/not/enum
/bar
was evaluated byproperties
skip. Pass ✔️/bob
is notnull
. Pass ✔️/unevaluatedProperties
/bar
and instance/bob
were both evaluated within theallOf
sounevaluatedProperties
has nothing to do. Pass ✔️All paths pass validation.
I'll write a test for this and create a PR. Just wanted to get an issue going for it.
The text was updated successfully, but these errors were encountered: