Repository: google/starlark-go

Issue:
Please add json.encode_indent
See Bazel: https://bazel.build/rules/lib/core/json#encode_indent

We've received a bug report internally that some tests which use starlark-go to parse .bzl files are failing because starlark-go doesn't know about json.encode_indent.

CC @tjgq and @hvadehra.

Hints from issue comments:
@adonovan: Seems reasonable. The implementation in Java is simply `indent(encode(x), prefix, indent)` (see cl/338137953).

Care to send a PR?


Solution patch:
diff --git a/cmd/starlark/starlark.go b/cmd/starlark/starlark.go
index 8076deb..f84ef03 100644
--- a/cmd/starlark/starlark.go
+++ b/cmd/starlark/starlark.go
@@ -38,13 +38,13 @@ func init() {
 	flag.BoolVar(&compile.Disassemble, "disassemble", compile.Disassemble, "show disassembly during compilation of each function")
 
 	// non-standard dialect flags
-	flag.BoolVar(&resolve.AllowSet, "set", resolve.AllowSet, "allow set data type")
 	flag.BoolVar(&resolve.AllowRecursion, "recursion", resolve.AllowRecursion, "allow while statements and recursive functions")
 	flag.BoolVar(&resolve.AllowGlobalReassign, "globalreassign", resolve.AllowGlobalReassign, "allow reassignment of globals, and if/for/while statements at top level")
 
-	// flags that are now standard
-	flag.BoolVar(&resolve.AllowFloat, "float", resolve.AllowFloat, "obsolete; no effect")
-	flag.BoolVar(&resolve.AllowLambda, "lambda", resolve.AllowLambda, "obsolete; no effect")
+	// obsolete flags for features that are now standard
+	flag.BoolVar(&resolve.AllowSet, "set", true, "obsolete; no effect")
+	flag.BoolVar(&resolve.AllowFloat, "float", true, "obsolete; no effect")
+	flag.BoolVar(&resolve.AllowLambda, "lambda", true, "obsolete; no effect")
 }
 
 func main() {
diff --git a/doc/spec.md b/doc/spec.md
index c5c1e71..29a38cb 100644
--- a/doc/spec.md
+++ b/doc/spec.md
@@ -3466,7 +3466,7 @@ x.setdefault("one")                     # 1
 x.setdefault("three", 0)                # 0
 x                                       # {"one": 1, "two": 2, "three": 0}
 x.setdefault("four")                    # None
-x                                       # {"one": 1, "two": 2, "three": None}
+x                                       # {"one": 1, "two": 2, "three": 0, "four": None}
 ```
 
 <a id='dict·update'></a>
diff --git a/lib/json/json.go b/lib/json/json.go
index bc67be5..eb16b2c 100644
--- a/lib/json/json.go
+++ b/lib/json/json.go
@@ -28,6 +28,7 @@ import (
 //
 //	json = module(
 //	   encode,
+//	   encode_indent,
 //	   decode,
 //	   indent,
 //	)
@@ -54,6 +55,10 @@ import (
 // (e.g. it implements both Iterable and HasFields), the first case takes precedence.
 // Encoding any other value yields an error.
 //
+// def encode_indent(x, *, prefix="", indent="\t"):
+//
+// Equivalent to json.indent(json.encode(x), prefix=prefix, indent=indent).
+//
 // def decode(x[, default]):
 //
 // The decode function has one required positional parameter, a JSON string.
@@ -76,9 +81,10 @@ import (
 var Module = &starlarkstruct.Module{
 	Name: "json",
 	Members: starlark.StringDict{
-		"encode": starlark.NewBuiltin("json.encode", encode),
-		"decode": starlark.NewBuiltin("json.decode", decode),
-		"indent": starlark.NewBuiltin("json.indent", indent),
+		"encode":        starlark.NewBuiltin("json.encode", encode),
+		"encode_indent": starlark.NewBuiltin("json.encode_indent", encodeIndent),
+		"decode":        starlark.NewBuiltin("json.decode", decode),
+		"indent":        starlark.NewBuiltin("json.indent", indent),
 	},
 }
 
@@ -233,6 +239,27 @@ func encode(thread *starlark.Thread, b *starlark.Builtin, args starlark.Tuple, k
 	return starlark.String(buf.String()), nil
 }
 
+func encodeIndent(thread *starlark.Thread, b *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
+	prefix, indent := "", "\t" // keyword-only
+	if err := starlark.UnpackArgs(b.Name(), nil, kwargs,
+		"prefix?", &prefix,
+		"indent?", &indent,
+	); err != nil {
+		return nil, err
+	}
+	// Rely on encode() to parse the positional parameter (since the signature matches); use our b so
+	// error messages are attributed to json.encode_indent.
+	str, err := encode(thread, b, args, nil)
+	if err != nil {
+		return nil, err
+	}
+	var buf bytes.Buffer
+	if err := json.Indent(&buf, []byte(str.(starlark.String)), prefix, indent); err != nil {
+		return nil, fmt.Errorf("%s: %v", b.Name(), err)
+	}
+	return starlark.String(buf.String()), nil
+}
+
 func pointer(i interface{}) unsafe.Pointer {
 	v := reflect.ValueOf(i)
 	switch v.Kind() {
diff --git a/lib/proto/cmd/star2proto/star2proto.go b/lib/proto/cmd/star2proto/star2proto.go
index dffa603..e3b7b9c 100644
--- a/lib/proto/cmd/star2proto/star2proto.go
+++ b/lib/proto/cmd/star2proto/star2proto.go
@@ -37,14 +37,12 @@ var (
 	descriptors = flag.String("descriptors", "", "comma-separated list of names of files containing proto.FileDescriptorProto messages")
 )
 
-// Starlark dialect flags
+// obsolete Starlark dialect flags
 func init() {
-	flag.BoolVar(&resolve.AllowSet, "set", resolve.AllowSet, "allow set data type")
-
-	// obsolete, no effect:
-	flag.BoolVar(&resolve.AllowFloat, "fp", true, "allow floating-point numbers")
-	flag.BoolVar(&resolve.AllowLambda, "lambda", resolve.AllowLambda, "allow lambda expressions")
-	flag.BoolVar(&resolve.AllowNestedDef, "nesteddef", resolve.AllowNestedDef, "allow nested def statements")
+	flag.BoolVar(&resolve.AllowFloat, "fp", true, "obsolete; no effect")
+	flag.BoolVar(&resolve.AllowLambda, "lambda", true, "obsolete; no effect")
+	flag.BoolVar(&resolve.AllowNestedDef, "nesteddef", true, "obsolete; no effect")
+	flag.BoolVar(&resolve.AllowSet, "set", true, "obsolete; no effect")
 }
 
 func main() {
diff --git a/resolve/resolve.go b/resolve/resolve.go
index e00d2b3..c2ca7eb 100644
--- a/resolve/resolve.go
+++ b/resolve/resolve.go
@@ -94,23 +94,29 @@ import (
 const debug = false
 const doesnt = "this Starlark dialect does not "
 
-// global options
-// These features are either not standard Starlark (yet), or deprecated
-// features of the BUILD language, so we put them behind flags.
+// Global options: these features are either not standard Starlark
+// (yet), or deprecated features of the BUILD language, so we put them
+// behind flags.
 //
 // Deprecated: use an explicit [syntax.FileOptions] argument instead,
-// as it avoids all the usual problems of global variables.
+// as it avoids all the usual problems of global variables,
+// and permits finer control.
+//
+// For example The legacy AllowGlobalReassign flag controls three
+// FileOptions: the availability of 'while' loops at all; the use of
+// if/for/while constructs at top level; and the ability to reassign
+// a global variable.
 var (
-	AllowSet            = false // allow the 'set' built-in
-	AllowGlobalReassign = false // allow reassignment to top-level names; also, allow if/for/while at top-level
-	AllowRecursion      = false // allow while statements and recursive functions
+	AllowGlobalReassign = false // allow reassignment to top-level names; while loops; and if/for/while at top-level
+	AllowRecursion      = false // allow recursive functions
 	LoadBindsGlobally   = false // load creates global not file-local bindings (deprecated)
 
 	// obsolete flags for features that are now standard. No effect.
-	AllowNestedDef = true
-	AllowLambda    = true
-	AllowFloat     = true
 	AllowBitwise   = true
+	AllowFloat     = true
+	AllowLambda    = true
+	AllowNestedDef = true
+	AllowSet       = true
 )
 
 // File resolves the specified file and records information about the
diff --git a/starlark/interp.go b/starlark/interp.go
index 261077f..a0af4cb 100644
--- a/starlark/interp.go
+++ b/starlark/interp.go
@@ -24,7 +24,17 @@ func (fn *Function) CallInternal(thread *Thread, args Tuple, kwargs []Tuple) (Va
 	// but allows CALL to avoid a copy.
 
 	f := fn.funcode
-	if !f.Prog.Recursion {
+	if f.Prog.Recursion {
+		// prevent stack overflow
+		//
+		// Each CallInternal recursion (via Call) uses ~1.4KB,
+		// but the stack limit is on the order of 1GB, so a
+		// maximum of about 700K recursive calls is possible.
+		// Limit it to much less here.
+		if len(thread.stack) > 100_000 {
+			return nil, fmt.Errorf("Starlark stack overflow")
+		}
+	} else {
 		// detect recursion
 		for _, fr := range thread.stack[:len(thread.stack)-1] {
 			// We look for the same function code,
diff --git a/starlark/library.go b/starlark/library.go
index 78de542..2ff2544 100644
--- a/starlark/library.go
+++ b/starlark/library.go
@@ -63,7 +63,7 @@ func init() {
 		"range":     NewBuiltin("range", range_),
 		"repr":      NewBuiltin("repr", repr),
 		"reversed":  NewBuiltin("reversed", reversed),
-		"set":       NewBuiltin("set", set), // requires resolve.AllowSet
+		"set":       NewBuiltin("set", set),
 		"sorted":    NewBuiltin("sorted", sorted),
 		"str":       NewBuiltin("str", str),
 		"tuple":     NewBuiltin("tuple", tuple),



Test patch:
diff --git a/starlark/testdata/json.star b/starlark/testdata/json.star
index 1df9dee..47ae0eb 100644
--- a/starlark/testdata/json.star
+++ b/starlark/testdata/json.star
@@ -3,7 +3,7 @@
 load("assert.star", "assert")
 load("json.star", "json")
 
-assert.eq(dir(json), ["decode", "encode", "indent"])
+assert.eq(dir(json), ["decode", "encode", "encode_indent", "indent"])
 
 # Some of these cases were inspired by github.com/nst/JSONTestSuite.
 
@@ -49,6 +49,20 @@ recursive_tuple = (1, 2, [])
 recursive_tuple[2].append(recursive_tuple)
 encode_error(recursive_tuple, 'json.encode: at tuple index 2: at list index 0: cycle in JSON structure')
 
+## json.encode_indent
+
+assert.eq(
+    json.encode_indent({"x": 1, "y": ["one", "two"]}, prefix="¶", indent="–––"),
+    json.indent(json.encode({"x": 1, "y": ["one", "two"]}), prefix="¶", indent="–––")
+)
+assert.eq(
+    json.encode_indent({"x": 1, "y": ["one", "two"]}),
+    json.indent(json.encode({"x": 1, "y": ["one", "two"]}), prefix="", indent="\t")
+)
+
+assert.fails(lambda: json.encode_indent('foo', 'bar'), 'json.encode_indent: got 2 arguments, want 1')
+assert.fails(lambda: json.encode_indent(recursive_map), 'json.encode_indent: in dict key "r": cycle in JSON structure')
+
 ## json.decode
 
 assert.eq(json.decode("null"), None)
diff --git a/starlark/testdata/recursion.star b/starlark/testdata/recursion.star
index 7029ea0..ebaabc5 100644
--- a/starlark/testdata/recursion.star
+++ b/starlark/testdata/recursion.star
@@ -12,3 +12,10 @@ def fib(n):
 	return fib(n-1) + fib(n-2)
 
 assert.eq(fib(5), 8)
+
+def runaway():
+	return runaway()
+
+# Runaway recursion should not overflow the Go stack (#617).
+# The interpreter imposes a limit long before then.
+assert.fails(runaway, "Starlark stack overflow")
\ No newline at end of file