Skip to content

Test for unevaluatedItems with contains #738

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

Closed
wants to merge 2 commits into from

Conversation

Vinit-Pandit
Copy link
Contributor

Based on the conclusion of the discussion in this GitHub issue, the contains keyword annotation is collected by unevaluatedItems in 2019-09 draft . Therefore, all the tests for contains in the 2020-12 draft should also apply to the 2019-09 draft, if i am not wrong here.

Additionally, since the extra items from minContains are also evaluated as shown in #293 , it would be beneficial to add tests for that as well.

@Vinit-Pandit Vinit-Pandit requested a review from a team as a code owner April 15, 2024 12:08
@karenetheridge
Copy link
Member

@Vinit-Pandit
Copy link
Contributor Author

Vinit-Pandit commented Apr 15, 2024

I don't think this is correct. See https://json-schema.org/draft/2020-12/release-notes#contains-and-unevaluateditems.

@karenetheridge I think that is outdated section If i am not wrong try that 2019-09 schema in json schema validator like this
https://json-schema.hyperjump.io/

@gregsdennis
Copy link
Member

No, Karen's correct. This behavior was introduced with 2020-12. In 2019-09 unevaluatedItems does not depend on contains. They probably pass in the online validator because you still have the 2020-12 meta-schema URI in $schema. That needs to be the 2019-09 meta-schema URI.

I also don't think the 2020-12 test you've included is necessary.

@Vinit-Pandit
Copy link
Contributor Author

Vinit-Pandit commented Apr 15, 2024

Then why this issue #292 #293 is in milestone of 2019-09 ? 🥲

In this given examaple contains is evaluated by the unevaluateditems.

@gregsdennis
Copy link
Member

gregsdennis commented Apr 15, 2024

That issue doesn't mention contains at all. And, it's closed.

@Vinit-Pandit
Copy link
Contributor Author

Vinit-Pandit commented Apr 15, 2024

This one #293

Sorry on phone !

{ "type": "array", "oneOf": [ { "contains": {"type": "string"} }, { "items": {"type": "integer"} } ], "unevaluatedItems": {"type": "boolean"} }

Instance : [true, "hello", false] is true against

@gregsdennis
Copy link
Member

That issue exists as a recognition that unevaluatedItems doesn't consider contains based on the specification text.

The behavior of this keyword depends on the annotation results of adjacent keywords that apply to the instance location being validated. Specifically, the annotations from "items" and "additionalItems", which can come from those keywords when they are adjacent to the "unevaluatedItems" keyword. Those two annotations, as well as "unevaluatedItems", can also result from any and all adjacent in-place applicator keywords. This includes but is not limited to the in-place applicators defined in this document.

contains is not listed as a dependency of unevaluatedItems.

It's a bug in the spec, but we can't edit the spec since it's already published. It was rectified with 2020-12.

Comparing the above to the 2020-12 text:

The behavior of this keyword depends on the annotation results of adjacent keywords that apply to the instance location being validated. Specifically, the annotations from "prefixItems", "items", and "contains", which can come from those keywords when they are adjacent to the "unevaluatedItems" keyword. Those three annotations, as well as "unevaluatedItems", can also result from any and all adjacent in-place applicator (Section 10.2) keywords. This includes but is not limited to the in-place applicators defined in this document.

where contains is explicitly listed, you see that it's now a dependency.

These tests aren't correct. In 2019-09, unevaluatedItems does not consider contains.

That issue can probably just be closed.


You always have to go back to the spec to determine what's required.

@Vinit-Pandit
Copy link
Contributor Author

Ok that issue and validator behviour convenced me to this conclusion , i will close this (currently on mobile)

But that issue still not understandable for me why it is even in milestone of 2019-09?

@gregsdennis
Copy link
Member

But that issue still not understandable for me why it is even in milestone of 2019-09?

I can't say, but it was opened after the spec released.

@Vinit-Pandit Vinit-Pandit deleted the main3 branch April 18, 2024 03:33
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.

3 participants