internal/pkg: make Len ignore optional fields

The Struct.Len function incorrectly includes optional fields in
the total field count. Therefore, `{a?: 1, b?: 2}` is considered
to have 2 fields instead of 0.
This causes the `struct.MinFields` and `struct.MaxFields` to
return incorrect results.

To fix this, we now exclude optional fields from the sum.

Fixes #2749.

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

Author: NoamTD

internal/pkg/types.go

@@ -50,7 +50,7 @@ func (s *Struct) Arcs() []*adt.Vertex {
 func (s *Struct) Len() int {
 	count := 0
 	for _, a := range s.Arcs() {
-		if a.Label.IsString() {
+		if a.Label.IsString() && !s.node.IsOptional(a.Label) {
 			count++
 		}
 	}

pkg/struct/testdata/struct.txtar

@@ -36,23 +36,21 @@ minFields.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fi
 minFields.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
+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
 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
-maxFields.ok4: 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
 
 Result:
 import "struct"
 
 minFields: {
 	incomplete1: {} & struct.MinFields(1)
-	fail1: _|_ // minFields.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
-	failOptional1: {
-		a?: 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
 	}
@@ -78,7 +76,10 @@ maxFields: {
 	ok3: {
 		a: 1
 	}
-	ok4: _|_ // maxFields.ok4: invalid value {a?:1,b?:2} (does not satisfy struct.MaxFields(1)): len(fields) > MaxFields(1) (2 > 1)
+	ok4: {
+		a?: 1
+		b?: 2
+	}
 	ok5: {
 		a: 1
 	}
@@ -88,11 +89,14 @@ 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,19 +2,15 @@
+@@ -2,15 +2,15 @@
  minFields.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1):
      ./in.cue:4:12
      ./in.cue:4:29
@@ -101,17 +105,31 @@ diff old new
      ./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
  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
- maxFields.ok4: 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:23:7
 
  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
+ 	}
 -- diff/todo/p2 --
 Missing error positions.
 -- out/structs --
@@ -128,20 +146,16 @@ maxFields.fail1: invalid value {a:1,b:2} (does not satisfy struct.MaxFields(1)):
     ./in.cue:18:12
     ./in.cue:18:29
     ./in.cue:27:9
-maxFields.ok4: 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:23:7
 
 Result:
 import "struct"
 
 minFields: {
 	incomplete1: {} & struct.MinFields(1)
 	fail1: _|_ // minFields.fail1: invalid value {} (does not satisfy struct.MinFields(1)): len(fields) < MinFields(1) (0 < 1)
-	failOptional1: {
+	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)
 	ok4: {
 		a: 1
@@ -168,7 +182,10 @@ maxFields: {
 	ok3: {
 		a: 1
 	}
-	ok4: _|_ // maxFields.ok4: invalid value {a?:1,b?:2} (does not satisfy struct.MaxFields(1)): len(fields) > MaxFields(1) (2 > 1)
+	ok4: {
+		a?: 1
+		b?: 2
+	}
 	ok5: {
 		a: 1
 	}