Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

evaluator: final/concrete evaluation does not eliminate unsatisfied required fields from disjunctions #3285

Open
myitcv opened this issue Jul 12, 2024 · 4 comments

Comments

@myitcv
Copy link
Member

myitcv commented Jul 12, 2024

What version of CUE are you using (cue version)?

$ cue version
cue version v0.0.0-20240712091910-3a379b7b9eca

go version go1.22.3
      -buildmode exe
       -compiler gc
  DefaultGODEBUG httplaxcontentlength=1,httpmuxgo121=1,tls10server=1,tlsrsakex=1,tlsunsafeekm=1
     CGO_ENABLED 1
          GOARCH arm64
            GOOS linux
             vcs git
    vcs.revision 3a379b7b9eca2f5d4747a4f3d68e4ee0cbf17a8d
        vcs.time 2024-07-12T09:19:10Z
    vcs.modified false
cue.lang.version v0.10.0

Does this issue reproduce with the latest release?

Yes

What did you do?

exec cue export x.cue
cmp stdout stdout.golden

-- x.cue --
#schema: ({
	a!: string
} | {
	a!: string
	b!: string
})

[string]: #schema

eg1: a: "a"
-- stdout.golden --
{
    "eg1": {
        "a": "a"
    }
}

What did you expect to see?

Passing test. Because I am running cue export, that should be sufficient indication to cmd/cue that there is no more values/data coming, and hence the second element in the disjunction can be eliminated as "in error" because the required field b is not satisfied.

What did you see instead?

> exec cue export x.cue
[stderr]
eg1: incomplete value {a:"a"} | {a:"a",b!:string}
[exit status 1]
FAIL: /tmp/testscript2379247909/repro.txtar/script.txtar:1: unexpected command failure

As similar sort of bug exists with the Go API, where passing cue.Final() or cue.Concrete(true) to a cue.Value.Validate call.

This bug follows discussion in #3165

@myitcv
Copy link
Member Author

myitcv commented Jul 13, 2024

An interesting follow-up to this issue.

What if the input were:

{b:"b"}

resulting in a value:

{a!:string} | {a!:string,b:"b"}

Would this eliminate both elements from the disjunction and return an error about an empty disjunction?

It's hard to see that we could do anything else. Despite there arguably being intent that the second element of the disjunction was trying to be selected, and the user simply missed off the a field.

@myitcv
Copy link
Member Author

myitcv commented Jul 15, 2024

One further observation regarding cue.Value.Subsume. We should double check that on fixing this bug it is the case that cue.Value.Subsume returns an error when appropriate. i.e. the following test should pass:

go mod tidy
go run .
cmp stdout stdout.golden
-- go.mod --
module mod.example

go 1.22.3

require cuelang.org/go v0.10.0-alpha.1.0.20240712164527-719893f23850

-- main.go --
package main

import (
	"fmt"

	"cuelang.org/go/cue"
	"cuelang.org/go/cue/cuecontext"
)

var doc = `
#schema: ({
 	a!: string
} | {
	a!: string
	b!: string
	c!: string
})

val: c: "c"
`

func main() {
	ctx := cuecontext.New()
	v := ctx.CompileString(doc)
	schema := v.LookupPath(cue.ParsePath("#schema"))
	val := v.LookupPath(cue.ParsePath("val"))

	res := val.Unify(schema)
	if err := res.Validate(cue.Final()); err != nil {
		fmt.Printf("error: %v\n", err)
	}
	if err := schema.Subsume(val); err != nil {
		fmt.Printf("subsume (no final): %v\n", err)
	}
	if err := schema.Subsume(val, cue.Final()); err != nil {
		fmt.Printf("subsume (final): %v\n", err)
	}
}
-- stdout.golden --
error: val.a: field is required but not present (and 1 more errors)
subsume (no final): field a not present in {c:"c"} (and 2 more errors)
subsume (final): field a not present in {c:"c"} (and 2 more errors)

Currently (as of 719893f) it fails with:

> go mod tidy
> go run .
[stdout]
error: val.a: field is required but not present (and 1 more errors)
subsume (no final): field a not present in {c:"c"} (and 2 more errors)

> cmp stdout stdout.golden
diff stdout stdout.golden
--- stdout
+++ stdout.golden
@@ -1,2 +1,3 @@
 error: val.a: field is required but not present (and 1 more errors)
 subsume (no final): field a not present in {c:"c"} (and 2 more errors)
+subsume (final): field a not present in {c:"c"} (and 2 more errors)

FAIL: /tmp/testscript3944689716/repro.txtar/script.txtar:3: stdout and stdout.golden differ

i.e. cue.Value.Subsume does not return an error when cue.Final is passed in.

@myitcv
Copy link
Member Author

myitcv commented Jul 15, 2024

Noting a further offline intuition/discussion, that if this logic applies to required fields then it has to also apply to regular fields (because required fields subsume regular fields). I will update this issue with a conclusion on that point.

@myitcv myitcv moved this from Backlog to v0.10.0-alpha.2 in Release v0.10 Jul 15, 2024
@myitcv myitcv moved this from v0.10.0-alpha.2 to Backlog in Release v0.10 Jul 17, 2024
@mvdan mvdan removed this from Release v0.10 Aug 1, 2024
@davidmdm
Copy link

davidmdm commented Oct 4, 2024

What is the status of this issue? I see it was moved out of release v0.10. Is a solution expected to land in a foreseeable release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants