From 44cceca806909f9e24292770ee8381b5138ea2bf Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Tue, 8 Oct 2024 12:49:07 +0200 Subject: [PATCH] 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 Change-Id: I2eed630a9f4f2240ed5ec7909bf49f85986d1f51 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202299 Unity-Result: CUE porcuepine TryBot-Result: CUEcueckoo Reviewed-by: Matthew Sackman --- internal/pkg/types.go | 14 ++- pkg/struct/struct.go | 2 +- pkg/struct/testdata/struct.txtar | 166 +++++++++++++++++++++---------- 3 files changed, 128 insertions(+), 54 deletions(-) diff --git a/internal/pkg/types.go b/internal/pkg/types.go index e7e3bfb62..801971bcc 100644 --- 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 index 39e2cda40..a56848886 100644 --- 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 index 9c89f9614..bf8975307 100644 --- a/pkg/struct/testdata/struct.txtar +++ b/pkg/struct/testdata/struct.txtar @@ -1,13 +1,16 @@ -- 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}) @@ -15,6 +18,17 @@ minFields: { 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: { -- 2.51.2