internal/core/adt: allow referencing required fields

This is intended to be a temporary measure to help
the transition of JSON schema to use ! for required
properties.

Currently, CUE code that relied on the JSON schema
generator to generate regular fields for required fields
could just reference those fields. This code is currently
broken by this transition.

In practice, however, there does not seem to be much
harm in allowing reference to refer to required fields. CUE
allows errors to be elided if another error is guaranteed to
be given otherwise. This is not always the case, but
but it seems that this is fine for any practical purpose.

Example where this is fully okay:

	#Foo: {
	  a!: 1
	  b: a
	}

	// c currently has errors for c.a (required field missing) and
	// c.b (referencing required field). With this change, the
	// second error is removed. But since there is still an error
	// for the underlying problem (a missing required field), this
	// is okay.
	c: #Foo

If, on the other hand, we would write:

	c: #Foo.a

then `c` will not produce an error where before this change it
would. Note that, for export at least, this is only true
if a required field is used in combination with a fully
concrete value.

We also think this solution does not complicate finding a
more principled solution later. If the JSON Schema change
is left in, users will be forced to adjust their code as is. To
facilitate such transition, we would have to write a `cue fix`.
This same mechansim could later be used to move code
that references required fields to using some alternative.
This CL allows for a smoother transition either way.

Note that there was a discrepancy in the semantics between
v2 and v3 that has been removed by this change.

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: I6b79bcc858ea7c2a76418b58e6ef22839c558df2
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1199898
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>

cue/testdata/eval/required.txtar

@@ -207,11 +207,11 @@ Leaks:  0
 Freed:  66
 Reused: 58
 Allocs: 8
-Retain: 13
+Retain: 10
 
 Unifications: 66
-Conjuncts:    92
-Disjuncts:    79
+Conjuncts:    90
+Disjuncts:    76
 -- out/evalalpha --
 (_|_){
   // [eval]
@@ -269,16 +269,10 @@ Disjuncts:    79
         #Foo: (#struct){
           a!: (int){ int }
         }
-        b: (_|_){
-          // [incomplete] reference.toWithinDefinition.p1.b: cannot reference optional field: a:
-          //     ./in.cue:43:11
-        }
+        b: (int){ int }
       }
       p2: (struct){
-        b: (_|_){
-          // [incomplete] reference.toWithinDefinition.p2.b: cannot reference optional field: a:
-          //     ./in.cue:46:11
-        }
+        b: (int){ int }
         #Foo: (#struct){
           a!: (int){ int }
         }
@@ -292,16 +286,10 @@ Disjuncts:    79
         b: (#struct){
           a!: (int){ int }
         }
-        c: (_|_){
-          // [incomplete] reference.toFieldFromDefinition.p1.c: cannot reference optional field: a:
-          //     ./in.cue:52:8
-        }
+        c: (int){ int }
       }
       p2: (struct){
-        c: (_|_){
-          // [incomplete] reference.toFieldFromDefinition.p2.c: cannot reference optional field: a:
-          //     ./in.cue:55:8
-        }
+        c: (int){ int }
         b: (#struct){
           a!: (int){ int }
         }
@@ -315,16 +303,10 @@ Disjuncts:    79
         x: (struct){
           y!: (_){ _ }
         }
-        b: (_|_){
-          // [incomplete] reference.toNonDefinition.p1.b: cannot reference optional field: y:
-          //     ./in.cue:61:8
-        }
+        b: (_){ _ }
       }
       p2: (struct){
-        b: (_|_){
-          // [incomplete] reference.toNonDefinition.p2.b: cannot reference optional field: y:
-          //     ./in.cue:64:8
-        }
+        b: (_){ _ }
         x: (struct){
           y!: (_){ _ }
         }
@@ -333,16 +315,10 @@ Disjuncts:    79
     toConcrete: (struct){
       p1: (struct){
         a!: (int){ 1 }
-        b: (_|_){
-          // [incomplete] reference.toConcrete.p1.b: cannot reference optional field: a:
-          //     ./in.cue:69:6
-        }
+        b: (int){ 2 }
       }
       p2: (struct){
-        b: (_|_){
-          // [incomplete] reference.toConcrete.p2.b: cannot reference optional field: a:
-          //     ./in.cue:72:6
-        }
+        b: (int){ 2 }
         a!: (int){ 1 }
       }
     }
@@ -422,16 +398,10 @@ Missing position.
         #Foo: (#struct){
           a!: (int){ int }
         }
