Remove redundant fields from CallNode and BinaryNode (#504)

Author: antonmedv

ast/node.go

@@ -2,7 +2,6 @@ package ast
 
 import (
 	"reflect"
-	"regexp"
 
 	"github.com/expr-lang/expr/file"
 )
@@ -104,10 +103,9 @@ type UnaryNode struct {
 // BinaryNode represents a binary operator.
 type BinaryNode struct {
 	base
-	Operator string         // Operator of the binary operator. Like "+" in "foo + bar" or "matches" in "foo matches bar".
-	Left     Node           // Left node of the binary operator.
-	Right    Node           // Right node of the binary operator.
-	Regexp   *regexp.Regexp // Internal. Regexp of the "matches" operator. Like "f.+".
+	Operator string // Operator of the binary operator. Like "+" in "foo + bar" or "matches" in "foo matches bar".
+	Left     Node   // Left node of the binary operator.
+	Right    Node   // Right node of the binary operator.
 }
 
 // ChainNode represents an optional chaining group.
@@ -151,20 +149,17 @@ type SliceNode struct {
 // CallNode represents a function or a method call.
 type CallNode struct {
 	base
-	Callee    Node      // Node of the call. Like "foo" in "foo()".
-	Arguments []Node    // Arguments of the call.
-	Typed     int       // Internal. Used to indicate compiler what type is one of vm.FuncTypes.
-	Fast      bool      // Internal. Used to indicate compiler what this call is a fast call.
-	Func      *Function // Internal. Used to pass function information from type checker to compiler.
+	Callee    Node   // Node of the call. Like "foo" in "foo()".
+	Arguments []Node // Arguments of the call.
 }
 
 // BuiltinNode represents a builtin function call.
 type BuiltinNode struct {
 	base
 	Name      string // Name of the builtin function. Like "len" in "len(foo)".
 	Arguments []Node // Arguments of the builtin function.
-	Throws    bool   // Internal. If true then accessing a field or array index can throw an error. Used by optimizer.
-	Map       Node   // Internal. Used by optimizer to fold filter() and map() builtins.
+	Throws    bool   // If true then accessing a field or array index can throw an error. Used by optimizer.
+	Map       Node   // Used by optimizer to fold filter() and map() builtins.
 }
 
 // ClosureNode represents a predicate.

checker/checker.go

@@ -10,7 +10,6 @@ import (
 	"github.com/expr-lang/expr/conf"
 	"github.com/expr-lang/expr/file"
 	"github.com/expr-lang/expr/parser"
-	"github.com/expr-lang/expr/vm"
 )
 
 func Check(tree *parser.Tree, config *conf.Config) (t reflect.Type, err error) {
@@ -374,11 +373,10 @@ func (v *checker) BinaryNode(node *ast.BinaryNode) (reflect.Type, info) {
 
 	case "matches":
 		if s, ok := node.Right.(*ast.StringNode); ok {
-			r, err := regexp.Compile(s.Value)
+			_, err := regexp.Compile(s.Value)
 			if err != nil {
 				return v.error(node, err.Error())
 			}
-			node.Regexp = r
 		}
 		if isString(l) && isString(r) {
 			return boolType, info{}
@@ -549,7 +547,6 @@ func (v *checker) functionReturnType(node *ast.CallNode) (reflect.Type, info) {
 	fn, fnInfo := v.visit(node.Callee)
 
 	if fnInfo.fn != nil {
-		node.Func = fnInfo.fn
 		return v.checkFunction(fnInfo.fn, node, node.Arguments)
 	}
 
@@ -571,33 +568,13 @@ func (v *checker) functionReturnType(node *ast.CallNode) (reflect.Type, info) {
 	case reflect.Interface:
 		return anyType, info{}
 	case reflect.Func:
-		inputParamsCount := 1 // for functions
-		if fnInfo.method {
-			inputParamsCount = 2 // for methods
-		}
-		// TODO: Deprecate OpCallFast and move fn(...any) any to TypedFunc list.
-		// To do this we need add support for variadic arguments in OpCallTyped.
-		if !isAny(fn) &&
-			fn.IsVariadic() &&
-			fn.NumIn() == inputParamsCount &&
-			fn.NumOut() == 1 &&
-			fn.Out(0).Kind() == reflect.Interface {
-			rest := fn.In(fn.NumIn() - 1) // function has only one param for functions and two for methods
-			if kind(rest) == reflect.Slice && rest.Elem().Kind() == reflect.Interface {
-				node.Fast = true
-			}
-		}
-
 		outType, err := v.checkArguments(fnName, fn, fnInfo.method, node.Arguments, node)
 		if err != nil {
 			if v.err == nil {
 				v.err = err
 			}
 			return anyType, info{}
 		}
-
-		v.findTypedFunc(node, fn, fnInfo.method)
-
 		return outType, info{}
 	}
 	return v.error(node, "%v is not callable", fn)
@@ -883,7 +860,13 @@ func (v *checker) checkFunction(f *ast.Function, node ast.Node, arguments []ast.
 	return v.error(node, "no matching overload for %v", f.Name)
 }
 
-func (v *checker) checkArguments(name string, fn reflect.Type, method bool, arguments []ast.Node, node ast.Node) (reflect.Type, *file.Error) {
+func (v *checker) checkArguments(
+	name string,
+	fn reflect.Type,
+	method bool,
+	arguments []ast.Node,
+	node ast.Node,
+) (reflect.Type, *file.Error) {
 	if isAny(fn) {
 		return anyType, nil
 	}
@@ -1122,44 +1105,3 @@ func (v *checker) PairNode(node *ast.PairNode) (reflect.Type, info) {
 	v.visit(node.Value)
 	return nilType, info{}
 }
-
-func (v *checker) findTypedFunc(node *ast.CallNode, fn reflect.Type, method bool) {
-	// OnCallTyped doesn't work for functions with variadic arguments,
-	// and doesn't work named function, like `type MyFunc func() int`.
-	// In PkgPath() is an empty string, it's unnamed function.
-	if !fn.IsVariadic() && fn.PkgPath() == "" {
-		fnNumIn := fn.NumIn()
-		fnInOffset := 0
-		if method {
-			fnNumIn--
-			fnInOffset = 1
-		}
-	funcTypes:
-		for i := range vm.FuncTypes {
-			if i == 0 {
-				continue
-			}
-			typed := reflect.ValueOf(vm.FuncTypes[i]).Elem().Type()
-			if typed.Kind() != reflect.Func {
-				continue
-			}
-			if typed.NumOut() != fn.NumOut() {
-				continue
-			}
-			for j := 0; j < typed.NumOut(); j++ {
-				if typed.Out(j) != fn.Out(j) {
-					continue funcTypes
-				}
-			}
-			if typed.NumIn() != fnNumIn {
-				continue
-			}
-			for j := 0; j < typed.NumIn(); j++ {
-				if typed.In(j) != fn.In(j+fnInOffset) {
-					continue funcTypes
-				}
-			}
-			node.Typed = i
-		}
-	}
-}

checker/checker_test.go

@@ -554,6 +554,11 @@ unknown pointer #unknown (1:11)
 cannot use int as type string in array (1:4)
  | 42 in ["a", "b", "c"]
  | ...^
+
+"foo" matches "[+"
+error parsing regexp: missing closing ]: ` + "`[+`" + ` (1:7)
+ | "foo" matches "[+"
+ | ......^
 `
 
 func TestCheck_error(t *testing.T) {
@@ -777,49 +782,6 @@ func TestCheck_TypeWeights(t *testing.T) {
 	}
 }
 
-func TestCheck_CallFastTyped(t *testing.T) {
-	env := map[string]any{
-		"fn": func([]any, string) string {
-			return "foo"
-		},
-	}
-
-	tree, err := parser.Parse("fn([1, 2], 'bar')")
-	require.NoError(t, err)
-
-	_, err = checker.Check(tree, conf.New(env))
-	require.NoError(t, err)
-
-	require.False(t, tree.Node.(*ast.CallNode).Fast)
-	require.Equal(t, 22, tree.Node.(*ast.CallNode).Typed)
-}
-
-func TestCheck_CallFastTyped_Method(t *testing.T) {
-	env := mock.Env{}
-
-	tree, err := parser.Parse("FuncTyped('bar')")
-	require.NoError(t, err)
-
-	_, err = checker.Check(tree, conf.New(env))
-	require.NoError(t, err)
-
-	require.False(t, tree.Node.(*ast.CallNode).Fast)
-	require.Equal(t, 42, tree.Node.(*ast.CallNode).Typed)
-}
-
-func TestCheck_CallTyped_excludes_named_functions(t *testing.T) {
-	env := mock.Env{}
-
-	tree, err := parser.Parse("FuncNamed('bar')")
-	require.NoError(t, err)
-
-	_, err = checker.Check(tree, conf.New(env))
-	require.NoError(t, err)
-
-	require.False(t, tree.Node.(*ast.CallNode).Fast)
-	require.Equal(t, 0, tree.Node.(*ast.CallNode).Typed)
-}
-
 func TestCheck_works_with_nil_types(t *testing.T) {
 	env := map[string]any{
 		"null": nil,
@@ -908,8 +870,7 @@ func TestCheck_Function_types_are_checked(t *testing.T) {
 
 			_, err = checker.Check(tree, config)
 			require.NoError(t, err)
-			require.NotNil(t, tree.Node.(*ast.CallNode).Func)
-			require.Equal(t, "add", tree.Node.(*ast.CallNode).Func.Name)
+			require.Equal(t, reflect.Int, tree.Node.Type().Kind())
 		})
 	}
 
@@ -943,8 +904,7 @@ func TestCheck_Function_without_types(t *testing.T) {
 
 	_, err = checker.Check(tree, config)
 	require.NoError(t, err)
-	require.NotNil(t, tree.Node.(*ast.CallNode).Func)
-	require.Equal(t, "add", tree.Node.(*ast.CallNode).Func.Name)
+	require.Equal(t, reflect.Interface, tree.Node.Type().Kind())
 }
 
 func TestCheck_dont_panic_on_nil_arguments_for_builtins(t *testing.T) {

checker/info.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/expr-lang/expr/ast"
 	"github.com/expr-lang/expr/conf"
+	"github.com/expr-lang/expr/vm"
 )
 
 func FieldIndex(types conf.TypesTable, node ast.Node) (bool, []int, string) {
@@ -48,3 +49,80 @@ func MethodIndex(types conf.TypesTable, node ast.Node) (bool, int, string) {
 	}
 	return false, 0, ""
 }
+
+func TypedFuncIndex(fn reflect.Type, method bool) (int, bool) {
+	if fn == nil {
+		return 0, false
+	}
+	if fn.Kind() != reflect.Func {
+		return 0, false
+	}
+	// OnCallTyped doesn't work for functions with variadic arguments.
+	if fn.IsVariadic() {
+		return 0, false
+	}
+	// OnCallTyped doesn't work named function, like `type MyFunc func() int`.
+	if fn.PkgPath() != "" { // If PkgPath() is not empty, it means that function is named.
+		return 0, false
+	}
+
+	fnNumIn := fn.NumIn()
+	fnInOffset := 0
+	if method {
+		fnNumIn--
+		fnInOffset = 1
+	}
+
+funcTypes:
+	for i := range vm.FuncTypes {
+		if i == 0 {
+			continue
+		}
+		typed := reflect.ValueOf(vm.FuncTypes[i]).Elem().Type()
+		if typed.Kind() != reflect.Func {
+			continue
+		}
+		if typed.NumOut() != fn.NumOut() {
+			continue
+		}
+		for j := 0; j < typed.NumOut(); j++ {
+			if typed.Out(j) != fn.Out(j) {
+				continue funcTypes
+			}
+		}
+		if typed.NumIn() != fnNumIn {
+			continue
+		}
+		for j := 0; j < typed.NumIn(); j++ {
+			if typed.In(j) != fn.In(j+fnInOffset) {
+				continue funcTypes
+			}
+		}
+		return i, true
+	}
+	return 0, false
+}
+
+func IsFastFunc(fn reflect.Type, method bool) bool {
+	if fn == nil {
+		return false
+	}
+	if fn.Kind() != reflect.Func {
+		return false
+	}
+	numIn := 1
+	if method {
+		numIn = 2
+	}
+	if !isAny(fn) &&
+		fn.IsVariadic() &&
+		fn.NumIn() == numIn &&
+		fn.NumOut() == 1 &&
+		fn.Out(0).Kind() == reflect.Interface {
+		rest := fn.In(fn.NumIn() - 1) // function has only one param for functions and two for methods
+		if kind(rest) == reflect.Slice && rest.Elem().Kind() == reflect.Interface {
+			return true
+		}
+	}
+	return false
+}

checker/info_test.go

@@ -0,0 +1,27 @@
+package checker_test
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/expr-lang/expr/checker"
+	"github.com/expr-lang/expr/test/mock"
+)
+
+func TestTypedFuncIndex(t *testing.T) {
+	fn := func([]any, string) string {
+		return "foo"
+	}
+	index, ok := checker.TypedFuncIndex(reflect.TypeOf(fn), false)
+	require.True(t, ok)
+	require.Equal(t, 22, index)
+}
+
+func TestTypedFuncIndex_excludes_named_functions(t *testing.T) {
+	var fn mock.MyFunc
+
+	_, ok := checker.TypedFuncIndex(reflect.TypeOf(fn), false)
+	require.False(t, ok)
+}