Skip to content

Commit

Permalink
encoding/jsonschema: do not generate defaults
Browse files Browse the repository at this point in the history
The `default` keyword in JSON Schema should be an annotation,
not a constraint, and in general schemas in CUE should be pure,
so having the defaults in there seems wrong, especially as:

- it precludes people from adding their own defaults
- defaults don't work when they're inside `matchN` calls
- defaults will not work as expected inside optional or required fields,
which are the only kind of fields that encoding/jsonschema produces.

In the future, we might wish to provide template-like values
that do include defaults (that's issue #3473), but for now, this keeps things simple
and avoids issues with collisions with defaults in user code.

So we remove support for `default`, and also remove the unused code
for `examples`: we can add it back later in the same or another form
as desired.

Fixes #3472

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I544734d3361c7b2eff410dae8f27d831ec8c0e97
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1207146
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
  • Loading branch information
rogpeppe committed Jan 14, 2025
1 parent 09c9cca commit fdf92d1
Show file tree
Hide file tree
Showing 9 changed files with 12 additions and 59 deletions.
13 changes: 2 additions & 11 deletions encoding/jsonschema/constraints_generic.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,8 @@ func constraintConst(key string, n cue.Value, s *state) {
}

func constraintDefault(key string, n cue.Value, s *state) {
sc := *s
s.default_ = sc.value(n)
// TODO: must validate that the default is subsumed by the normal value,
// as CUE will otherwise broaden the accepted values with the default.
s.examples = append(s.examples, s.default_)
// TODO make the default value available in a separate
// template-like CUE value outside of the usual schema output.
}

func constraintDeprecated(key string, n cue.Value, s *state) {
Expand Down Expand Up @@ -90,12 +87,6 @@ func constraintExamples(key string, n cue.Value, s *state) {
if n.Kind() != cue.ListKind {
s.errf(n, `value of "examples" must be an array, found %v`, n.Kind())
}
// TODO: implement examples properly.
// for _, n := range s.listItems("examples", n, true) {
// if ex := s.value(n); !isAny(ex) {
// s.examples = append(s.examples, ex)
// }
// }
}

func constraintNullable(key string, n cue.Value, s *state) {
Expand Down
15 changes: 0 additions & 15 deletions encoding/jsonschema/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,8 +451,6 @@ type state struct {
all constraintInfo // values and oneOf etc.
nullable *ast.BasicLit // nullable

default_ ast.Expr
examples []ast.Expr
exclusiveMin bool // For OpenAPI and legacy support.
exclusiveMax bool // For OpenAPI and legacy support.

Expand Down Expand Up @@ -680,19 +678,6 @@ func (s *state) finalize() (e ast.Expr) {
a = []ast.Expr{s.nullable, e}
}

outer:
switch {
case s.default_ != nil:
// check conditions where default can be skipped.
switch x := s.default_.(type) {
case *ast.ListLit:
if s.allowedTypes == cue.ListKind && len(x.Elts) == 0 {
break outer
}
}
a = append(a, &ast.UnaryExpr{Op: token.MUL, X: s.default_})
}

e = ast.NewBinExpr(token.OR, a...)

if len(s.definitions) > 0 {
Expand Down
8 changes: 4 additions & 4 deletions encoding/jsonschema/external_teststats.txt
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
# Generated by CUE_UPDATE=1 go test. DO NOT EDIT
v2:
schema extract (pass / total): 1054 / 1363 = 77.3%
tests (pass / total): 3788 / 4803 = 78.9%
tests on extracted schemas (pass / total): 3788 / 3955 = 95.8%
tests (pass / total): 3793 / 4803 = 79.0%
tests on extracted schemas (pass / total): 3793 / 3955 = 95.9%

v3:
schema extract (pass / total): 1054 / 1363 = 77.3%
tests (pass / total): 3778 / 4803 = 78.7%
tests on extracted schemas (pass / total): 3778 / 3955 = 95.5%
tests (pass / total): 3783 / 4803 = 78.8%
tests on extracted schemas (pass / total): 3783 / 3955 = 95.7%

Optional tests

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,7 @@
"data": {
"alpha": 5
},
"valid": false,
"skip": {
"v2": "unexpected success",
"v3": "unexpected success"
}
"valid": false
},
{
"description": "missing properties are not filled in with the default",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,7 @@
"data": {
"alpha": 5
},
"valid": false,
"skip": {
"v2": "unexpected success",
"v3": "unexpected success"
}
"valid": false
},
{
"description": "missing properties are not filled in with the default",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,7 @@
"data": {
"alpha": 5
},
"valid": false,
"skip": {
"v2": "unexpected success",
"v3": "unexpected success"
}
"valid": false
},
{
"description": "missing properties are not filled in with the default",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,7 @@
"data": {
"alpha": 5
},
"valid": false,
"skip": {
"v2": "unexpected success",
"v3": "unexpected success"
}
"valid": false
},
{
"description": "missing properties are not filled in with the default",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,7 @@
"data": {
"alpha": 5
},
"valid": false,
"skip": {
"v2": "unexpected success",
"v3": "unexpected success"
}
"valid": false
},
{
"description": "missing properties are not filled in with the default",
Expand Down
5 changes: 1 addition & 4 deletions encoding/jsonschema/testdata/txtar/type.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ close({
intString?: null | bool | int | string | [...]
object?: {
...
} | *{
foo: "bar"
baz: 1.3
}
numOrList?: matchN(1, [number, [...number]]) | *[1, 2, 3]
numOrList?: matchN(1, [number, [...number]])
})

0 comments on commit fdf92d1

Please sign in to comment.