-        b: (_|_){
-          // [incomplete] reference.toWithinDefinition.p1.b: cannot reference optional field: a:
-          //     ./in.cue:43:11
-        }
+        b: (int){ int }
       }
       p2: (struct){
-        b: (_|_){
-          // [incomplete] reference.toWithinDefinition.p2.b: cannot reference optional field: a:
-          //     ./in.cue:46:11
-        }
+        b: (int){ int }
         #Foo: (#struct){
           a!: (int){ int }
         }
@@ -445,16 +415,10 @@ Missing position.
         b: (#struct){
           a!: (int){ int }
         }
-        c: (_|_){
-          // [incomplete] reference.toFieldFromDefinition.p1.c: cannot reference optional field: a:
-          //     ./in.cue:52:8
-        }
+        c: (int){ int }
       }
       p2: (struct){
-        c: (_|_){
-          // [incomplete] reference.toFieldFromDefinition.p2.c: cannot reference optional field: a:
-          //     ./in.cue:55:8
-        }
+        c: (int){ int }
         b: (#struct){
           a!: (int){ int }
         }
@@ -468,16 +432,10 @@ Missing position.
         x: (struct){
           y!: (_){ _ }
         }
-        b: (_|_){
-          // [incomplete] reference.toNonDefinition.p1.b: cannot reference optional field: y:
-          //     ./in.cue:61:8
-        }
+        b: (_){ _ }
       }
       p2: (struct){
-        b: (_|_){
-          // [incomplete] reference.toNonDefinition.p2.b: cannot reference optional field: y:
-          //     ./in.cue:64:8
-        }
+        b: (_){ _ }
         x: (struct){
           y!: (_){ _ }
         }
@@ -486,16 +444,10 @@ Missing position.
     toConcrete: (struct){
       p1: (struct){
         a!: (int){ 1 }
-        b: (_|_){
-          // [incomplete] reference.toConcrete.p1.b: cannot reference optional field: a:
-          //     ./in.cue:69:6
-        }
+        b: (int){ 2 }
       }
       p2: (struct){
-        b: (_|_){
-          // [incomplete] reference.toConcrete.p2.b: cannot reference optional field: a:
-          //     ./in.cue:72:6
-        }
+        b: (int){ 2 }
         a!: (int){ 1 }
       }
     }

internal/core/adt/context.go

@@ -948,7 +948,9 @@ func (c *OpContext) lookup(x *Vertex, pos token.Pos, l Feature, flags combinedFl
 			c.unify(a, deprecated(c, partial))
 		}
 
-		if a.IsConstraint() {
+		// TODO(refRequired): see comment in unify.go:Vertex.lookup near the
+		// namesake TODO.
+		if a.ArcType == ArcOptional {
 			code := IncompleteError
 			if hasCycle {
 				code = CycleError

internal/core/adt/unify.go

@@ -727,10 +727,19 @@ func (v *Vertex) lookup(c *OpContext, pos token.Pos, f Feature, flags combinedFl
 
 handleArcType:
 	switch arc.ArcType {
-	case ArcMember:
+	case ArcMember, ArcRequired:
 		return arcReturn
 
-	case ArcOptional, ArcRequired:
+	case ArcOptional:
+		// Technically, this failure also applies to required fields. We assume
+		// however, that if a reference field that is made regular will already
+		// result in an error, so that piling up another error is not strictly
+		// necessary. Note that the spec allows for eliding an error if it is
+		// guaranteed another error is generated elsewhere. This does not
+		// properly cover the case where a reference is made directly within the
+		// definition, but this is fine for the purpose it serves.
+		// TODO(refRequired): revisit whether referencing required fields should
+		// fail.
 		label := f.SelectorString(c.Runtime)
 		b := &Bottom{
 			Code: IncompleteError,