cuelang / cue

CUE has moved to https://github.com/cue-lang/cue
https://cuelang.org
Apache License 2.0
3.09k stars 171 forks source link

cmd/cue: somewhat arbitrary eval output #1022

Closed dubo-dubon-duponey closed 3 years ago

dubo-dubon-duponey commented 3 years ago

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

$ cue version
cue version 0.4.0 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What did you do?

package foo

import (
        "strings"
)

#something: {
        image:    string | * "some default"
        // This function call seems to apply BEFORE the default are computed
        toString: strings.Join([image], "")
}

onething: #something & {
        image: "now this will work"
}

What did you expect to see?


import "strings"

#something: {
    image:    "some default"
    toString:    "some default"
}
onething: {
    image:    "now this will work"
    toString: "now this will work"
}

What did you see instead?

import "strings"

#something: {
    image:    "some default"
    toString: strings.Join([image], "")
}
onething: {
    image:    "now this will work"
    toString: "now this will work"
}
dubo-dubon-duponey commented 3 years ago

Vaguely reminiscent of #507 ?

dubo-dubon-duponey commented 3 years ago

This feels similar, but not sure if it's the same issue...

package foo

#nothing: {
        image: string
        thisWorks: image
        // This call applies TOO early, before image is actually set and this never succeeds
        if image != _|_ {
                thisDoesNotWork: image
        }
}

all: {
        none: #nothing,
        other: #nothing
}

for k, v in all {
        all: "\(k)": image: k
}
myitcv commented 3 years ago

Hi @dubo-dubon-duponey

#something: {
    image:    "some default"
    toString:    "some default"
}

Can you explain why this is your expectation? Is it consistency with the fact the image field has had its default selected? With eval we have this slightly weird beast which is a hybrid of actual CUE/not, somewhere between def and export. The difference with def in this case is that we are selecting the default, but then choosing not to evaluate the builtin. I don't know what the "right" answer is here. There is a plan to remove eval at some point (slightly ageing issue https://github.com/cuelang/cue/issues/337) and so as part of that we could consider what "right" means (for what will be the plain cue command).

Your second issue is more related to https://github.com/cuelang/cue/issues/990 AFAICT.

dubo-dubon-duponey commented 3 years ago

Hey Marcel ;)

Can you explain why this is your expectation?

The closest in my mental model would be:

image=${someVar:-default_value}
toString="$(dosomethingwithit "$someVar")"

I understand cue is not shell, but it's really hard to imagine why the default would only apply "after". It also feels to me like it's defeating the purpose of a default here.

Pretty much this is saying toString is evaluating to anything BUT the final, actual value of the variable it is referencing.

For anyone who is new to this, it's really hard to explain why:

#something: {
        image:    string | * "some default"
        if image != _|_ {
                status: image // "works"
        }
        toString: strings.Join([image], "") // nope, this operates on a different value of image
        another: image // "works"
}
dubo-dubon-duponey commented 3 years ago

Your second issue is more related to #990 AFAICT.

Ok, this second problem is also hurting quite a lot. Is this a bug in your opinion, or something that requires clarification / design?

Thanks Marcel for you time!

myitcv commented 3 years ago

Hey Marcel ;)

If only I was Marcel ;-)

Whilst looking at this I uncovered a bug, raised as https://github.com/cuelang/cue/issues/1025. That won't be helping our intuition here. This bug also explains the strange "in between" behaviour I described in https://github.com/cuelang/cue/issues/1022#issuecomment-850771121.

Perhaps that clears things up? Because side-stepping that bug via:

exec cue eval

-- x.cue --
package x

import "strings"

#something: {
    image: string | *"some default"
    if image != _|_ {
        status: image
    }

    toString: strings.TrimSpace(image)
    another:  image
}

x: #something
y: #something & {
    image: "ok"
}

gives:

#something: {
    image:    "some default"
    toString: "some default"
    status:   "some default"
    another:  "some default"
}
x: {
    image:    "some default"
    toString: "some default"
    status:   "some default"
    another:  "some default"
}
y: {
    image:    "ok"
    toString: "ok"
    status:   "ok"
    another:  "ok"
}

Is this a bug in your opinion, or something that requires clarification / design?

I will defer to the real Marcel here: https://github.com/cuelang/cue/issues/990#issuecomment-846421752

dubo-dubon-duponey commented 3 years ago

Hey Marcel ;)

If only I was Marcel ;-)

🤦🏾‍♂️

Sorry! Never do emails on a Saturday evening!

Let me start again: "thank you for your time Paul, and apologies for calling you names :-)"

1025 is exactly this problem I'm hitting here indeed (we can close this one if you prefer).

As for the other part (https://github.com/cuelang/cue/issues/1022#issuecomment-850647715), looking forward for the Real Slim Marcel to stand up :-))).

dubo-dubon-duponey commented 3 years ago

Another thing that ignores default values: disjunction of structs.

Maybe I just do not understand something fundamental about cue here.

So far (and as far as I understand this), disjunction of struct and builtins ignore default values, while everything else works as one would "intuitively" expect it (operators, interpolation, conditional fields).

(* this ^ is unrelated to eval vs. export btw)

myitcv commented 3 years ago

Another thing that ignores default values: disjunction of structs.

If you think there is an issue here, please do open a new issue because it's good to try and flush out these sorts of issues. Even if we conclude they're dupes/non-issues, the commentary is useful for others who might end up going down the same path (and conversely good material for explanations on why things are the way they are). But per your comment above I'll close this in favour of #1025.

cueckoo commented 3 years ago

This issue has been migrated to https://github.com/cue-lang/cue/issues/1022.

For more details about CUE's migration to a new home, please see https://github.com/cue-lang/cue/issues/1078.