Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does embedding recursively open definitions? #1576

Open
verdverm opened this issue Mar 8, 2022 · 2 comments
Open

Does embedding recursively open definitions? #1576

verdverm opened this issue Mar 8, 2022 · 2 comments
Labels
closedness Discuss Requires maintainer discussion NeedsDesign Functionality seems desirable, but not sure how it should look like. roadmap/evaluator Specific tag for roadmap issue #338

Comments

@verdverm
Copy link

verdverm commented Mar 8, 2022

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

0.4.2

What did you do?

package repro

#Foo: {
  bars: [...#Bar]
}

#Bar: {
  bar: string
}

#Foo

bars: [{
  bar: "bar"
  baz: "baz"
}]

also same when not at top level

package repro

#Foo: {
  bars: [...#Bar]
}

#Bar: {
  bar: string
}

foo: {
  #Foo
  bars: [{
    bar: "bar"
    baz: "baz"
  }]
}

What did you expect to see?

An error like bars.0: field not allowed: baz:

What did you see instead?

$ cue eval repro.cue 
#Foo: {
    bars: []
}
#Bar: {
    bar: string
}
bars: [{
    bar: "bar"
    baz: "baz"
}]
@verdverm verdverm added NeedsInvestigation Triage Requires triage/attention labels Mar 8, 2022
@mpvl mpvl added the roadmap/evaluator Specific tag for roadmap issue #338 label Mar 22, 2022
@mpvl
Copy link
Member

mpvl commented Mar 22, 2022

@verdverm This is a very interesting case.

Definitions are indeed recursively opened according to the spec, although there are cases where it does not erroneously.

But arguably it should not open up these cases where a type is defined by a reference to a defintion.

That said, it does put in question the use of the embedding at top level pattern as seen here. For the non-top-level case one could simply write:

foo:  #Foo
foo: {
  bars: [{
    bar: "bar"
    baz: "baz"
  }]
}

But it is not possible to do this at the top level. So we may need another way to express this somehow, without writing

#Foo&{
  bars: [{
    bar: "bar"
    baz: "baz"
  }]
}

at the top level.

@mpvl mpvl added pre v1.0 closedness NeedsDesign Functionality seems desirable, but not sure how it should look like. and removed Triage Requires triage/attention NeedsInvestigation labels Mar 22, 2022
@myitcv
Copy link
Member

myitcv commented May 11, 2022

That said, it does put in question the use of the embedding at top level pattern as seen here

Didn't we envisage using must() in situations where the top level should be constrained by a definition but that definition not be opened?

But arguably it should not open up these cases where a type is defined by a reference to a defintion.

I'm not clear what the "right" answer is here. There is at least some symmetry to embedding recursively opening a definition where referencing a definition recursively closes it. The reason I hesitate on the relaxing of the embedding rule to "not open up these cases where a type is defined by a reference to a definition" is that people might be referencing definitions within a structure to enable/promote reuse, but still be relying on the existing behaviour of things being opened recursively.

So perhaps we need to collect examples that help to inform thinking here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closedness Discuss Requires maintainer discussion NeedsDesign Functionality seems desirable, but not sure how it should look like. roadmap/evaluator Specific tag for roadmap issue #338
Projects
None yet
Development

No branches or pull requests

4 participants