Skip to content

Commit

Permalink
internal/core/validate: disallow incomplete errors in "Final" mode
Browse files Browse the repository at this point in the history
Previously "Final" was only used for disallowing
unsatisfied required fields. However, this issue is
more general. We now disallow any incomplete
errors in final mode that are not within definitions.

Fixes #3651

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: I13eb9126c03c7e62b37068c74aa8db6f97c496d8
Dispatch-Trailer: {"type":"trybot","CL":1206950,"patchset":1,"ref":"refs/changes/50/1206950/1","targetBranch":"master"}
  • Loading branch information
mpvl authored and cueckoo committed Jan 9, 2025
1 parent e38ed07 commit 34c9ae2
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 16 deletions.
11 changes: 9 additions & 2 deletions cmd/cue/cmd/testdata/script/dev.txtar
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
exec cue eval
cmp stdout outv2
env CUE_EXPERIMENT=evalv3=0
! exec cue eval
cmp stderr outv2

env CUE_EXPERIMENT=evalv3
exec cue eval
Expand All @@ -15,3 +16,9 @@ y
-- outv3 --
y: 1
-- outv2 --
2 errors in empty disjunction:
conflicting values 1 and {y:(1|{y:1}),y} (mismatched types int and struct):
./test.cue:1:1
./test.cue:4:4
cannot add field y: was already used:
./test.cue:4:9
4 changes: 2 additions & 2 deletions cmd/cue/cmd/testdata/script/get_go_non_local.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ language: version: "v0.9.0"
-- uses_imports_root.cue --
package p

import "externalmod.test/imports/root"
import "externalmod.test/blah"

root.Type
blah.Type

-- go.mod --
module mod.test/local
Expand Down
6 changes: 4 additions & 2 deletions cmd/cue/cmd/testdata/script/pkg_patterns.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ stdout -count=1 '^self: "mod.com/alpha1/leaf"$'

# A wildcard in the middle of a pattern, with the current module as a prefix
# either as "dot" or as the full path.
# TODO: Why does this succeed? What does the output mean? This seems broken.
exec cue eval ./.../leaf
! exec cue eval ./.../leaf
cmp stderr dot-dots-middle.stderr

# TODO: Unlike other cases, this still returns a "matched no packages" error.
! exec cue eval mod.com/.../leaf
cmp stderr current-dots-middle.stderr
Expand All @@ -61,6 +61,8 @@ pattern not allowed in external package path "..."
pattern not allowed in external package path ".../leaf"
-- dot-dots-middle.stderr --
// mod.com@v0:root
undefined field: bar:
./root.cue:5:5
-- current-dots-middle.stderr --
cue: "mod.com/.../leaf" matched no packages
-- external-dots-middle.stderr --
Expand Down
20 changes: 12 additions & 8 deletions cue/testdata/cycle/builtins.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,11 @@ Result:
}
issue3633: (struct){
final: (struct){
data: (#struct){
data: (_|_){
// [incomplete] issue3633.final.data: invalid value {} (does not satisfy matchN): 0 matched, expected 1:
// ./matchn.cue:63:6
// ./matchn.cue:62:8
// ./matchn.cue:63:13
}
#s: (_){ matchN(1, (#list){
0: (_|_){// &[matchN(1, [
Expand Down Expand Up @@ -497,12 +501,8 @@ diff old new
n: (struct){
n: (_){ _ }
}
@@ -167,14 +179,14 @@
}
issue3633: (struct){
final: (struct){
- data: (struct){
+ data: (#struct){
@@ -174,11 +186,11 @@
// ./matchn.cue:63:13
}
#s: (_){ matchN(1, (#list){
- 0: (_|_){// matchN(1, [
Expand Down Expand Up @@ -687,7 +687,11 @@ Result:
}
issue3633: (struct){
final: (struct){
data: (struct){
data: (_|_){
// [incomplete] issue3633.final.data: invalid value {} (does not satisfy matchN): 0 matched, expected 1:
// ./matchn.cue:63:6
// ./matchn.cue:62:8
// ./matchn.cue:63:13
}
#s: (_){ matchN(1, (#list){
0: (_|_){// matchN(1, [
Expand Down
8 changes: 6 additions & 2 deletions internal/core/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ func (v *validator) checkConcrete() bool {
return v.Concrete && v.inDefinition == 0
}

func (v *validator) checkFinal() bool {
return (v.Concrete || v.Final) && v.inDefinition == 0
}

func (v *validator) add(b *adt.Bottom) {
if !v.AllErrors {
v.err = adt.CombineErrors(nil, v.err, b)
Expand All @@ -77,12 +81,12 @@ func (v *validator) validate(x *adt.Vertex) {
if b := x.Bottom(); b != nil {
switch b.Code {
case adt.CycleError:
if v.checkConcrete() || v.DisallowCycles {
if v.checkFinal() || v.DisallowCycles {
v.add(b)
}

case adt.IncompleteError:
if v.checkConcrete() {
if v.checkFinal() {
v.add(b)
}

Expand Down
15 changes: 15 additions & 0 deletions internal/core/validate/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,21 @@ y: conflicting values 4 and 2:
x: string | {foo!: string}
`,
out: "incomplete\nx.foo: field is required but not present:\n test:3:18",
}, {
name: "disallow incomplete error with final",
cfg: &validate.Config{Final: true},
in: `
x: y + 1
y: int
`,
out: "incomplete\nx: non-concrete value int in operand to +:\n test:2:7\n test:3:7",
}, {
name: "allow incomplete error with final while in definition",
cfg: &validate.Config{Final: true},
in: `
#D: x: #D.y + 1
#D: y: int
`,
}}

cuetdtest.Run(t, testCases, func(t *cuetdtest.T, tc *testCase) {
Expand Down

0 comments on commit 34c9ae2

Please sign in to comment.