cue-lang
cue: embedded disjunction of optional fields is incomplete
Originally opened by @rogpeppe in https://github.com/cuelang/cue/issues/353
commit 6f37a71e39b2c183c39875f2f442b8c8ae2b2526
The following code fails, but it seems like it should be OK.
e: Example
e: a: "hello"
Example :: {
a: string
{
value?: string
} | {
externalValue?: string
}
}
The error from cue vet -c is:
e: incomplete value ((C{a: (string & "hello"), value?: string} | C{a: (string & "hello"), externalValue?: string})):
./x.cue:1:4
./x.cue:3:4
./x.cue:5:12
./x.cue:7:2
Original reply by @mpvl in https://github.com/cuelang/cue/issues/353#issuecomment-759951670
Actually, the "fix" was an introduction of a bug. This should indeed be an incomplete value: the embedded disjunction results in two different structs. When made concrete they indeed will appear as the same value to the user. But for evaluation purposes, these are two different structs and both should be retained: at a later point they may still be unified with either optional value.
The simple solution is to mark on of the conjuncts as default.
One could argue that evaluation always reinserts the raw conjuncts anyway, and it doesn't matter to drop the optional fields. This is an implementation detail though, and that may not happen in the future.
Another convenience may be to say at at the point of concretizing a value, we first unique disjuncts disregarding optional values. But that should be an explicit decision.
Either way, if we do want this to not be an incomplete value, it needs to be carefully considered why.
Original reply by @mpvl in https://github.com/cuelang/cue/issues/353#issuecomment-760072003
To add other possible avenues FTR: in general it is not a good idea to have a disjunction of structs without a mandatory disambiguation field. If this is not possible, using a default value is highly encouraged. In this case this could be
*{} | { value?: string } | { externalValue?: string }
We could have a linter make such a suggestion.
Original reply by @mpvl in https://github.com/cuelang/cue/issues/353#issuecomment-760076213
Another thought is to do another pass of duplicate elimination at the time a value is made concrete, but at that time not considering optional fields.
We would have to closely consider what the impact would be if we allow dropping regular non-concrete fields by default, as proposed. Would we do the same thing there? Any difference in behavior there would likely be perceived as weird.
Also, that solution would perhaps suggest that int | 1 should be disambiguated to 1, which feels not right and dangerous. If that feeling is shared, we should strongly reconsider doing a similar disambiguation for structs with optional fields, as logically this is the same thing.
OTOH, allowing such a disambiguation step at manifestation time would eliminate the need to write * in many cases. Again, this is potentially a treacherous road to go in to, so this needs to be considered with care.
Original reply by @mpvl in https://github.com/cuelang/cue/issues/353#issuecomment-774633281
FTR, this can probably be solved by allowing to disambiguate based on concrete values only and then adding the unification of all optional fields of all disjuncts (a failed optional field just means the field is not allowed).
This would be somewhat similar to closing an open list upon manifestation.
Just dropping optionals is problematic as when resolving this as operant to a selection would drop types.