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

Change in behavior in 1.15 with in and float64 #426

Closed
zachaller opened this issue Sep 1, 2023 · 4 comments
Closed

Change in behavior in 1.15 with in and float64 #426

zachaller opened this issue Sep 1, 2023 · 4 comments
Labels

Comments

@zachaller
Copy link

zachaller commented Sep 1, 2023

In argo rollouts we had our tests starting to fail on 1.15 it seems that we needed to be explicit in converting to floats which we did not have to before here are two examples.

1.14.0: https://goplay.tools/snippet/gaA2hxV9auw

        env := map[string]interface{}{
		"res": []float64{10, 11},
	}
	code := "10 in res"

	program, err := expr.Compile(code, expr.Env(env))
	fmt.Println(err)

        //output: true

1.15.0: https://goplay.tools/snippet/rZsOUyfRRop

        env := map[string]interface{}{
		"res": []float64{10, 11},
	}
	code := "10 in res"

	program, err := expr.Compile(code, expr.Env(env))
	fmt.Println(err)

        //output: cannot use int as type float64 in array

Working in 1.15.0 with explicit casting: https://goplay.tools/snippet/XBqyWpRfj6I

        env := map[string]interface{}{
		"res": []float64{10, 11},
	}
	code := "float(10) in res"

	program, err := expr.Compile(code, expr.Env(env))
	fmt.Println(err)

        //output: true

Is this expected behavior?

@antonmedv
Copy link
Member

I see. The type checker got more strict, looks like in this case, we can bring back int->float conversion.

@antonmedv
Copy link
Member

@zachaller Also what about adding Argo's expressions test corpus to directly to expr?

For example, here are expression tests for CrowdSec:

This way we can prevent such regressions in the future.

@zachaller
Copy link
Author

@zachaller Also what about adding Argo's expressions test corpus to directly to expr?

For example, here are expression tests for CrowdSec:

This way we can prevent such regressions in the future.

Yea, I will spend some time looking into this and opening a PR!

@antonmedv
Copy link
Member

Fixed. Let's add Argo expression corpus for repression testing 😉

antonmedv added a commit to anto-lang/anto that referenced this issue Sep 20, 2023
antonmedv added a commit to anto-lang/anto that referenced this issue Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants