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.

Sign up for GitHub

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

Jump to bottom

evalv3: unexpected failure with explicit _|_ in matchN arguments #3420

Open
rogpeppe opened this issue Sep 2, 2024 · 2 comments
Open

evalv3: unexpected failure with explicit _|_ in matchN arguments #3420

rogpeppe opened this issue Sep 2, 2024 · 2 comments
Labels
encoding jsonschema / openapi NeedsFix

Comments

@rogpeppe
Copy link
Member

rogpeppe commented Sep 2, 2024

What version of CUE are you using (cue version)?

$ cue version
1440b9eae7fa9ea40133418df0e5bacdcca29cb4

Does this issue reproduce with the latest stable release?

N/A

What did you do?

env CUE_EXPERIMENT=evalv3
exec cue vet schema.cue

-- schema.cue --
matchN(1, [_, _|_])

What did you expect to see?

Passing test (and under evalv2, this does indeed pass).

What did you see instead?

> env CUE_EXPERIMENT=evalv3
> exec cue vet schema.cue
[stderr]
explicit error (_|_ literal) in source:
    ./schema.cue:1:15
[exit status 1]
FAIL: /tmp/x.txtar:2: unexpected command failure
@mvdan
Copy link
Member

mvdan commented Sep 3, 2024

Why are you writing a bottom like that in your CUE? How did this come about from jsonschema?

cueckoo pushed a commit that referenced this issue Sep 3, 2024
@rogpeppe
encoding/jsonschema: encode oneOf etc using matchN
bfce150 
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 <rogpeppe@gmail.com>
Change-Id: I2630a6d2b1614b2479802e788c16249d2cf4aa6b
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1200526
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
@rogpeppe
Copy link
Member Author

rogpeppe commented Sep 5, 2024

Why are you writing a bottom like that in your CUE? How did this come about from jsonschema?

In JSON Schema, the value false is valid as a schema that never validates. The most natural translation of that into CUE is as bottom.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
encoding jsonschema / openapi NeedsFix
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@rogpeppe @mvdan