cue: fail if non-optional definitions are bottom
This puts the implemenation on par with the spec.
The spec has been made slightly clearer to inidicate
that a non-optional failed definition is also a failure,
just like regular fields.
The code is copied from the previously used implementation
of walk. The old implementation just walked over
concrete values, so we needed another one to visit
all fields.
Change-Id: Ia20f83b7c1ba6299cb7372b018cf38d57038290b
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/3769
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cmd/cue/cmd/testdata/script/eval_errs.txt b/cmd/cue/cmd/testdata/script/eval_errs.txt
new file mode 100644
index 0000000..c2ce221
--- /dev/null
+++ b/cmd/cue/cmd/testdata/script/eval_errs.txt
@@ -0,0 +1,20 @@
+! cue eval errs.cue
+cmp stderr expect-stderr
+cmp stdout expect-stdout
+
+-- expect-stdout --
+-- expect-stderr --
+bar: empty disjunction: conflicting values int and "str" (mismatched types int and string):
+ ./errs.cue:5:10
+ ./errs.cue:6:16
+x.q: conflicting values "hello" and "goodbye":
+ ./errs.cue:1:4
+ ./errs.cue:2:4
+-- errs.cue --
+a: "hello"
+b: "goodbye"
+x: {q: a, q: b}
+
+foo: {a: int} | {b: string}
+bar: foo & {a: "str", b: 2}
+
diff --git a/cmd/cue/cmd/vet.go b/cmd/cue/cmd/vet.go
index 45c9300..9d2913d 100644
--- a/cmd/cue/cmd/vet.go
+++ b/cmd/cue/cmd/vet.go
@@ -117,6 +117,7 @@
opt := []cue.Option{
cue.Attributes(true),
cue.Optional(true),
+ cue.Definitions(true),
cue.Hidden(true),
}
w := cmd.Stderr()
diff --git a/cue/ast.go b/cue/ast.go
index 35ce5c6..08d6072 100644
--- a/cue/ast.go
+++ b/cue/ast.go
@@ -335,9 +335,6 @@
opt := n.Optional != token.NoPos
isDef := n.Token == token.ISA
if isDef {
- if opt {
- v.errf(n, "a definition may not be optional")
- }
ctx := v.ctx()
ctx.inDefinition++
defer func() { ctx.inDefinition-- }()
diff --git a/cue/types.go b/cue/types.go
index ac7412b..581f286 100644
--- a/cue/types.go
+++ b/cue/types.go
@@ -1107,6 +1107,7 @@
obj := v.eval(ctx).(*structLit)
// TODO: This is expansion appropriate?
+ // TODO: return an incomplete error if there are still expansions remaining.
obj, err := obj.expandFields(ctx) // expand comprehensions
if err != nil {
return structValue{}, err
@@ -1571,29 +1572,81 @@
// more than one error, retrievable with errors.Errors, if more than one
// exists.
func (v Value) Validate(opts ...Option) error {
- o := getOptions(opts)
- var errs errors.Error
- v.Walk(func(v Value) bool {
- if err := v.checkKind(v.ctx(), bottomKind); err != nil {
- if !o.concrete && isIncomplete(err) {
- if o.disallowCycles && err.code == codeCycle {
- errs = errors.Append(errs, v.toErr(err))
- }
- return false
+ x := validator{}
+ o := options{omitOptional: true}
+ o.updateOptions(opts)
+ x.walk(v, o)
+ return errors.Sanitize(x.errs)
+}
+
+type validator struct {
+ errs errors.Error
+ depth int
+}
+
+func (x *validator) before(v Value, o options) bool {
+ if err := v.checkKind(v.ctx(), bottomKind); err != nil {
+ if !o.concrete && isIncomplete(err) {
+ if o.disallowCycles && err.code == codeCycle {
+ x.errs = errors.Append(x.errs, v.toErr(err))
}
- errs = errors.Append(errs, v.toErr(err))
- if len(errors.Errors(errs)) > 50 {
- return false // mostly to avoid some hypothetical cycle issue
- }
+ return false
}
- if o.concrete {
- if err := isGroundRecursive(v.ctx(), v.eval(v.ctx())); err != nil {
- errs = errors.Append(errs, v.toErr(err))
- }
+ x.errs = errors.Append(x.errs, v.toErr(err))
+ if len(errors.Errors(x.errs)) > 50 {
+ return false // mostly to avoid some hypothetical cycle issue
}
- return true
- }, nil)
- return errors.Sanitize(errs)
+ }
+ if o.concrete {
+ if err := isGroundRecursive(v.ctx(), v.eval(v.ctx())); err != nil {
+ x.errs = errors.Append(x.errs, v.toErr(err))
+ }
+ }
+ return true
+}
+
+func (x *validator) walk(v Value, opts options) {
+ // TODO(#42): we can get rid of the arbitrary evaluation depth once CUE has
+ // proper structural cycle detection. See Issue #42. Currently errors
+ // occuring at a depth > 20 will not be detected.
+ if x.depth > 20 {
+ return
+ }
+ ctx := v.ctx()
+ switch v.Kind() {
+ case StructKind:
+ if !x.before(v, opts) {
+ return
+ }
+ x.depth++
+ obj, err := v.structValOpts(ctx, opts)
+ if err != nil {
+ x.errs = errors.Append(x.errs, v.toErr(err))
+ }
+ for i := 0; i < obj.Len(); i++ {
+ _, v := obj.At(i)
+ opts := opts
+ if obj.n.arcs[i].definition {
+ opts.concrete = false
+ }
+ x.walk(v, opts)
+ }
+ x.depth--
+
+ case ListKind:
+ if !x.before(v, opts) {
+ return
+ }
+ x.depth++
+ list, _ := v.List()
+ for list.Next() {
+ x.walk(list.Value(), opts)
+ }
+ x.depth--
+
+ default:
+ x.before(v, opts)
+ }
}
func isGroundRecursive(ctx *context, v value) *bottom {
diff --git a/cue/types_test.go b/cue/types_test.go
index c892443..0b7dcd7 100644
--- a/cue/types_test.go
+++ b/cue/types_test.go
@@ -1075,6 +1075,29 @@
a: 1
b: { c: 2, d: 3 }
c d e f: 5
+ g?: int
+ `,
+ opts: []Option{Concrete(true)},
+ }, {
+ desc: "definition error",
+ in: `
+ b :: 1 & 2
+ `,
+ opts: []Option{},
+ err: true,
+ }, {
+ desc: "definition error okay if optional",
+ in: `
+ b? :: 1 & 2
+ `,
+ opts: []Option{},
+ }, {
+ desc: "definition with optional",
+ in: `
+ b:: {
+ a: int
+ b?: >=0
+ }
`,
opts: []Option{Concrete(true)},
}, {
@@ -1124,6 +1147,12 @@
a: { b: time.Duration } | { c: time.Duration }
`,
+ }, {
+ desc: "comprehension error",
+ in: `
+ a: { if b == "foo" { field: 2 } }
+ `,
+ err: true,
}}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
diff --git a/doc/ref/spec.md b/doc/ref/spec.md
index f07149d..4efc3a7 100644
--- a/doc/ref/spec.md
+++ b/doc/ref/spec.md
@@ -1010,7 +1010,7 @@
`a.f` and `b.f` are optional.
Any [references](#References) to `a` or `b`
in their respective field values need to be replaced with references to `c`.
-The result of a unification is bottom (`_|_`) if any of its required
+The result of a unification is bottom (`_|_`) if any of its non-optional
fields evaluates to bottom, recursively.
<!--NOTE: About bottom values for optional fields being okay.
