Optional eval (google#92)

* Optional arguments

.gitignore

@@ -4,3 +4,4 @@ coverage.out
 /tests_path.source
 /jsonnet/jsonnet
 *.so
+*.prof

ast/ast.go

@@ -322,8 +322,8 @@ type NamedParameter struct {
 }
 
 type Parameters struct {
-	Positional Identifiers
-	Named      []NamedParameter
+	Required Identifiers
+	Optional []NamedParameter
 }
 
 // ---------------------------------------------------------------------------

builtins.go

@@ -224,7 +224,7 @@ func builtinLength(e *evaluator, xp potentialValue) (value, error) {
 	case *valueString:
 		num = x.length()
 	case *valueFunction:
-		num = len(x.parameters())
+		num = len(x.parameters().required)
 	default:
 		return nil, e.typeErrorGeneral(x)
 	}
@@ -614,13 +614,12 @@ func getBuiltinEvaluator(e *evaluator, name ast.Identifier) *evaluator {
 }
 
 func (b *UnaryBuiltin) EvalCall(args callArguments, e *evaluator) (value, error) {
-
-	// TODO check args
-	return b.function(getBuiltinEvaluator(e, b.name), args.positional[0])
+	flatArgs := flattenArgs(args, b.Parameters())
+	return b.function(getBuiltinEvaluator(e, b.name), flatArgs[0])
 }
 
-func (b *UnaryBuiltin) Parameters() ast.Identifiers {
-	return b.parameters
+func (b *UnaryBuiltin) Parameters() Parameters {
+	return Parameters{required: b.parameters}
 }
 
 type BinaryBuiltin struct {
@@ -629,13 +628,38 @@ type BinaryBuiltin struct {
 	parameters ast.Identifiers
 }
 
+// flattenArgs transforms all arguments to a simple array of positional arguments.
+// It's needed, because it's possible to use named arguments for required parameters.
+// For example both `toString("x")` and `toString(a="x")` are allowed.
+// It assumes that we have already checked for duplicates.
+func flattenArgs(args callArguments, params Parameters) []potentialValue {
+	if len(args.named) == 0 {
+		return args.positional
+	}
+	if len(params.optional) != 0 {
+		panic("Can't normalize arguments if optional parameters are present")
+	}
+	needed := make(map[ast.Identifier]int)
+
+	for i := len(args.positional); i < len(params.required); i++ {
+		needed[params.required[i]] = i
+	}
+
+	flatArgs := make([]potentialValue, len(params.required))
+	copy(flatArgs, args.positional)
+	for _, arg := range args.named {
+		flatArgs[needed[arg.name]] = arg.pv
+	}
+	return flatArgs
+}
+
 func (b *BinaryBuiltin) EvalCall(args callArguments, e *evaluator) (value, error) {
-	// TODO check args
-	return b.function(getBuiltinEvaluator(e, b.name), args.positional[0], args.positional[1])
+	flatArgs := flattenArgs(args, b.Parameters())
+	return b.function(getBuiltinEvaluator(e, b.name), flatArgs[0], flatArgs[1])
 }
 
-func (b *BinaryBuiltin) Parameters() ast.Identifiers {
-	return b.parameters
+func (b *BinaryBuiltin) Parameters() Parameters {
+	return Parameters{required: b.parameters}
 }
 
 type TernaryBuiltin struct {
@@ -645,12 +669,12 @@ type TernaryBuiltin struct {
 }
 
 func (b *TernaryBuiltin) EvalCall(args callArguments, e *evaluator) (value, error) {
-	// TODO check args
-	return b.function(getBuiltinEvaluator(e, b.name), args.positional[0], args.positional[1], args.positional[2])
+	flatArgs := flattenArgs(args, b.Parameters())
+	return b.function(getBuiltinEvaluator(e, b.name), flatArgs[0], flatArgs[1], flatArgs[2])
 }
 
-func (b *TernaryBuiltin) Parameters() ast.Identifiers {
-	return b.parameters
+func (b *TernaryBuiltin) Parameters() Parameters {
+	return Parameters{required: b.parameters}
 }
 
 var desugaredBop = map[ast.BinaryOp]ast.Identifier{
@@ -698,7 +722,7 @@ var uopBuiltins = []*UnaryBuiltin{
 var funcBuiltins = map[string]evalCallable{
 	"extVar":          &UnaryBuiltin{name: "extVar", function: builtinExtVar, parameters: ast.Identifiers{"x"}},
 	"length":          &UnaryBuiltin{name: "length", function: builtinLength, parameters: ast.Identifiers{"x"}},
-	"toString":        &UnaryBuiltin{name: "toString", function: builtinToString, parameters: ast.Identifiers{"x"}},
+	"toString":        &UnaryBuiltin{name: "toString", function: builtinToString, parameters: ast.Identifiers{"a"}},
 	"makeArray":       &BinaryBuiltin{name: "makeArray", function: builtinMakeArray, parameters: ast.Identifiers{"sz", "func"}},
 	"flatMap":         &BinaryBuiltin{name: "flatMap", function: builtinFlatMap, parameters: ast.Identifiers{"func", "arr"}},
 	"filter":          &BinaryBuiltin{name: "filter", function: builtinFilter, parameters: ast.Identifiers{"func", "arr"}},

desugarer.go

@@ -86,28 +86,6 @@ func stringUnescape(loc *ast.LocationRange, s string) (string, error) {
 }
 
 func desugarFields(location ast.LocationRange, fields *ast.ObjectFields, objLevel int) error {
-
-	// Desugar children
-	for i := range *fields {
-		field := &((*fields)[i])
-		if field.Expr1 != nil {
-			err := desugar(&field.Expr1, objLevel)
-			if err != nil {
-				return err
-			}
-		}
-		err := desugar(&field.Expr2, objLevel+1)
-		if err != nil {
-			return err
-		}
-		if field.Expr3 != nil {
-			err := desugar(&field.Expr3, objLevel+1)
-			if err != nil {
-				return err
-			}
-		}
-	}
-
 	// Simplify asserts
 	for i := range *fields {
 		field := &(*fields)[i]
@@ -187,7 +165,7 @@ func desugarFields(location ast.LocationRange, fields *ast.ObjectFields, objLeve
 func simpleLambda(body ast.Node, paramName ast.Identifier) ast.Node {
 	return &ast.Function{
 		Body:       body,
-		Parameters: ast.Parameters{Positional: ast.Identifiers{paramName}},
+		Parameters: ast.Parameters{Required: ast.Identifiers{paramName}},
 	}
 }
 
@@ -233,8 +211,6 @@ func desugarObjectComp(comp *ast.ObjectComp, objLevel int) (ast.Node, error) {
 		comp.Fields = append(comp.Fields, ast.ObjectFieldLocalNoMethod(&dollar, &ast.Self{}))
 	}
 
-	// TODO(sbarzowski) find a consistent convention to prevent desugaring the same thing twice
-	// here we deeply desugar fields and it will happen again
 	err := desugarFields(*comp.Loc(), &comp.Fields, objLevel+1)
 	if err != nil {
 		return nil, err
@@ -383,6 +359,7 @@ func desugar(astPtr *ast.Node, objLevel int) (err error) {
 					Expr: buildStdCall(desugaredBop[ast.BopManifestEqual], node.Left, node.Right),
 				}
 			} else if node.Op == ast.BopIn {
+				// reversed order of arguments
 				*astPtr = buildStdCall(funcname, node.Right, node.Left)
 			} else {
 				*astPtr = buildStdCall(funcname, node.Left, node.Right)
@@ -429,6 +406,13 @@ func desugar(astPtr *ast.Node, objLevel int) (err error) {
 		}
 
 	case *ast.Function:
+		for i := range node.Parameters.Optional {
+			param := &node.Parameters.Optional[i]
+			err = desugar(&param.DefaultArg, objLevel)
+			if err != nil {
+				return
+			}
+		}
 		err = desugar(&node.Body, objLevel)
 		if err != nil {
 			return
@@ -476,7 +460,10 @@ func desugar(astPtr *ast.Node, objLevel int) (err error) {
 			node.Step = &ast.LiteralNull{}
 		}
 		*astPtr = buildStdCall("slice", node.Target, node.BeginIndex, node.EndIndex, node.Step)
-		desugar(astPtr, objLevel)
+		err = desugar(astPtr, objLevel)
+		if err != nil {
+			return
+		}
 
 	case *ast.Local:
 		for i := range node.Binds {
@@ -529,9 +516,32 @@ func desugar(astPtr *ast.Node, objLevel int) (err error) {
 		}
 
 		*astPtr = buildDesugaredObject(node.NodeBase, node.Fields)
+		err = desugar(astPtr, objLevel)
+		if err != nil {
+			return
+		}
 
 	case *ast.DesugaredObject:
-		return nil
+		for i := range node.Fields {
+			field := &((node.Fields)[i])
+			if field.Name != nil {
+				err := desugar(&field.Name, objLevel)
+				if err != nil {
+					return err
+				}
+			}
+			err := desugar(&field.Body, objLevel+1)
+			if err != nil {
+				return err
+			}
+		}
+		for i := range node.Asserts {
+			assert := &((node.Asserts)[i])
+			err := desugar(assert, objLevel+1)
+			if err != nil {
+				return err
+			}
+		}
 
 	case *ast.ObjectComp:
 		comp, err := desugarObjectComp(node, objLevel)

interpreter.go

@@ -464,11 +464,16 @@ func (i *interpreter) evaluate(a ast.Node) (value, error) {
 
 		arguments := callArguments{
 			positional: make([]potentialValue, len(ast.Arguments.Positional)),
+			named:      make([]namedCallArgument, len(ast.Arguments.Named)),
 		}
 		for i, arg := range ast.Arguments.Positional {
 			arguments.positional[i] = makeThunk(argEnv, arg)
 		}
 
+		for i, arg := range ast.Arguments.Named {
+			arguments.named[i] = namedCallArgument{name: arg.Name, pv: makeThunk(argEnv, arg.Arg)}
+		}
+
 		return e.evaluate(function.call(arguments))
 
 	default:

main_test.go

@@ -23,7 +23,6 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
-	"unicode/utf8"
 
 	"github.com/sergi/go-diff/diffmatchpatch"
 )
@@ -40,25 +39,6 @@ type mainTest struct {
 	golden string
 }
 
-func removeExcessiveWhitespace(s string) string {
-	var buf bytes.Buffer
-	separated := true
-	for i, w := 0, 0; i < len(s); i += w {
-		runeValue, width := utf8.DecodeRuneInString(s[i:])
-		if runeValue == '\n' || runeValue == ' ' {
-			if !separated {
-				buf.WriteString(" ")
-				separated = true
-			}
-		} else {
-			buf.WriteRune(runeValue)
-			separated = false
-		}
-		w = width
-	}
-	return buf.String()
-}
-
 func setExtVars(vm *VM) {
 	// TODO(sbarzowski) extract, so that it's possible to define extvars per-test
 	// Check that it doesn't get evaluated.

mutually_recursive_defaults.input

@@ -0,0 +1 @@
+(function(a=[1, b[1]], b=[a[0], 2]) [a, b])()

parser/context.go

@@ -313,9 +313,9 @@ func addContext(node ast.Node, context *string, bind string) {
 	case *ast.Function:
 		funContext := functionContext(bind)
 		addContext(node.Body, funContext, anonymous)
-		for i := range node.Parameters.Named {
+		for i := range node.Parameters.Optional {
 			// Default arguments have the same context as the function body.
-			addContext(node.Parameters.Named[i].DefaultArg, funContext, anonymous)
+			addContext(node.Parameters.Optional[i].DefaultArg, funContext, anonymous)
 		}
 	case *ast.Object:
 		// TODO(sbarzowski) include fieldname, maybe even chains

parser/parser.go

@@ -200,10 +200,10 @@ func (p *parser) parseParameters(elementKind string) (*ast.Parameters, bool, err
 		if !ok {
 			return nil, false, MakeStaticError(fmt.Sprintf("Expected simple identifier but got a complex expression."), *arg.Loc())
 		}
-		params.Positional = append(params.Positional, *id)
+		params.Required = append(params.Required, *id)
 	}
 	for _, arg := range args.Named {
-		params.Named = append(params.Named, ast.NamedParameter{Name: arg.Name, DefaultArg: arg.Arg})
+		params.Optional = append(params.Optional, ast.NamedParameter{Name: arg.Name, DefaultArg: arg.Arg})
 	}
 	return &params, trailingComma, nil
 }

static_analyzer.go

@@ -46,6 +46,9 @@ func analyzeVisit(a ast.Node, inObject bool, vars ast.IdentifierSet) error {
 		for _, arg := range a.Arguments.Positional {
 			visitNext(arg, inObject, vars, s)
 		}
+		for _, arg := range a.Arguments.Named {
+			visitNext(arg.Arg, inObject, vars, s)
+		}
 	case *ast.Array:
 		for _, elem := range a.Elements {
 			visitNext(elem, inObject, vars, s)
@@ -60,18 +63,24 @@ func analyzeVisit(a ast.Node, inObject bool, vars ast.IdentifierSet) error {
 	case *ast.Error:
 		visitNext(a.Expr, inObject, vars, s)
 	case *ast.Function:
-		// TODO(sbarzowski) check duplicate function parameters
-		// or maybe somewhere else as it doesn't require any context
 		newVars := vars.Clone()
-		for _, param := range a.Parameters.Positional {
+		for _, param := range a.Parameters.Required {
 			newVars.Add(param)
 		}
+		for _, param := range a.Parameters.Optional {
+			newVars.Add(param.Name)
+		}
+		for _, param := range a.Parameters.Optional {
+			visitNext(param.DefaultArg, inObject, newVars, s)
+		}
 		visitNext(a.Body, inObject, newVars, s)
 		// Parameters are free inside the body, but not visible here or outside
-		for _, param := range a.Parameters.Positional {
+		for _, param := range a.Parameters.Required {
 			s.freeVars.Remove(param)
 		}
-		// TODO(sbarzowski) when we have default values of params check them
+		for _, param := range a.Parameters.Optional {
+			s.freeVars.Remove(param.Name)
+		}
 	case *ast.Import:
 		//nothing to do here
 	case *ast.ImportStr:

std.thisFile.jsonnet

@@ -0,0 +1 @@
+std.thisFile

testdata/bad_function_call.golden

@@ -1,4 +1,4 @@
-RUNTIME ERROR: function expected 1 argument(s), but got 0
+RUNTIME ERROR: Missing argument: x
 -------------------------------------------------
 	testdata/bad_function_call:1:1-18	$
 

testdata/bad_function_call2.golden

@@ -1,4 +1,4 @@
-RUNTIME ERROR: function expected 1 argument(s), but got 2
+RUNTIME ERROR: Function expected 1 positional argument(s), but got 2
 -------------------------------------------------
 	testdata/bad_function_call2:1:1-22	$
 

testdata/bad_function_call_and_error.golden

@@ -1,4 +1,4 @@
-RUNTIME ERROR: function expected 1 argument(s), but got 2
+RUNTIME ERROR: Function expected 1 positional argument(s), but got 2
 -------------------------------------------------
 	testdata/bad_function_call_and_error:1:1-38	$
 

testdata/optional_args.golden

@@ -0,0 +1 @@
+42

testdata/optional_args.jsonnet

@@ -0,0 +1 @@
+local foo(x=42) = x; foo()

testdata/optional_args10.golden

@@ -0,0 +1 @@
+2

testdata/optional_args10.jsonnet

@@ -0,0 +1 @@
+(function(x, y) x)(y=1, x=2)

testdata/optional_args11.golden

@@ -0,0 +1,10 @@
+RUNTIME ERROR: Argument x already provided
+-------------------------------------------------
+	testdata/optional_args11:1:1-30	$
+
+(function(x, y) 42)(42, x=42)
+
+-------------------------------------------------
+	During evaluation	
+
+

testdata/optional_args11.jsonnet

@@ -0,0 +1 @@
+(function(x, y) 42)(42, x=42)

testdata/optional_args12.golden

@@ -0,0 +1 @@
+42

testdata/optional_args12.jsonnet

@@ -0,0 +1 @@
+(function(x=42, y=42) 42)(y=42)