cue-lang / cue

The home of the CUE language! Validate and define text-based and dynamic configuration
https://cuelang.org
Apache License 2.0
5.02k stars 287 forks source link

evaluator: order of disjunction with default case seems to affect if comprehensions #2354

Open prahaladramji opened 1 year ago

prahaladramji commented 1 year ago

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

cue version 0.5.0

go version go1.20.3
      -buildmode exe
       -compiler gc
       -trimpath true
     CGO_ENABLED 1
          GOARCH arm64
            GOOS darwin

Does this issue reproduce with the latest stable release?

Yes

What did you do?

Given the below input, I would like to conditionally add the source.path field only if the source type _type is helm. However as you can see in the cue playground, even though X._type resolves to the right string inside the object the condition is incorrectly always matching helm.

I've also noticed that if you change the order of #ApplicationSourceType then it switches to match what's the first element, it’s like the order of this decides what the X._type is during the condition evaluation.

Application: X={
    spec: #ArgoApplicationSpec

    argoApplication: _Application & {_X: {
        spec:     X.spec
    }}
}

#ArgoApplicationSpec: source:  X={
    #ApplicationSourceType

    repoURL: string | *"https://github.com/foo/bar.git"
    targetRevision: string | *"main"

    // This has no meaning for Helm charts pulled directly from a Helm repo instead of git.
    if X._type != "helm" {
        path: *X._type | string
    }
}

#ApplicationSourceType:
    #SourceHelm |
    *#SourcePlugin

#SourceHelm: {
    _type: "helm"

    // helm specific config
    chart?: string

    helm: {
        // Values file as block file
        values?: string
    }
}

#SourcePlugin: {
    _type: "plugin"

    plugin: name?: string
}

_Application: {
    _X: _

    apiVersion: "argoproj.io/v1alpha1"
    kind:       "Application"

    metadata: namespace: "argocd"

    spec: source: _X.spec.source
}

What did you expect to see?

// if _type == "plugin"
// snip ...
source: {
  repoURL:        "https://github.com/foo/bar.git"
  targetRevision: "main"
  path: "plugin"
  plugin: {}
}

// snip ...

// if _type == "helm"
source: {
  repoURL:        "https://github.com/foo/bar.git"
  targetRevision: "main"
  helm: {}
}

What did you see instead?

// if _type == "plugin"
// snip ...
source: {
  repoURL:        "https://github.com/foo/bar.git"
  targetRevision: "main"
  path: "plugin"
  plugin: {}
}

// snip ...

// if _type == "helm"
source: {
  repoURL:        "https://github.com/foo/bar.git"
  targetRevision: "main"
  path: "helm"  // this shouldn't exist. 
  helm: {}
}
mvdan commented 1 year ago

Reduced this to ten lines of CUE. See the testscript below:

exec cue export foo_then_bar.cue
cmp stdout stdout.golden

exec cue export bar_then_foo.cue
cmp stdout stdout.golden

-- foo_then_bar.cue --
type: _
{type: "foo"} | *{type: "bar"}
// *{type: "bar"} | {type: "foo"}

if type == "foo" {
    foo: true
}
if type == "bar" {
    bar: true
}
-- bar_then_foo.cue --
type: _
// {type: "foo"} | *{type: "bar"}
*{type: "bar"} | {type: "foo"}

if type == "foo" {
    foo: true
}
if type == "bar" {
    bar: true
}
-- stdout.golden --
{
    "type": "bar",
    "bar": true
}

Running it with v0.5.0, v0.6.0-alpha.1, or master (59080b65a5ba539bb3f3cb45d3346f39df67cf01) shows different behavior depending on the order of the disjunction:

$ testscript -v -continue repro.txtar
> exec cue export foo_then_bar.cue
[stdout]
{
    "type": "bar",
    "foo": true
}
> cmp stdout stdout.golden
diff stdout stdout.golden
--- stdout
+++ stdout.golden
@@ -1,4 +1,4 @@
 {
     "type": "bar",
-    "foo": true
+    "bar": true
 }

FAIL: /tmp/testscript1478993161/repro.txtar/script.txtar:2: stdout and stdout.golden differ
> exec cue export bar_then_foo.cue
[stdout]
{
    "type": "bar",
    "bar": true
}
> cmp stdout stdout.golden
error running repro.txtar in /tmp/testscript1478993161/repro.txtar

Note how *{type: "bar"} | {type: "foo"} gives the expected output, but {type: "foo"} | *{type: "bar"}, which should be equivalent, gives the wrong output. type remains at bar which is correct, but the conditional field is foo, which is incorrect.

Worth noting that this is technically not a regression from v0.4.3; that version failed with incomplete cause disjunction.

This bug reminds me of https://github.com/cue-lang/cue/issues/2209 slightly; in that case, removing a disjunction in a definition changed the output in an unexpected and buggy way. However, that case worked on v0.4.3, so I don't think it's a duplicate.