internal/core/adt: implement required fields.

Introduce the internal and external types to represent required fields. Fixes #2003 Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Iff492d01f929cfcc4ef6c36846da5f30d5b70a77 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/551206 Reviewed-by: Aram Hăvărneanu <aram@cue.works> Reviewed-by: Roger Peppe <rogpeppe@gmail.com> TryBot-Result: CUEcueckoo <cueckoo@cuelang.org> Unity-Result: CUEcueckoo <cueckoo@cuelang.org>
cue-lang · Mar 22, 2023 · 0b681f5 · 0b681f5
1 parent 1da0fd9
commit 0b681f5
Show file tree

Hide file tree

Showing 10 changed files with 232 additions and 43 deletions.
diff --git a/cmd/cue/cmd/get_go.go b/cmd/cue/cmd/get_go.go
@@ -1005,10 +1005,20 @@ func (e *extractor) makeField(name string, kind fieldKind, expr types.Type, doc
 		cueast.SetRelPos(doc, cuetoken.NewSection)
 	}
 
-	if kind == optional {
-		f.Token = cuetoken.COLON
-		f.Optional = cuetoken.Blank.Pos()
+	var tok cuetoken.Token
+	switch kind {
+	case definition:
+		// neither optional nor required.
+	case regular:
+		// TODO(required): use idiomatic CUE (token.NOT) if CUE version is
+		// higher than a certain number? We may not be able to determine this
+		// accurately enough.
+	case optional:
+		tok = cuetoken.OPTION
 	}
+
+	internal.SetConstraint(f, tok)
+
 	b, _ := format.Node(typ)
 	return f, string(b)
 }

