proc: better error messages for ambiguous function calls/type casts (#2903)

Try to produce better error messages when we can't distinguish between
a function call and a type cast.

Fixes #2902

pkg/proc/eval.go

@@ -745,13 +745,7 @@ func (scope *EvalScope) evalToplevelTypeCast(t ast.Expr, cfg LoadConfig) (*Varia
 func (scope *EvalScope) evalAST(t ast.Expr) (*Variable, error) {
 	switch node := t.(type) {
 	case *ast.CallExpr:
-		if len(node.Args) == 1 {
-			v, err := scope.evalTypeCast(node)
-			if err == nil || err != reader.ErrTypeNotFound {
-				return v, err
-			}
-		}
-		return evalFunctionCall(scope, node)
+		return scope.evalTypeCastOrFuncCall(node)
 
 	case *ast.Ident:
 		return scope.evalIdent(node)
@@ -850,6 +844,70 @@ func removeParen(n ast.Expr) ast.Expr {
 	return n
 }
 
+// evalTypeCastOrFuncCall evaluates a type cast or a function call
+func (scope *EvalScope) evalTypeCastOrFuncCall(node *ast.CallExpr) (*Variable, error) {
+	if len(node.Args) != 1 {
+		// Things that have more or less than one argument are always function calls.
+		return evalFunctionCall(scope, node)
+	}
+
+	ambiguous := func() (*Variable, error) {
+		// Ambiguous, could be a function call or a type cast, if node.Fun can be
+		// evaluated then try to treat it as a function call, otherwise try the
+		// type cast.
+		_, err0 := scope.evalAST(node.Fun)
+		if err0 == nil {
+			return evalFunctionCall(scope, node)
+		}
+		v, err := scope.evalTypeCast(node)
+		if err == reader.ErrTypeNotFound {
+			return nil, fmt.Errorf("could not evaluate function or type %s: %v", exprToString(node.Fun), err0)
+		}
+		return v, err
+	}
+
+	fnnode := removeParen(node.Fun)
+	if n, _ := fnnode.(*ast.StarExpr); n != nil {
+		fnnode = removeParen(n.X)
+	}
+
+	switch n := fnnode.(type) {
+	case *ast.BasicLit:
+		// It can only be a ("type string")(x) type cast
+		return scope.evalTypeCast(node)
+	case *ast.ArrayType, *ast.StructType, *ast.FuncType, *ast.InterfaceType, *ast.MapType, *ast.ChanType:
+		return scope.evalTypeCast(node)
+	case *ast.SelectorExpr:
+		if _, isident := n.X.(*ast.Ident); isident {
+			return ambiguous()
+		}
+		return evalFunctionCall(scope, node)
+	case *ast.Ident:
+		if supportedBuiltins[n.Name] {
+			return evalFunctionCall(scope, node)
+		}
+		return ambiguous()
+	case *ast.IndexExpr:
+		// Ambiguous, could be a parametric type
+		switch n.X.(type) {
+		case *ast.Ident, *ast.SelectorExpr:
+			// Do the type-cast first since evaluating node.Fun could be expensive.
+			v, err := scope.evalTypeCast(node)
+			if err == nil || err != reader.ErrTypeNotFound {
+				return v, err
+			}
+			return evalFunctionCall(scope, node)
+		default:
+			return evalFunctionCall(scope, node)
+		}
+	case *astIndexListExpr:
+		return scope.evalTypeCast(node)
+	default:
+		// All other expressions must be function calls
+		return evalFunctionCall(scope, node)
+	}
+}
+
 // Eval type cast expressions
 func (scope *EvalScope) evalTypeCast(node *ast.CallExpr) (*Variable, error) {
 	argv, err := scope.evalAST(node.Args[0])
@@ -970,6 +1028,8 @@ func convertInt(n uint64, signed bool, size int64) uint64 {
 	return r
 }
 
+var supportedBuiltins = map[string]bool{"cap": true, "len": true, "complex": true, "imag": true, "real": true}
+
 func (scope *EvalScope) evalBuiltinCall(node *ast.CallExpr) (*Variable, error) {
 	fnnode, ok := node.Fun.(*ast.Ident)
 	if !ok {

pkg/proc/eval_go117.go

@@ -0,0 +1,10 @@
+//go:build !go1.18
+// +build !go1.18
+
+package proc
+
+import "go/ast"
+
+type astIndexListExpr struct {
+	ast.Expr
+}

pkg/proc/eval_go118.go

@@ -0,0 +1,8 @@
+//go:build go1.18
+// +build go1.18
+
+package proc
+
+import "go/ast"
+
+type astIndexListExpr = ast.IndexListExpr

pkg/proc/fncall.go

@@ -547,6 +547,9 @@ func funcCallEvalArgs(scope *EvalScope, fncall *functionCallState, formalScope *
 
 		actualArg, err := scope.evalAST(fncall.expr.Args[i])
 		if err != nil {
+			if _, ispanic := err.(fncallPanicErr); ispanic {
+				return err
+			}
 			return fmt.Errorf("error evaluating %q as argument %s in function %s: %v", exprToString(fncall.expr.Args[i]), formalArg.name, fncall.fn.Name, err)
 		}
 		actualArg.Name = exprToString(fncall.expr.Args[i])

service/test/variables_test.go

@@ -840,6 +840,16 @@ func TestEvalExpression(t *testing.T) {
 		{"ni8 << 8", false, "0", "0", "int8", nil},
 		{"ni8 >> 1", false, "-3", "-3", "int8", nil},
 		{"bytearray[0] * bytearray[0]", false, "144", "144", "uint8", nil},
+
+		// function call / typecast errors
+		{"unknownthing(1, 2)", false, "", "", "", errors.New("function calls not allowed without using 'call'")},
+		{"(unknownthing)(1, 2)", false, "", "", "", errors.New("function calls not allowed without using 'call'")},
+		{"afunc(2)", false, "", "", "", errors.New("function calls not allowed without using 'call'")},
+		{"(afunc)(2)", false, "", "", "", errors.New("function calls not allowed without using 'call'")},
+		{"(*afunc)(2)", false, "", "", "", errors.New("could not evaluate function or type (*afunc): expression \"afunc\" (func()) can not be dereferenced")},
+		{"unknownthing(2)", false, "", "", "", errors.New("could not evaluate function or type unknownthing: could not find symbol value for unknownthing")},
+		{"(*unknownthing)(2)", false, "", "", "", errors.New("could not evaluate function or type (*unknownthing): could not find symbol value for unknownthing")},
+		{"(*strings.Split)(2)", false, "", "", "", errors.New("could not evaluate function or type (*strings.Split): could not find symbol value for strings")},
 	}
 
 	ver, _ := goversion.Parse(runtime.Version())
@@ -1230,7 +1240,7 @@ func TestCallFunction(t *testing.T) {
 		{`onetwothree(intcallpanic(2))`, []string{`:[]int:[]int len: 3, cap: 3, [3,4,5]`}, nil},
 		{`onetwothree(intcallpanic(0))`, []string{`~panic:interface {}:interface {}(string) "panic requested"`}, nil},
 		{`onetwothree(intcallpanic(2)+1)`, []string{`:[]int:[]int len: 3, cap: 3, [4,5,6]`}, nil},
-		{`onetwothree(intcallpanic("not a number"))`, nil, errors.New("can not convert \"not a number\" constant to int")},
+		{`onetwothree(intcallpanic("not a number"))`, nil, errors.New("error evaluating \"intcallpanic(\\\"not a number\\\")\" as argument n in function main.onetwothree: can not convert \"not a number\" constant to int")},
 
 		// Variable setting tests
 		{`pa2 = getAStructPtr(8); pa2`, []string{`pa2:*main.astruct:*main.astruct {X: 8}`}, nil},