json-schema-org / JSON-Schema-Test-Suite

A language agnostic test suite for the JSON Schema specifications
MIT License
615 stars 205 forks source link

Test for unevaluatedItems with contains #738

Closed MeastroZI closed 5 months ago

MeastroZI commented 5 months ago

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.

karenetheridge commented 5 months ago

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

MeastroZI commented 5 months ago

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 commented 5 months ago

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.

MeastroZI commented 5 months ago

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

In this given examaple contains is evaluated by the unevaluateditems.

gregsdennis commented 5 months ago

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

MeastroZI commented 5 months ago

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 commented 5 months ago

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.

MeastroZI commented 5 months ago

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 commented 5 months ago

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.