tamasfe / taplo

A TOML toolkit written in Rust
https://taplo.tamasfe.dev
MIT License
1.45k stars 114 forks source link

Discrepancy in validation with JSON Schema draft 2020-12 #497

Open uncenter opened 1 year ago

uncenter commented 1 year ago

With this schema, which defines an object with a property test that contains an array of objects that can have certain properties, I am having issues where Taplo should be invalidating a test case but it isn't. This is probably due to something with the new property introduced in this draft, unevaluatedProperties (see this discussion about it).

{
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "type": "object",
    "properties": {
        "test": {
            "type": "array",
            "items": {
                "$comment": "This schema is closed, and any property used which aren't defined are invalid/not allowed.",
                "type": "object",
                "properties": {
                    "foo": { "type": "string" }
                },
                "allOf": [{ "$ref": "#/$defs/bazExtension" }],
                "unevaluatedProperties": false
            }
        }
    },
    "$defs": {
        "bazExtension": {
            "$comment": "This schema can be used as an extension",
            "type": "object",
            "properties": {
                "baz": { "type": "boolean" }
            }
        }
    }
}

These tests should be valid, with any combination of foo and baz together or separate:

{
    "test": [
        {
            "foo": "foo",
            "baz": true
        }
    ]
}
{
    "test": [
        {
            "foo": "foo"
        }
    ]
}
{
    "test": [
        {
            "baz": true
        }
    ]
}
[[test]]
foo = "foo"
baz = true
[[test]]
foo = "foo"
[[test]]
baz = true

And these should be invalid, with an additional property not in the definition:

{
    "test": [
        {
            "foo": "foo",
            "baz": true,
            "otherProperty": true
        }
    ]
}
[[test]]
foo = "foo"
baz = true
otherProperty = true

You can test these on https://json-everything.net/json-schema/ for JSON and with taplo obviously for TOML. My issue is that Taplo isn't invalidating the last example with the additional property otherProperty when it should be.

ia0 commented 1 year ago

Thanks for the report! However, I don't think this can be fixed by Taplo. Taplo is using jsonschema for validation and I can see 2 possibly relevant open issues: https://github.com/Stranger6667/jsonschema-rs/issues/44 and https://github.com/Stranger6667/jsonschema-rs/issues/195. I assume that once those issues are fixed, we would need to update the jsonschema dependency version to benefit from them and thus fix this issue.

uncenter commented 1 year ago

Ah okay sorry. I mistakenly assumed Taplo had its own validator! I'll check those issues out.

uncenter commented 1 year ago

It looks like unevaluatedProperties should have been implemented in https://github.com/Stranger6667/jsonschema-rs/pull/419 and https://github.com/Stranger6667/jsonschema-rs/pull/423 and we are on the latest version? Hmmm... I'll open an issue with them and let you know when we can update.

ia0 commented 1 year ago

The fix is simple (see #498). However, I have no idea of the implications and why this is not automatic. Maybe it's because the draft202012 feature is not complete yet? Maybe once https://github.com/Stranger6667/jsonschema-rs/issues/195 is fixed it would be automatic.

uncenter commented 1 year ago

Huh alright. Thanks for taking a look.

salim-b commented 11 months ago

However, I have no idea of the implications and why this is not automatic. Maybe it's because the draft202012 feature is not complete yet?

Exactly this. As the main author of jsonschema-rs states here:

I think some changes could be done and merged, but we may not expose them (e.g. new keyword impls) until their respective drafts are ready. So, there should be no problem with accumulating changes step-by-step. Though, we can still test them high-level via some cfg(test) items - e.g. conditional Draft201909 enum variant.

pamburus commented 5 months ago

How about switching from jsonschema-rs to boon? It looks like it is 100% compatible with Draft 2020-12, 2019-09, Draft 7, Draft 6 and Draft 4. https://bowtie.report/#/implementations/rust-boon

salim-b commented 2 months ago

jsonschema-rs 0.19.0 just got released which enables the draft201909 and draft202012 features by default.

Draft implementation status/progress details are found here.

Stranger6667 commented 1 month ago

Folks, FYI, in jsonschema==0.21.0 only unevaluatedItems & vocabulary are missing (other non-optional tests from the JSON Schema test suite depend on unevaluatedItems). I am going to implement unevaluatedItems in the next few releases + will make Draft 2020-12 default. Thank you for your patience.

UPDATE 2024-10-24: jsonschema==0.25.0 is 100% compatible with Draft 4, 6, 7, 2019-09, and 2020-12 (bowtie builds are not yet updated)