Skip to content
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

encoding/jsonschema: implement not, oneof and anyof in terms of matchN #3380

Closed
myitcv opened this issue Aug 19, 2024 · 1 comment
Closed

Comments

@myitcv
Copy link
Member

myitcv commented Aug 19, 2024

Following the landing of https://review.gerrithub.io/c/cue-lang/cue/+/1198922, we should adapt the JSON Schema encoding to use matchN for the correct semantics.

See #3165 for related discussion.

@myitcv myitcv added the FeatureRequest New feature or request label Aug 19, 2024
cueckoo pushed a commit that referenced this issue Sep 3, 2024
This CL updates JSON Schema generation to use the newly added
`matchN` primitive for `allOf`, `anyOf` and `oneOf`.

This has a much closer correlation with the JSON Schema primitives
than the current approach of using CUE's `&` and `|` operators.
Specifically:
- the schema arguments should not affect the final result other
than to validate it, but both `&` and `|` can affect the result.
- the result could become non-concrete due to `|` ambiguity in `anyOf`.

Although this does fix a bunch of issues, there are some regressions.
See issues #3418, #3420, #3422 for details.

Comparative test stats (pre-matchN before post-matchN):

```
v2:
	schema extract (pass / total): 971 / 1637 = 59.3%
	schema extract (pass / total): 975 / 1637 = 59.6%
	tests (pass / total): 3081 / 7175 = 42.9%
	tests (pass / total): 3140 / 7175 = 43.8%
	tests on extracted schemas (pass / total): 3081 / 3542 = 87.0%
	tests on extracted schemas (pass / total): 3140 / 3546 = 88.6%

v3:
	schema extract (pass / total): 971 / 1637 = 59.3%
	schema extract (pass / total): 967 / 1637 = 59.1%
	tests (pass / total): 3063 / 7175 = 42.7%
	tests (pass / total): 3074 / 7175 = 42.8%
	tests on extracted schemas (pass / total): 3063 / 3542 = 86.5%
	tests on extracted schemas (pass / total): 3074 / 3538 = 86.9%
```

This change also requires that we update the CI generated code
and remove the workaround for the previous `oneOf` limitation.

Also add a `brokenInV2` tag to the encoding/jsonschema tests to work
around the fact that some V2 tests are now broken.

For #3380
For #3165

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: I2630a6d2b1614b2479802e788c16249d2cf4aa6b
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1200526
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
@rogpeppe
Copy link
Member

Fixed by https://cuelang.org/cl/1200526.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants