Skip to content

Commit

Permalink
internal/core/export: always export required fields
Browse files Browse the repository at this point in the history
Required fields are either printed (in case of a definition) or
erroneous (in case of export). We now always print them and
leave error reporting of required fields, if desired, up to the user.

Fixes #2305

Change-Id: Id440a20a5f4b247a52ce4c74555903df39c7214f
Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/552169
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>
Unity-Result: CUEcueckoo <cueckoo@cuelang.org>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
  • Loading branch information
mpvl committed Apr 5, 2023
1 parent d05d9c1 commit d5dd9ec
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 10 deletions.
42 changes: 42 additions & 0 deletions internal/core/export/testdata/main/def.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@ c: [string]: int
#d: {
e!: string
e?: =~"a"

f?: 1
}

// Issue #2305
g: #d
-- out/definition --
a: int | *2
b?: 4 | 5
Expand All @@ -15,19 +20,34 @@ c: {
}
#d: {
e!: =~"a"
f?: 1
}

// Issue #2305
g: #d
-- out/doc --
[]
[a]
[b]
[c]
[#d]
[#d e]
[#d f]
[g]
- Issue #2305

[g e]
[g f]
-- out/value --
== Simplified
{
a: *2 | int
c: {}

// Issue #2305
g: {
e!: =~"a"
}
}
== Raw
{
Expand All @@ -36,12 +56,22 @@ c: {
c: {}
#d: {
e!: =~"a"
f?: 1
}

// Issue #2305
g: {
e!: =~"a"
f?: 1
}
}
== Final
{
a: 2
c: {}
g: {
e!: =~"a"
}
}
== All
{
Expand All @@ -50,6 +80,13 @@ c: {
c: {}
#d: {
e!: =~"a"
f?: 1
}

// Issue #2305
g: {
e!: =~"a"
f?: 1
}
}
== Eval
Expand All @@ -59,5 +96,10 @@ c: {
c: {}
#d: {
e!: =~"a"
f?: 1
}
g: {
e!: =~"a"
f?: 1
}
}
20 changes: 10 additions & 10 deletions internal/core/export/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,9 @@ func (e *exporter) vertex(n *adt.Vertex) (result ast.Expr) {

case *adt.Bottom:
switch {
case n.IsConstraint():
// Constraints may always be the original value.
// TODO: this was included for backwards compatibility. But
// should we show the error here? It signifies that this field
// may not be used.
case n.ArcType == adt.ArcOptional:
// Optional fields may always be the original value.

case e.cfg.ShowErrors && x.ChildError:
// TODO(perf): use precompiled arc statistics
if len(n.Arcs) > 0 && n.Arcs[0].Label.IsInt() && !e.showArcs(n) && attrs == nil {
Expand Down Expand Up @@ -438,12 +436,14 @@ func (e *exporter) structComposite(v *adt.Vertex, attrs []*ast.Attribute) ast.Ex
continue
}

if isOptional := arc.IsConstraint(); isOptional {
if !p.ShowOptional {
continue
}
internal.SetConstraint(f, arc.ArcType.Token())
if arc.ArcType == adt.ArcOptional && !p.ShowOptional {
continue
}
// TODO: report an error for required fields in Final mode?
// This package typically does not create errors that did not result
// from evaluation already.

internal.SetConstraint(f, arc.ArcType.Token())

f.Value = e.vertex(arc)

Expand Down

0 comments on commit d5dd9ec

Please sign in to comment.