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

pkg/math: MultipleOf can't be used as a validator #2927

Closed
jpluscplusm opened this issue Mar 7, 2024 · 1 comment
Closed

pkg/math: MultipleOf can't be used as a validator #2927

jpluscplusm opened this issue Mar 7, 2024 · 1 comment

Comments

@jpluscplusm
Copy link
Collaborator

jpluscplusm commented Mar 7, 2024

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

$ cue version
cue version v0.8.0-alpha.5

go version go1.22.0
      -buildmode exe
       -compiler gc
       -trimpath true
     CGO_ENABLED 0
          GOARCH amd64
            GOOS linux
         GOAMD64 v1

Does this issue reproduce with the latest stable release?

Yes, 0.7.0

What did you do?

exec cue vet file.cue
-- file.cue --
import "math"

x: 10
x: math.MultipleOf(2)

I tried to use math.MultipleOf as a validator, which I believe should work given

  1. its (bool, error) return type
  2. other multi-parameter functions work as validators, e.g. https://alpha.cuelang.org/docs/howto/use-list-contains-as-a-field-validator/

What did you expect to see?

A passing test. Note that the values chosen, 10 and 2, are not affected by #2926.

What did you see instead?

> exec cue vet file.cue
[stderr]
x: invalid value 10 (does not satisfy math.MultipleOf(2)): error in call to math.MultipleOf: interface conversion: adt.Value is *adt.Vertex, not *adt.Num:
    ./file.cue:4:4
    ./file.cue:3:4
    ./file.cue:4:20
[exit status 1]
FAIL: /tmp/testscript1312268940/math.MultipleOf.doesntWorkAsValidator.txtar/script.txtar:1: unexpected command failure
@jpluscplusm jpluscplusm added NeedsInvestigation Triage Requires triage/attention labels Mar 7, 2024
@mogsie
Copy link

mogsie commented Mar 8, 2024

In addition, the following relatively natural construct:

import "math"

foo: 3
bar: 9 & math.MultipleOf(foo)

fails with:

bar: invalid value 9 (does not satisfy math.MultipleOf(3)): error in call to math.MultipleOf: interface conversion: adt.Value is *adt.Vertex, not *adt.Num:

I guess it's the same error, but might be worth to include in a test case.

cueckoo pushed a commit that referenced this issue Apr 19, 2024
The hiddenValue.Decimal() method currently makes the assumption that
its function argument is an *adt.Num. This means that the "Validator"
form of the builtin math.MultipleOf(n) will not work, because in that
case the first argument will be an *adt.Vertex.

This changes the explicit interface convertion to instead use the
value.Decimal() method, which will work in all cases.

Fixes #2927

Signed-off-by: Noam Dolovich <noam.tzvi.dolovich@gmail.com>
Change-Id: I35f780efde92a6523548ed04e1e7a6a960b191ce
cueckoo pushed a commit that referenced this issue Apr 19, 2024
The hiddenValue.Decimal() method currently makes the assumption that
its function argument is an *adt.Num. This means that the "Validator"
form of the builtin math.MultipleOf(n) will not work, because in that
case the first argument will be an *adt.Vertex.

This changes the explicit interface convertion to instead use the
value.Decimal() method, which will work in all cases.

Fixes #2927

Signed-off-by: Noam Dolovich <noam.tzvi.dolovich@gmail.com>
Change-Id: I35f780efde92a6523548ed04e1e7a6a960b191ce
@mvdan mvdan added NeedsFix and removed Triage Requires triage/attention labels Apr 19, 2024
cueckoo pushed a commit that referenced this issue Apr 19, 2024
The cue.Value.Decimal method currently makes the assumption that
its function argument is an *adt.Num. This means that the "Validator"
form of the builtin math.MultipleOf(n) will not work, because in that
case the first argument will be an *adt.Vertex.

This changes the explicit interface convertion to instead use the
cue.value.Decimal method, which will work in all cases.

Fixes #2927

Signed-off-by: Noam Dolovich <noam.tzvi.dolovich@gmail.com>
Change-Id: I35f780efde92a6523548ed04e1e7a6a960b191ce
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