diff --git a/cue/path.go b/cue/path.go
@@ -48,6 +48,8 @@ const (
 
 	// OptionalConstraint represents an optional constraint (?).
 	OptionalConstraint
+	// RequiredConstraint represents a required constraint (!).
+	RequiredConstraint
 	// PatternConstraint represents a selector of fields in a struct
 	// or array that match a constraint.
 	PatternConstraint
@@ -62,7 +64,7 @@ func (t SelectorType) LabelType() SelectorType {
 
 // ConstraintType reports the constraint type of t.
 func (t SelectorType) ConstraintType() SelectorType {
-	return t & 0b0110_0000
+	return t & 0b1110_0000
 }
 
 var selectorTypeStrings = [...]string{
@@ -73,6 +75,7 @@ var selectorTypeStrings = [...]string{
 	"HiddenLabel",
 	"HiddenDefinitionLabel",
 	"OptionalConstraint",
+	"RequiredConstraint",
 	"PatternConstraint",
 }
 

diff --git a/cue/path_test.go b/cue/path_test.go
@@ -300,7 +300,7 @@ func TestSelectorTypeString(t *testing.T) {
 	if got, want := (StringLabel | OptionalConstraint).String(), "StringLabel|OptionalConstraint"; got != want {
 		t.Errorf("unexpected SelectorType.String result; got %q want %q", got, want)
 	}
-	if got, want := SelectorType(255).String(), "StringLabel|IndexLabel|DefinitionLabel|HiddenLabel|HiddenDefinitionLabel|OptionalConstraint|PatternConstraint"; got != want {
+	if got, want := SelectorType(255).String(), "StringLabel|IndexLabel|DefinitionLabel|HiddenLabel|HiddenDefinitionLabel|OptionalConstraint|RequiredConstraint|PatternConstraint"; got != want {
 		t.Errorf("unexpected SelectorType.String result; got %q want %q", got, want)
 	}
 }

diff --git a/cue/testdata/eval/required.txtar b/cue/testdata/eval/required.txtar
@@ -0,0 +1,134 @@
+-- in.cue --
+self: t1: {
+	a?: int
+}
+
+self: t2: {
+	a!: int
+	a!: int
+}
+
+unify: t1: p1: {
+	a!: int
+	a: int
+}
+
+unify: t1: p2: {
+	a: int
+	a!: int
+}
+
+unify: t2: p1: {
+	a!: int
+	a?: int
+}
+
+unify: t2: p2: {
+	a?: int
+	a!: int
+}
+#Def: t1: {
+	a!: int
+}
+-- out/compile --
+--- in.cue
+{
+  self: {
+    t1: {
+      a?: int
+    }
+  }
+  self: {
+    t2: {
+      a!: int
+      a!: int
+    }
+  }
+  unify: {
+    t1: {
+      p1: {
+        a!: int
+        a: int
+      }
+    }
+  }
+  unify: {
+    t1: {
+      p2: {
+        a: int
+        a!: int
+      }
+    }
+  }
+  unify: {
+    t2: {
+      p1: {
+        a!: int
+        a?: int
+      }
+    }
+  }
+  unify: {
+    t2: {
+      p2: {
+        a?: int
+        a!: int
+      }
+    }
+  }
+  #Def: {
+    t1: {
+      a!: int
+    }
+  }
+}
+-- out/eval/stats --
+Leaks:  0
+Freed:  20
+Reused: 15
+Allocs: 5
+Retain: 0
+
+Unifications: 20
+Conjuncts:    31
+Disjuncts:    20
+-- out/eval --
+Errors:
+self.t2.a: field is required but not present
+unify.t2.p1.a: field is required but not present
+unify.t2.p2.a: field is required but not present
+
+Result:
+(struct){
+  self: (struct){
+    t1: (struct){
+      a?: (int){ int }
+    }
+    t2: (struct){
+      a!: (int){ int }
+    }
+  }
+  unify: (struct){
+    t1: (struct){
+      p1: (struct){
+        a: (int){ int }
+      }
+      p2: (struct){
+        a: (int){ int }
+      }
+    }
+    t2: (struct){
+      p1: (struct){
+        a!: (int){ int }
+      }
+      p2: (struct){
+        a!: (int){ int }
+      }
+    }
+  }
+  #Def: (#struct){
+    t1: (#struct){
+      a!: (int){ int }
+    }
+  }
+}
diff --git a/cue/types.go b/cue/types.go
@@ -1732,6 +1732,8 @@ func (v Value) FillPath(p Path, x interface{}) Value {
 			switch sel.ConstraintType() {
 			case OptionalConstraint:
 				f.ArcType = adt.ArcOptional
+			case RequiredConstraint:
+				f.ArcType = adt.ArcRequired
 			}
 
 			expr = &adt.StructLit{Decls: []adt.Decl{f}}

diff --git a/internal/core/adt/composite.go b/internal/core/adt/composite.go
@@ -245,7 +245,7 @@ func (v *Vertex) isDefined() bool {
 
 // IsConstraint reports whether the Vertex is an optional or required field.
 func (v *Vertex) IsConstraint() bool {
-	return v.ArcType == ArcOptional
+	return v.ArcType == ArcOptional || v.ArcType == ArcRequired
 }
 
 // IsDefined indicates whether this arc is defined meaning it is not a
@@ -267,7 +267,12 @@ const (
 	// (including regular, hidden, and definition fields).
 	ArcMember ArcType = iota
 
-	// ArcOptional represents fields of the form foo?.
+	// ArcRequired is like optional, but requires that a field be specified.
+	// Fields are of the form foo!.
+	ArcRequired
+
+	// ArcOptional represents fields of the form foo? and defines constraints
+	// for foo in case it is defined.
 	ArcOptional
 
 	// TODO: define a type for optional arcs. This will be needed for pulling
@@ -287,9 +292,8 @@ func ConstraintFromToken(t token.Token) ArcType {
 	switch t {
 	case token.OPTION:
 		return ArcOptional
-		// TODO
-		// case token.NOT:
-		// 	return ArcRequired
+	case token.NOT:
+		return ArcRequired
 	}
 	return ArcMember
 }
@@ -300,6 +304,8 @@ func (a ArcType) Token() (t token.Token) {
 	switch a {
 	case ArcOptional:
 		t = token.OPTION
+	case ArcRequired:
+		t = token.NOT
 	}
 	return t
 }
@@ -310,6 +316,8 @@ func (a ArcType) Suffix() string {
 	switch a {
 	case ArcOptional:
 		return "?"
+	case ArcRequired:
+		return "!"
 	}
 	return ""
 }
@@ -850,9 +858,7 @@ func (v *Vertex) hasConjunct(c Conjunct) (added bool) {
 	switch f := c.x.(type) {
 	case *BulkOptionalField, *Ellipsis:
 	case *Field:
-		if f.ArcType == ArcMember {
-			v.ArcType = ArcMember
-		}
+		v.UpdateArcType(f.ArcType)
 	default:
 		v.ArcType = ArcMember
 	}

diff --git a/internal/core/export/expr.go b/internal/core/export/expr.go
@@ -20,6 +20,7 @@ import (
 
 	"cuelang.org/go/cue/ast"
 	"cuelang.org/go/cue/token"
+	"cuelang.org/go/internal"
 	"cuelang.org/go/internal/core/adt"
 )
 
@@ -156,6 +157,7 @@ func (x *exporter) mergeValues(label adt.Feature, src *adt.Vertex, a []conjunct,
 		for _, a := range src.Arcs {
 			if x, ok := e.fields[a.Label]; ok {
 				x.arc = a
+				x.arcType = a.ArcType
 				e.fields[a.Label] = x
 			}
 		}
@@ -232,7 +234,7 @@ func (x *exporter) mergeValues(label adt.Feature, src *adt.Vertex, a []conjunct,
 		if f.IsLet() {
 			continue
 		}
-		field := e.fields[f]
+		field := e.getField(f)
 		c := field.conjuncts
 
 		label := e.stringLabel(f)
@@ -261,9 +263,7 @@ func (x *exporter) mergeValues(label adt.Feature, src *adt.Vertex, a []conjunct,
 			x.inDefinition--
 		}
 
-		if isOptional(a) {
-			d.Optional = token.Blank.Pos()
-		}
+		internal.SetConstraint(d, field.arcType.Token())
 		if x.cfg.ShowDocs {
 			docs := extractDocs(src, a)
 			ast.SetComments(d, docs)
@@ -317,6 +317,14 @@ type conjuncts struct {
 	isData bool
 }
 
+func (c *conjuncts) getField(label adt.Feature) field {
+	f, ok := c.fields[label]
+	if !ok {
+		f.arcType = adt.ArcVoid
+	}
+	return f
+}
+
 func (c *conjuncts) addValueConjunct(src *adt.Vertex, env *adt.Environment, x adt.Elem) {
 	switch b, ok := x.(adt.BaseValue); {
 	case ok && src != nil && isTop(b) && !isTop(src.BaseValue):
@@ -326,8 +334,12 @@ func (c *conjuncts) addValueConjunct(src *adt.Vertex, env *adt.Environment, x ad
 	}
 }
 
-func (c *conjuncts) addConjunct(f adt.Feature, env *adt.Environment, n adt.Node) {
-	x := c.fields[f]
+func (c *conjuncts) addConjunct(f adt.Feature, t adt.ArcType, env *adt.Environment, n adt.Node) {
+	x := c.getField(f)
+	if t < x.arcType {
+		x.arcType = t
+	}
+
 	v := adt.MakeRootConjunct(env, n)
 	x.conjuncts = append(x.conjuncts, conjunct{
 		c:  v,
@@ -338,6 +350,7 @@ func (c *conjuncts) addConjunct(f adt.Feature, env *adt.Environment, n adt.Node)
 }
 
 type field struct {
+	arcType   adt.ArcType
 	docs      []*ast.CommentGroup
 	arc       *adt.Vertex
 	conjuncts []conjunct
@@ -372,9 +385,11 @@ func (e *conjuncts) addExpr(env *adt.Environment, src *adt.Vertex, x adt.Elem, i
 
 		for _, d := range x.Decls {
 			var label adt.Feature
+			t := adt.ArcVoid
 			switch f := d.(type) {
 			case *adt.Field:
 				label = f.Label
+				t = f.ArcType
 				// TODO: mark optional here, if needed.
 			case *adt.LetField:
 				continue
@@ -389,7 +404,7 @@ func (e *conjuncts) addExpr(env *adt.Environment, src *adt.Vertex, x adt.Elem, i
 			default:
 				panic(fmt.Sprintf("Unexpected type %T", d))
 			}
-			e.addConjunct(label, env, d)
+			e.addConjunct(label, t, env, d)
 		}
 		e.top().upCount--
 
@@ -428,7 +443,7 @@ func (e *conjuncts) addExpr(env *adt.Environment, src *adt.Vertex, x adt.Elem, i
 						continue
 					}
 
-					e.addConjunct(a.Label, env, a)
+					e.addConjunct(a.Label, a.ArcType, env, a)
 				}
 			}
 		}
@@ -473,28 +488,6 @@ func isTop(x adt.BaseValue) bool {
 	}
 }
 
-// TODO: find a better way to annotate optionality. Maybe a special conjunct
-// or store it in the field information?
-func isOptional(a []adt.Conjunct) bool {
-	if len(a) == 0 {
-		return false
-	}
-	for _, c := range a {
-		if v, ok := c.Elem().(*adt.Vertex); ok && !v.IsData() && len(v.Conjuncts) > 0 {
-			return isOptional(v.Conjuncts)
-		}
-		switch f := c.Source().(type) {
-		case nil:
-			return false
-		case *ast.Field:
-			if f.Optional == token.NoPos {
-				return false
-			}
-		}
-	}
-	return true
-}
-
 func isComplexStruct(s *adt.StructLit) bool {
 	for _, d := range s.Decls {
 		switch x := d.(type) {