cue: fix bug that dropped optional fields in def
Made recExpr more clearly distinguish between
schema and data mode, so that schema mode will
never inadvertently fall back into "concrete" mode.
Closes #304
Change-Id: I253d928f48fc14dd788318bbbfc5f39525da8d5d
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/5248
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cmd/cue/cmd/testdata/script/issue304.txt b/cmd/cue/cmd/testdata/script/issue304.txt
new file mode 100644
index 0000000..01036d1
--- /dev/null
+++ b/cmd/cue/cmd/testdata/script/issue304.txt
@@ -0,0 +1,22 @@
+cue def -e Foo x.cue
+cmp stdout expect-stdout
+
+-- expect-stdout --
+
+close({
+ x: int
+ body?: close({
+ a: int
+ b?: string
+ })
+})
+-- x.cue --
+package example
+
+Foo :: {
+ x: int
+ body?: {
+ a: int
+ b?: string
+ }
+}
diff --git a/cue/export.go b/cue/export.go
index 885a101..9ea215b 100644
--- a/cue/export.go
+++ b/cue/export.go
@@ -340,31 +340,48 @@
m = p.ctx.manifest(e)
}
isComplete := p.isComplete(m, false)
- if optional || (!isComplete && (!p.mode.concrete)) {
- resolve := p.mode.resolveReferences && !optional
- if !p.mode.final && v.kind().hasReferences() && !resolve {
- return p.expr(v)
- }
- if p.mode.concrete && !m.kind().isGround() {
- p.addIncomplete(v)
- }
- // TODO: do something more principled than this hack.
- // This likely requires disjunctions to keep track of original
- // values (so using arcs instead of values).
- opts := options{concrete: true, raw: true}
- p := &exporter{p.ctx, opts, p.stack, p.top, p.imports, p.inDef, nil}
- if isDisjunction(v) || isBottom(e) {
- return p.expr(v)
- }
- if v.kind()&structKind == 0 {
- return p.expr(e)
- }
- if optional || isDisjunction(e) {
- // Break cycles: final and resolveReferences really should not be
- // used with optional.
- p.mode.resolveReferences = false
- p.mode.final = false
- return p.expr(v)
+ if optional || (!isComplete && !p.mode.concrete) {
+ if !p.mode.final {
+ // Schema mode.
+
+ // Print references as they are, if applicable.
+ //
+ // TODO: We probably should not allow resolving references in
+ // schema mode, or at most allow resolving _some_ references, like
+ // those defined outside of packages.
+ noResolve := !p.mode.resolveReferences
+ if optional {
+ // Don't resolve references when a field is optional.
+ // This may break some unnecessary cycles.
+ noResolve = true
+ }
+ if isBottom(e) || (v.kind().hasReferences() && noResolve) {
+ return p.expr(v)
+ }
+ } else {
+ // Data mode.
+
+ if p.mode.concrete && !m.kind().isGround() {
+ p.addIncomplete(v)
+ }
+ // TODO: do something more principled than this hack.
+ // This likely requires disjunctions to keep track of original
+ // values (so using arcs instead of values).
+ opts := options{concrete: true, raw: true}
+ p := &exporter{p.ctx, opts, p.stack, p.top, p.imports, p.inDef, nil}
+ if isDisjunction(v) || isBottom(e) {
+ return p.expr(v)
+ }
+ if v.kind()&structKind == 0 {
+ return p.expr(e)
+ }
+ if optional || isDisjunction(e) {
+ // Break cycles: final and resolveReferences really should not be
+ // used with optional.
+ p.mode.resolveReferences = false
+ p.mode.final = false
+ return p.expr(v)
+ }
}
}
return p.expr(e)
@@ -398,15 +415,14 @@
if p.mode.concrete && !x.kind().isGround() {
p.addIncomplete(v)
}
- if isBottom(e) {
+ switch {
+ case isBottom(e):
if p.mode.concrete {
p.addIncomplete(v)
}
p = &exporter{p.ctx, options{raw: true}, p.stack, p.top, p.imports, p.inDef, nil}
return p.expr(v)
- }
- switch {
- case !p.mode.final && v.kind().hasReferences() && !p.mode.resolveReferences:
+ case v.kind().hasReferences() && !p.mode.resolveReferences:
case doEval(p.mode):
v = e
}
diff --git a/cue/export_test.go b/cue/export_test.go
index f1f8ad0..e1d5eca 100644
--- a/cue/export_test.go
+++ b/cue/export_test.go
@@ -1050,6 +1050,20 @@
}
A :: string | [A]
}`),
+ }, {
+ eval: true,
+ opts: []Option{Definitions(true)},
+ in: `
+ body?: {
+ a: int
+ b?: string
+ }
+ `,
+ out: unindent(`
+ body?: {
+ a: int
+ b?: string
+ }`),
}}
for _, tc := range testCases {
t.Run("", func(t *testing.T) {
