From 2fb6f453cfb767b50ee7c6fffea668a48c71f590 Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Thu, 5 Sep 2024 13:01:19 +0200 Subject: [PATCH] internal/core/validate: fix validation of disjunctions for evalv3 The v3 evaluator represents disjunctions slightly differently from v2. The validator did not take this into account and did not dereference disjunctions when needed. Fixes #3422 Signed-off-by: Marcel van Lohuizen Change-Id: Idb47d65d9ba44ebb7d133feee33dc71dddb13314 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1200732 Reviewed-by: Roger Peppe Unity-Result: CUE porcuepine TryBot-Result: CUEcueckoo --- encoding/jsonschema/external_teststats.txt | 4 +-- .../external/tests/draft2019-09/allOf.json | 25 ++++--------------- .../external/tests/draft2019-09/oneOf.json | 20 +++------------ .../external/tests/draft2020-12/allOf.json | 25 ++++--------------- .../external/tests/draft2020-12/oneOf.json | 20 +++------------ .../testdata/external/tests/draft4/allOf.json | 25 ++++--------------- .../testdata/external/tests/draft4/oneOf.json | 20 +++------------ .../testdata/external/tests/draft6/allOf.json | 25 ++++--------------- .../testdata/external/tests/draft6/oneOf.json | 20 +++------------ .../testdata/external/tests/draft7/allOf.json | 25 ++++--------------- .../testdata/external/tests/draft7/oneOf.json | 20 +++------------ internal/core/validate/validate.go | 4 +-- internal/core/validate/validate_test.go | 17 +++++++++++++ 13 files changed, 66 insertions(+), 184 deletions(-) diff --git a/encoding/jsonschema/external_teststats.txt b/encoding/jsonschema/external_teststats.txt index e21760e9d16..a17d53f6514 100644 --- a/encoding/jsonschema/external_teststats.txt +++ b/encoding/jsonschema/external_teststats.txt @@ -6,5 +6,5 @@ v2: v3: schema extract (pass / total): 1003 / 1637 = 61.3% - tests (pass / total): 3214 / 7175 = 44.8% - tests on extracted schemas (pass / total): 3214 / 3678 = 87.4% + tests (pass / total): 3259 / 7175 = 45.4% + tests on extracted schemas (pass / total): 3259 / 3678 = 88.6% diff --git a/encoding/jsonschema/testdata/external/tests/draft2019-09/allOf.json b/encoding/jsonschema/testdata/external/tests/draft2019-09/allOf.json index fe4c995b78e..8d9ab07e278 100644 --- a/encoding/jsonschema/testdata/external/tests/draft2019-09/allOf.json +++ b/encoding/jsonschema/testdata/external/tests/draft2019-09/allOf.json @@ -40,20 +40,14 @@ "data": { "foo": "baz" }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "mismatch first", "data": { "bar": 2 }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "wrong type", @@ -124,10 +118,7 @@ "bar": 2, "baz": null }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "mismatch second allOf", @@ -135,20 +126,14 @@ "foo": "quux", "bar": 2 }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "mismatch both", "data": { "bar": 2 }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false } ] }, diff --git a/encoding/jsonschema/testdata/external/tests/draft2019-09/oneOf.json b/encoding/jsonschema/testdata/external/tests/draft2019-09/oneOf.json index 3ef71969727..9e67767d808 100644 --- a/encoding/jsonschema/testdata/external/tests/draft2019-09/oneOf.json +++ b/encoding/jsonschema/testdata/external/tests/draft2019-09/oneOf.json @@ -184,20 +184,14 @@ "data": { "bar": 2 }, - "valid": true, - "skip": { - "v3": "invalid value {bar:2} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:int},null | bool | number | string | [] | {foo!:string}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:1:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "second oneOf valid (complex)", "data": { "foo": "baz" }, - "valid": true, - "skip": { - "v3": "invalid value {foo:\"baz\"} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:int},null | bool | number | string | [] | {foo!:string}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:1:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "both oneOf valid (complex)", @@ -330,20 +324,14 @@ "data": { "bar": 8 }, - "valid": true, - "skip": { - "v3": "invalid value {bar:8} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:_,baz?:_},null | bool | number | string | [] | {foo!:_}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:1:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "second oneOf valid", "data": { "foo": "foo" }, - "valid": true, - "skip": { - "v3": "invalid value {foo:\"foo\"} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:_,baz?:_},null | bool | number | string | [] | {foo!:_}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:1:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "both oneOf valid", diff --git a/encoding/jsonschema/testdata/external/tests/draft2020-12/allOf.json b/encoding/jsonschema/testdata/external/tests/draft2020-12/allOf.json index fcdc1ba7620..7f8d90cf8f3 100644 --- a/encoding/jsonschema/testdata/external/tests/draft2020-12/allOf.json +++ b/encoding/jsonschema/testdata/external/tests/draft2020-12/allOf.json @@ -40,20 +40,14 @@ "data": { "foo": "baz" }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "mismatch first", "data": { "bar": 2 }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "wrong type", @@ -124,10 +118,7 @@ "bar": 2, "baz": null }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "mismatch second allOf", @@ -135,20 +126,14 @@ "foo": "quux", "bar": 2 }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "mismatch both", "data": { "bar": 2 }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false } ] }, diff --git a/encoding/jsonschema/testdata/external/tests/draft2020-12/oneOf.json b/encoding/jsonschema/testdata/external/tests/draft2020-12/oneOf.json index eda13ac3875..6ba3f5403f1 100644 --- a/encoding/jsonschema/testdata/external/tests/draft2020-12/oneOf.json +++ b/encoding/jsonschema/testdata/external/tests/draft2020-12/oneOf.json @@ -184,20 +184,14 @@ "data": { "bar": 2 }, - "valid": true, - "skip": { - "v3": "invalid value {bar:2} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:int},null | bool | number | string | [] | {foo!:string}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:1:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "second oneOf valid (complex)", "data": { "foo": "baz" }, - "valid": true, - "skip": { - "v3": "invalid value {foo:\"baz\"} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:int},null | bool | number | string | [] | {foo!:string}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:1:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "both oneOf valid (complex)", @@ -330,20 +324,14 @@ "data": { "bar": 8 }, - "valid": true, - "skip": { - "v3": "invalid value {bar:8} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:_,baz?:_},null | bool | number | string | [] | {foo!:_}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:1:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "second oneOf valid", "data": { "foo": "foo" }, - "valid": true, - "skip": { - "v3": "invalid value {foo:\"foo\"} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:_,baz?:_},null | bool | number | string | [] | {foo!:_}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:1:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "both oneOf valid", diff --git a/encoding/jsonschema/testdata/external/tests/draft4/allOf.json b/encoding/jsonschema/testdata/external/tests/draft4/allOf.json index 4bdeb0a478b..3c63906c066 100644 --- a/encoding/jsonschema/testdata/external/tests/draft4/allOf.json +++ b/encoding/jsonschema/testdata/external/tests/draft4/allOf.json @@ -39,20 +39,14 @@ "data": { "foo": "baz" }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "mismatch first", "data": { "bar": 2 }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "wrong type", @@ -122,10 +116,7 @@ "bar": 2, "baz": null }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "mismatch second allOf", @@ -133,20 +124,14 @@ "foo": "quux", "bar": 2 }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "mismatch both", "data": { "bar": 2 }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false } ] }, diff --git a/encoding/jsonschema/testdata/external/tests/draft4/oneOf.json b/encoding/jsonschema/testdata/external/tests/draft4/oneOf.json index 9531b105b73..0d30b50b280 100644 --- a/encoding/jsonschema/testdata/external/tests/draft4/oneOf.json +++ b/encoding/jsonschema/testdata/external/tests/draft4/oneOf.json @@ -97,20 +97,14 @@ "data": { "bar": 2 }, - "valid": true, - "skip": { - "v3": "invalid value {bar:2} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:int},null | bool | number | string | [] | {foo!:string}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "second oneOf valid (complex)", "data": { "foo": "baz" }, - "valid": true, - "skip": { - "v3": "invalid value {foo:\"baz\"} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:int},null | bool | number | string | [] | {foo!:string}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "both oneOf valid (complex)", @@ -240,20 +234,14 @@ "data": { "bar": 8 }, - "valid": true, - "skip": { - "v3": "invalid value {bar:8} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:_,baz?:_},null | bool | number | string | [] | {foo!:_}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "second oneOf valid", "data": { "foo": "foo" }, - "valid": true, - "skip": { - "v3": "invalid value {foo:\"foo\"} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:_,baz?:_},null | bool | number | string | [] | {foo!:_}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "both oneOf valid", diff --git a/encoding/jsonschema/testdata/external/tests/draft6/allOf.json b/encoding/jsonschema/testdata/external/tests/draft6/allOf.json index 209a5b98a26..9b9fc9b95c8 100644 --- a/encoding/jsonschema/testdata/external/tests/draft6/allOf.json +++ b/encoding/jsonschema/testdata/external/tests/draft6/allOf.json @@ -39,20 +39,14 @@ "data": { "foo": "baz" }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "mismatch first", "data": { "bar": 2 }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "wrong type", @@ -122,10 +116,7 @@ "bar": 2, "baz": null }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "mismatch second allOf", @@ -133,20 +124,14 @@ "foo": "quux", "bar": 2 }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "mismatch both", "data": { "bar": 2 }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false } ] }, diff --git a/encoding/jsonschema/testdata/external/tests/draft6/oneOf.json b/encoding/jsonschema/testdata/external/tests/draft6/oneOf.json index 73f96da2a3e..a0c3cee8fde 100644 --- a/encoding/jsonschema/testdata/external/tests/draft6/oneOf.json +++ b/encoding/jsonschema/testdata/external/tests/draft6/oneOf.json @@ -177,20 +177,14 @@ "data": { "bar": 2 }, - "valid": true, - "skip": { - "v3": "invalid value {bar:2} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:int},null | bool | number | string | [] | {foo!:string}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "second oneOf valid (complex)", "data": { "foo": "baz" }, - "valid": true, - "skip": { - "v3": "invalid value {foo:\"baz\"} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:int},null | bool | number | string | [] | {foo!:string}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "both oneOf valid (complex)", @@ -320,20 +314,14 @@ "data": { "bar": 8 }, - "valid": true, - "skip": { - "v3": "invalid value {bar:8} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:_,baz?:_},null | bool | number | string | [] | {foo!:_}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "second oneOf valid", "data": { "foo": "foo" }, - "valid": true, - "skip": { - "v3": "invalid value {foo:\"foo\"} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:_,baz?:_},null | bool | number | string | [] | {foo!:_}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "both oneOf valid", diff --git a/encoding/jsonschema/testdata/external/tests/draft7/allOf.json b/encoding/jsonschema/testdata/external/tests/draft7/allOf.json index 209a5b98a26..9b9fc9b95c8 100644 --- a/encoding/jsonschema/testdata/external/tests/draft7/allOf.json +++ b/encoding/jsonschema/testdata/external/tests/draft7/allOf.json @@ -39,20 +39,14 @@ "data": { "foo": "baz" }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "mismatch first", "data": { "bar": 2 }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "wrong type", @@ -122,10 +116,7 @@ "bar": 2, "baz": null }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "mismatch second allOf", @@ -133,20 +124,14 @@ "foo": "quux", "bar": 2 }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false }, { "description": "mismatch both", "data": { "bar": 2 }, - "valid": false, - "skip": { - "v3": "unexpected success" - } + "valid": false } ] }, diff --git a/encoding/jsonschema/testdata/external/tests/draft7/oneOf.json b/encoding/jsonschema/testdata/external/tests/draft7/oneOf.json index 73f96da2a3e..a0c3cee8fde 100644 --- a/encoding/jsonschema/testdata/external/tests/draft7/oneOf.json +++ b/encoding/jsonschema/testdata/external/tests/draft7/oneOf.json @@ -177,20 +177,14 @@ "data": { "bar": 2 }, - "valid": true, - "skip": { - "v3": "invalid value {bar:2} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:int},null | bool | number | string | [] | {foo!:string}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "second oneOf valid (complex)", "data": { "foo": "baz" }, - "valid": true, - "skip": { - "v3": "invalid value {foo:\"baz\"} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:int},null | bool | number | string | [] | {foo!:string}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "both oneOf valid (complex)", @@ -320,20 +314,14 @@ "data": { "bar": 8 }, - "valid": true, - "skip": { - "v3": "invalid value {bar:8} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:_,baz?:_},null | bool | number | string | [] | {foo!:_}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "second oneOf valid", "data": { "foo": "foo" }, - "valid": true, - "skip": { - "v3": "invalid value {foo:\"foo\"} (does not satisfy matchN(1, [null | bool | number | string | [] | {bar!:_,baz?:_},null | bool | number | string | [] | {foo!:_}])): 2 matched, expected 1:\n generated.cue:2:1\n generated.cue:2:8\n instance.json:1:1\n" - } + "valid": true }, { "description": "both oneOf valid", diff --git a/internal/core/validate/validate.go b/internal/core/validate/validate.go index 08ee960aa42..22fa91dd6b4 100644 --- a/internal/core/validate/validate.go +++ b/internal/core/validate/validate.go @@ -70,10 +70,10 @@ func (v *validator) add(b *adt.Bottom) { func (v *validator) validate(x *adt.Vertex) { defer v.ctx.PopArc(v.ctx.PushArc(x)) - // Dereference values, but only those that are non-rooted. This includes let + // Dereference values, but only those that are not shared. This includes let // values. This prevents us from processing structure-shared nodes more than // once and prevents potential cycles. - x = x.DerefNonRooted() + x = x.DerefNonShared() if b := x.Bottom(); b != nil { switch b.Code { case adt.CycleError: diff --git a/internal/core/validate/validate_test.go b/internal/core/validate/validate_test.go index 31d1d43330b..19430b4f57a 100644 --- a/internal/core/validate/validate_test.go +++ b/internal/core/validate/validate_test.go @@ -224,6 +224,23 @@ y: conflicting values 4 and 2: } `, out: "", + }, { + name: "indirect resolved disjunction using matchN", + cfg: &validate.Config{Final: true}, + in: ` + x: {} + x: matchN(0, [bool | {x!: _}]) + `, + out: "", + // TODO: add this test once the new evaluator correctly reports the + // error position. + // }, { + // name: "indirect resolved disjunction", + // cfg: &validate.Config{Final: true}, + // in: ` + // x: {bar: 2} + // x: string | {foo!: string} + // `, }} cuetdtest.Run(t, testCases, func(t *cuetdtest.T, tc *testCase) {