cue-lang / cue

The home of the CUE language! Validate and define text-based and dynamic configuration
https://cuelang.org
Apache License 2.0
5.02k stars 285 forks source link

evaluator: embedded definition cannot be opened recursively #2023

Open rogpeppe opened 1 year ago

rogpeppe commented 1 year ago

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

$ cue version
cue version v0.0.0-20221001181137-79d80efc2882

      -buildmode exe
       -compiler gc
     CGO_ENABLED 1
          GOARCH amd64
            GOOS linux
         GOAMD64 v1
             vcs git
    vcs.revision 79d80efc288208410f77d47ae9aedd37e106d84b
        vcs.time 2022-10-01T18:11:37Z
    vcs.modified false

Does this issue reproduce with the latest release?

Yes

What did you do?

exec cue export test.cue
cmp stdout expect.json

-- test.cue --
#D: x: int
#E: d: #D

#F: {
    #E
    ...
}

x: #F
x: d: x: 9
x: d: y: 10
-- expect.json --
{
    "x": {
        "d": {
            "x": 9,
            "y": 10
        }
    }
}

What did you expect to see?

A passing test.

What did you see instead?

> exec cue export test.cue
[stderr]
x.d.y: field not allowed:
    ./test.cue:1:5
    ./test.cue:2:8
    ./test.cue:5:2
    ./test.cue:9:4
    ./test.cue:10:7
    ./test.cue:11:7
[exit status 1]
FAIL: /tmp/testscript620117862/x.txtar/script.txtar:1: unexpected command failure

The definition is opened up at the top level, but not at any level below that, which I believe should be expected, according to the spec:

An embedded value of type struct is unified with the struct in which it is embedded, but disregarding the restrictions imposed by closed structs.

In this case, the restrictions imposed by closed structs are clearly not being disregarded.

myitcv commented 1 year ago

Did you perhaps mean for your example to be:

exec cue export test.cue
cmp stdout expect.json

-- test.cue --
#D: x: int
#E: d: #D

#F: {
    #E
    d: {
        ...
    }
}

x: #F
x: d: x: 9
x: d: y: 10
-- expect.json --
{
    "x": {
        "d": {
            "x": 9,
            "y": 10
        }
    }
}

In your example the ... is opening up #F, not #F.d

myitcv commented 1 year ago

For reference, linking to https://github.com/cue-lang/cue/issues/1576.

mpvl commented 1 year ago

In addition to Paul's suggestion, there are two other options:

But both of these approaches are currently broken.

rogpeppe commented 1 year ago

Did you perhaps mean for your example to be:

exec cue export test.cue
cmp stdout expect.json

-- test.cue --
#D: x: int
#E: d: #D

#F: {
  #E
  d: {
      ...
  }
}

x: #F
x: d: x: 9
x: d: y: 10
-- expect.json --
{
    "x": {
        "d": {
            "x": 9,
            "y": 10
        }
    }
}

In your example the ... is opening up #F, not #F.d

I intended what I wrote. The spec doesn't say anything about it only applying to a single level when it says "disregarding the restrictions imposed by closed structs.". #E is a closed struct. It's embedded inside #F. #F without the embedded #E allows any field at any level. Therefore #F with #E embedded should, by my understanding of the language of the spec, allow fields that are inside #F.d but not in #E.

rogpeppe commented 1 year ago

In addition to Paul's suggestion, there are two other options:

  • embedding {...} should have the effect of opening all fields recursively.

Presumably that would be the same as just embedding ... because in general embedding {x} is the same as embedding x for any x AIUI.

This would then be a language change rather than enhancement, because currently:

#B: a: int
#A: {
    foo: #B
    ...
}

would not allow:

a: #A
a: foo: b: 1

but if embedding ... opened recursively, that would change.

  • embedding _ should have a separate effect.

What kind of separate effect are you thinking of here?

But both of these approaches are currently broken.