Comparison with named integer type fails

[mbertschler]

Hey @antonmedv. Thanks for this amazing library, it is a lot of fun to work with.

After deciding to use it for a new feature with dynamic configuration, we unfortunately ran into some unexpected behavior. The problem is that the expression Named == 4 returns false with this environment definition:

type NamedUint uint32
type Env struct {
    Named NamedUint
}
env := &Env{Named: 4}

It seems to be because we use a named type NamedUint instead of the basic type uint32 in our struct.

Is this behavior expected?

Reproducing Test

Version: github.com/expr-lang/expr v1.16.1

func TestExprUint(t *testing.T) {
	type NamedUint uint32

	type Env struct {
		Plain uint32
		Named NamedUint
	}

	testEnv := &Env{
		Plain: 3,
		Named: 4,
	}

	evaluateCode := func(t *testing.T, code string, expected interface{}) {
		program, err := expr.Compile(code, expr.Env(&Env{}))
		if err != nil {
			t.Error("Compile", err)
		}

		output, err := expr.Run(program, testEnv)
		if err != nil {
			t.Error("Run", err)
		}
		formatted := fmt.Sprintf("%v", output)
		if formatted != fmt.Sprintf("%v", expected) {
			t.Errorf("expected %v, got %v. Code: %q Env: %+v", expected, output, code, testEnv)
		}
	}

	t.Run("Plain return", func(t *testing.T) {
		code := `Plain`
		evaluateCode(t, code, 3)
	})

	t.Run("Named return", func(t *testing.T) {
		code := `Named`
		evaluateCode(t, code, 4)
	})

	t.Run("Plain equal", func(t *testing.T) {
		code := `Plain == 3`
		evaluateCode(t, code, true)
	})

	// --- FAIL: TestExprUint (0.00s)
	//     --- FAIL: TestExprUint/Named_equal (0.00s)
	//         skan_assignment_test.go:394: expected true, got false. Code: "Named == 4" Env: &{Plain:3 Named:4}
	t.Run("Named equal", func(t *testing.T) {
		code := `Named == 4`
		evaluateCode(t, code, true)
	})
}

[antonmedv]

Hey @mbertschler, thanks!

I understand this problem. This is actually a Golang related one:

https://go.dev/play/p/nYw6xmQ9Ll2

package main

import (
	"fmt"
	"reflect"
)

type MyInt int

func main() {
	var my MyInt = 1
	var i int = 1
	fmt.Println(my == i) // Compilation will fail.
	fmt.Println(reflect.DeepEqual(my, i)) // Will return false.
}

Recently in #611 we improved int() builtin function to unwrap custom int types:

int(Named) == 4 // This will work in Expr.

One thing you can do is to write a patcher which will wrap all custom ints with int(...) call.

[mbertschler]

In the end I solved this with a patcher and a custom wrapper function, that can deal with more than just int types.

// unpackBasicTypesPatcher is a patcher that unwraps named types into their basic values.
// This is required because the following code will not work:
//
//	type namedInt int
//	var x namedInt = 42
//
// Running this code: "x == 42" will return false.
// To make it work, we need to unwrap the named type into its basic value.
// For this we patch the AST to change any node that has a named basic type and
// wrap it with a call to the unwrap() function that we also make available to expr.
//
//	type of x is "bidder.namedInt"
//	type of unwrap(x) is "int"
type unpackBasicTypesPatcher struct{}

func (unpackBasicTypesPatcher) Visit(node *ast.Node) {
	typ := (*node).Type()

	// typ can be nil, if the node is an ast.NilNode.
	// example code: "InventorySplit != nil"
	if typ == nil {
		return
	}

	switch typ.Kind() {

	// all the basic types that can be unwrapped:
	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
		reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64,
		reflect.Float32, reflect.Float64,
		reflect.String:

		// Check if the kind is the same as the type name.
		// Examples:
		//     type namedInt int
		//     basic type:  "int" == "int"
		//     named type:  "int" == "bidder.namedInt"
		if typ.Kind().String() == typ.Name() {
			// type is basic, doesn't have to be wrapped
			return
		}

		// wrap the named type in our special unpack function
		wrapper := ast.CallNode{
			Callee:    &ast.IdentifierNode{Value: "unpack"},
			Arguments: []ast.Node{*node},
		}
		ast.Patch(node, &wrapper)
	}
}

// unpack is a wrapper that adapts the unpackBasicType function to the expr library.
func unpack(params ...any) (any, error) {
	if len(params) == 0 {
		return nil, errors.New("no params")
	}
	return unpackBasicType(params[0]), nil
}

// unpackBasicType unwraps any named basic type into a value of the basic type.
func unpackBasicType(in any) any {
	v := reflect.ValueOf(in)

	// return the basic type value based on the underlying type Kind
	switch v.Kind() {
	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
		return v.Int()
	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
		return v.Uint()
	case reflect.Float32, reflect.Float64:
		return v.Float()
	case reflect.Bool:
		return v.Bool()
	case reflect.String:
		return v.String()
	}

	// can't unpack this type
	return in
}

Test example:

package example

import (
	"testing"

	"github.com/expr-lang/expr"
	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
)

func TestExprNamedTypeUnpacking(t *testing.T) {
	type namedIntType int16
	type namedFloatType float32
	type namedStringType string

	env := map[string]any{
		"namedFloat":  func() namedFloatType { return 3.0 },
		"namedInt":    func() namedIntType { return 42 },
		"namedString": func() namedStringType { return "abc" },
		"basicFloat":  func() float32 { return 3.0 },
		"basicInt":    func() int { return 42 },
		"basicString": func() string { return "abc" },
	}

	cases := []string{
		`namedFloat() == 3.0`,
		`namedInt() == 42`,
		`namedString() == "abc"`,
		`basicFloat() == 3.0`,
		`basicInt() == 42`,
		`basicString() == "abc"`,
	}

	options := []expr.Option{
		expr.Env(env),
		expr.Function("unpack", unpack),
		expr.Patch(unpackBasicTypesPatcher{}),
	}

	for _, code := range cases {
		t.Run(code, func(t *testing.T) {
			program, err := expr.Compile(code, options...)
			require.NoError(t, err)

			val, err := expr.Run(program, env)
			require.NoError(t, err)
			result, ok := val.(bool)
			require.True(t, ok)

			assert.True(t, result)
		})
	}
}

While this solves my use case without any more concerns, I wonder if this could become a built in option in the future that could also be handled during compile time?

[antonmedv]

Cool patcher! We can add unpackBasicTypesPatcher as an extension in Expr repo.

could also be handled during compile time?

What do you mean by that? This is already handled at compile time by your patcher. Patcher wraps all needed vars into unwrap() function during compilation. This unwrapping should be done somewhere all the time. Even in Go code.