internal/core/adt: fix error type of MinFields for v3

In v3, information about what type of constraint fields exists is compeletely different. In MinFields and MaxFields, we did not account for the fact that optional fields could still become regular, changing the count. The old explanation of a change was incorrect and is removed. V2 and v3 are now equal for what used to be called failOptional1 Fixes #3450 Issue #3141 Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: I2eed630a9f4f2240ed5ec7909bf49f85986d1f51 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202299 Unity-Result: CUE porcuepine <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Reviewed-by: Matthew Sackman <[email protected]>
cue-lang · Oct 10, 2024 · 44cceca · 44cceca
1 parent ad21c10
commit 44cceca
Show file tree

Hide file tree

Showing 3 changed files with 128 additions and 54 deletions.
diff --git a/internal/pkg/types.go b/internal/pkg/types.go
@@ -62,7 +62,7 @@ func (s *Struct) Len() int {
 	return count
 }
 
-// IsOpen reports whether s allows more fields than are currently defined.
+// IsOpen reports whether s is open or has pattern constraints.
 func (s *Struct) IsOpen() bool {
 	if !s.node.IsClosedStruct() {
 		return true
@@ -77,6 +77,18 @@ func (s *Struct) IsOpen() bool {
 	return ot&^adt.HasDynamic != 0
 }
 
+// NumConstraintFields reports the number of explicit optional and required
+// fields, excluding pattern constraints.
+func (s Struct) NumConstraintFields() (count int) {
+	// If we have any optional arcs, we allow more fields.
+	for _, a := range s.node.Arcs {
+		if a.ArcType != adt.ArcMember && a.Label.IsRegular() {
+			count++
+		}
+	}
+	return count
+}
+
 // A ValidationError indicates an error that is only valid if a builtin is used
 // as a validator.
 type ValidationError struct {

diff --git a/pkg/struct/struct.go b/pkg/struct/struct.go
@@ -30,7 +30,7 @@ import (
 func MinFields(object pkg.Struct, n int) (bool, error) {
 	count := object.Len()
 	code := adt.EvalError
-	if object.IsOpen() {
+	if object.IsOpen() || count+object.NumConstraintFields() >= n {
 		code = adt.IncompleteError
 	}
 	if count < n {

diff --git a/pkg/struct/testdata/struct.txtar b/pkg/struct/testdata/struct.txtar
@@ -1,20 +1,34 @@
 -- in.cue --
 import "struct"
 
-minFields: {
+minFields1: {
 	[string]: struct.MinFields(1)
 
 	incomplete1: {}
+	optIncomplete: {a?: string}
+
 	fail1: close({})
-	failOptional1: close({a?: 1})
+	optCloseIncomplete: close({a?: 1})
 	failHidden1: close({_a: 1})
+
 	ok4: {_a: 1, a: 1}
 	ok1: {a: 1}
 	ok2: close({a: 1})
 	ok3: {a?: 1, a: 1}
 	ok5: {#a: int, a: #a & 1}
 }
 
+minFields2: {
+	[string]: struct.MinFields(2)
+
+	incomplete1: close({a?: string, b: 1})
+	incomplete2: close({a?: string, b?: int})
+	incomplete3: close({a?: string, b?: int, c: 1})
+	incomplete4: close({a?: string, b?: int, c?: int})
+
+	fail: close({a?: string})
+}
+
 maxFields: {
 	[string]: struct.MaxFields(1)
 
@@ -30,27 +44,32 @@ maxFields: {
 
 -- out/structs-v3 --
 Errors:
-minFields.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1):
-    ./in.cue:4:12
-    ./in.cue:4:29
-minFields.failHidden1: invalid value {_a:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1):
+minFields1.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1):
     ./in.cue:4:12
     ./in.cue:4:29
-minFields.failOptional1: invalid value {a?:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1):
+minFields1.failHidden1: invalid value {_a:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1):
     ./in.cue:4:12
     ./in.cue:4:29
+minFields2.fail: invalid value {a?:string} (does not satisfy struct.MinFields(2)): len(fields) < MinFields(2) (0 < 2):
+    ./in.cue:21:12
+    ./in.cue:21:29
 maxFields.fail1: invalid value {a:1,b:2} (does not satisfy struct.MaxFields(1)): len(fields) > MaxFields(1) (2 > 1):
-    ./in.cue:18:12
-    ./in.cue:18:29
+    ./in.cue:32:12
+    ./in.cue:32:29
 
 Result:
 import "struct"
 
-minFields: {
+minFields1: {
 	incomplete1: {} & struct.MinFields(1)
-	fail1:         _|_ // minFields.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
-	failOptional1: _|_ // minFields.failOptional1: invalid value {a?:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
-	failHidden1:   _|_ // minFields.failHidden1: invalid value {_a:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
+	optIncomplete: {
+		a?: string
+	} & struct.MinFields(1)
+	fail1: _|_ // minFields1.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
+	optCloseIncomplete: close({
+		a?: 1
+	}) & struct.MinFields(1)
+	failHidden1: _|_ // minFields1.failHidden1: invalid value {_a:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
 	ok4: {
 		a: 1
 	}
@@ -68,6 +87,27 @@ minFields: {
 		a:  1
 	}
 }
+minFields2: {
+	incomplete1: close({
+		a?: string
+		b:  1
+	}) & struct.MinFields(2)
+	incomplete2: close({
+		a?: string
+		b?: int
+	}) & struct.MinFields(2)
+	incomplete3: close({
+		a?: string
+		b?: int
+		c:  1
+	}) & struct.MinFields(2)
+	incomplete4: close({
+		a?: string
+		b?: int
+		c?: int
+	}) & struct.MinFields(2)
+	fail: _|_ // minFields2.fail: invalid value {a?:string} (does not satisfy struct.MinFields(2)): len(fields) < MinFields(2) (0 < 2)
+}
 maxFields: {
 	ok1: {}
 	ok2: {
@@ -89,74 +129,73 @@ maxFields: {
 	}
 	fail1: _|_ // maxFields.fail1: invalid value {a:1,b:2} (does not satisfy struct.MaxFields(1)): len(fields) > MaxFields(1) (2 > 1)
 }
--- diff/explanation --
-failOptional1: the new evaluator fails as expected, but the old evaluator doesn't -
-perhaps due to a bug in either the old evaluator or test code.
 -- diff/-out/structs-v3<==>+out/structs --
 diff old new
 --- old
 +++ new
 @@ -2,15 +2,15 @@
- minFields.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1):
+ minFields1.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1):
      ./in.cue:4:12
      ./in.cue:4:29
--    ./in.cue:7:9
- minFields.failHidden1: invalid value {_a:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1):
+-    ./in.cue:9:9
+ minFields1.failHidden1: invalid value {_a:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1):
      ./in.cue:4:12
      ./in.cue:4:29
--    ./in.cue:9:15
-+minFields.failOptional1: invalid value {a?:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1):
-+    ./in.cue:4:12
-+    ./in.cue:4:29
+-    ./in.cue:11:15
++minFields2.fail: invalid value {a?:string} (does not satisfy struct.MinFields(2)): len(fields) < MinFields(2) (0 < 2):
++    ./in.cue:21:12
++    ./in.cue:21:29
  maxFields.fail1: invalid value {a:1,b:2} (does not satisfy struct.MaxFields(1)): len(fields) > MaxFields(1) (2 > 1):
-     ./in.cue:18:12
-     ./in.cue:18:29
--    ./in.cue:27:9
+     ./in.cue:32:12
+     ./in.cue:32:29
+-    ./in.cue:41:9
 
  Result:
  import "struct"
-@@ -17,11 +17,9 @@
-
- minFields: {
- 	incomplete1: {} & struct.MinFields(1)
--	fail1: _|_ // minFields.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
--	failOptional1: close({
--		a?: 1
--	}) & struct.MinFields(1)
--	failHidden1: _|_ // minFields.failHidden1: invalid value {_a:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
-+	fail1:         _|_ // minFields.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
-+	failOptional1: _|_ // minFields.failOptional1: invalid value {a?:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
-+	failHidden1:   _|_ // minFields.failHidden1: invalid value {_a:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
- 	ok4: {
- 		a: 1
- 	}
+@@ -61,9 +61,7 @@
+ 		b?: int
+ 		c?: int
+ 	}) & struct.MinFields(2)
+-	fail: close({
+-		a?: string
+-	}) & struct.MinFields(2)
++	fail: _|_ // minFields2.fail: invalid value {a?:string} (does not satisfy struct.MinFields(2)): len(fields) < MinFields(2) (0 < 2)
+ }
+ maxFields: {
+ 	ok1: {}
 -- diff/todo/p2 --
 Missing error positions.
+-- diff/explanation --
+minFields1.fail1: the new evaluator fails as expected. It is more precise than
+the old evaluator.
 -- out/structs --
 Errors:
-minFields.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1):
+minFields1.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1):
     ./in.cue:4:12
     ./in.cue:4:29
-    ./in.cue:7:9
-minFields.failHidden1: invalid value {_a:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1):
+    ./in.cue:9:9
+minFields1.failHidden1: invalid value {_a:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1):
     ./in.cue:4:12
     ./in.cue:4:29
-    ./in.cue:9:15
+    ./in.cue:11:15
 maxFields.fail1: invalid value {a:1,b:2} (does not satisfy struct.MaxFields(1)): len(fields) > MaxFields(1) (2 > 1):
-    ./in.cue:18:12
-    ./in.cue:18:29
-    ./in.cue:27:9
+    ./in.cue:32:12
+    ./in.cue:32:29
+    ./in.cue:41:9
 
 Result:
 import "struct"
 
-minFields: {
+minFields1: {
 	incomplete1: {} & struct.MinFields(1)
-	fail1: _|_ // minFields.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
-	failOptional1: close({
+	optIncomplete: {
+		a?: string
+	} & struct.MinFields(1)
+	fail1: _|_ // minFields1.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
+	optCloseIncomplete: close({
 		a?: 1
 	}) & struct.MinFields(1)
-	failHidden1: _|_ // minFields.failHidden1: invalid value {_a:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
+	failHidden1: _|_ // minFields1.failHidden1: invalid value {_a:1} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
 	ok4: {
 		a: 1
 	}
@@ -174,6 +213,29 @@ minFields: {
 		a:  1
 	}
 }
+minFields2: {
+	incomplete1: close({
+		a?: string
+		b:  1
+	}) & struct.MinFields(2)
+	incomplete2: close({
+		a?: string
+		b?: int
+	}) & struct.MinFields(2)
+	incomplete3: close({
+		a?: string
+		b?: int
+		c:  1
+	}) & struct.MinFields(2)
+	incomplete4: close({
+		a?: string
+		b?: int
+		c?: int
+	}) & struct.MinFields(2)
+	fail: close({
+		a?: string
+	}) & struct.MinFields(2)
+}
 maxFields: {
 	ok1: {}
 	ok2: {