Repository: google/starlark-go (go)

Issue:
FR: json.valid
As Starlark doesn't include `try` and `except`, it would be valuable to have a `json.valid` or similar function to enable custom handling (overwrite, `fail`, etc) of invalid json encoding.

Pull request description:
Allow json.decode to return a default value if decoding fails
This is the starlark-go counterpart to bazelbuild/bazel@71fb1e4

It is useful to be able to decode a potentially invalid JSON string without
failing Starlark evaluation. Typically, in this case, we would want to return
a default value - either a reasonable safe default or some domain-specific
representation of "we encountered an error".

Note that the original behavior of json.decode failing on invalid JSON input
still remains (for lack of a better word) the default: the `default` value,
even if it is `None`, must be specified explicitly.

Fixes #465

Patch:
diff --git a/lib/json/json.go b/lib/json/json.go
index 6d9e99e..f44dbaf 100644
--- a/lib/json/json.go
+++ b/lib/json/json.go
@@ -11,7 +11,6 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"log"
 	"math"
 	"math/big"
 	"reflect"
@@ -54,15 +53,16 @@ import (
 // (e.g. it implements both Iterable and HasFields), the first case takes precedence.
 // Encoding any other value yields an error.
 //
-// def decode(x):
+// def decode(x[, default]):
 //
-// The decode function accepts one positional parameter, a JSON string.
+// The decode function has one required positional parameter, a JSON string.
 // It returns the Starlark value that the string denotes.
 // - Numbers are parsed as int or float, depending on whether they
 //   contain a decimal point.
 // - JSON objects are parsed as new unfrozen Starlark dicts.
 // - JSON arrays are parsed as new unfrozen Starlark lists.
-// Decoding fails if x is not a valid JSON string.
+// If x is not a valid JSON string, the behavior depends on the "default"
+// parameter: if present, Decode returns its value; otherwise, Decode fails.
 //
 // def indent(str, *, prefix="", indent="\t"):
 //
@@ -200,8 +200,13 @@ func encode(thread *starlark.Thread, b *starlark.Builtin, args starlark.Tuple, k
 			sort.Strings(names)
 			for i, name := range names {
 				v, err := x.Attr(name)
-				if err != nil || v == nil {
-					log.Fatalf("internal error: dir(%s) includes %q but value has no .%s field", x.Type(), name, name)
+				if err != nil {
+					return fmt.Errorf("cannot access attribute %s.%s: %w", x.Type(), name, err)
+				}
+				if v == nil {
+					// x.AttrNames() returned name, but x.Attr(name) returned nil, stating
+					// that the field doesn't exist.
+					return fmt.Errorf("missing attribute %s.%s (despite %q appearing in dir()", x.Type(), name, name)
 				}
 				if i > 0 {
 					buf.WriteByte(',')
@@ -283,11 +288,17 @@ func indent(thread *starlark.Thread, b *starlark.Builtin, args starlark.Tuple, k
 	return starlark.String(buf.String()), nil
 }
 
-func decode(thread *starlark.Thread, b *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (_ starlark.Value, err error) {
+func decode(thread *starlark.Thread, b *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (v starlark.Value, err error) {
 	var s string
-	if err := starlark.UnpackPositionalArgs(b.Name(), args, kwargs, 1, &s); err != nil {
+	var d starlark.Value
+	if err := starlark.UnpackArgs(b.Name(), args, kwargs, "x", &s, "default?", &d); err != nil {
 		return nil, err
 	}
+	if len(args) < 1 {
+		// "x" parameter is positional only; UnpackArgs does not allow us to
+		// directly express "def decode(x, *, default)"
+		return nil, fmt.Errorf("%s: unexpected keyword argument x", b.Name())
+	}
 
 	// The decoder necessarily makes certain representation choices
 	// such as list vs tuple, struct vs dict, int vs float.
@@ -295,6 +306,11 @@ func decode(thread *starlark.Thread, b *starlark.Builtin, args starlark.Tuple, k
 	// control the returned types, but there's no compelling need yet.
 
 	// Use panic/recover with a distinguished type (failure) for error handling.
+	// If "default" is set, we only want to return it when encountering invalid
+	// json - not for any other possible causes of panic.
+	// In particular, if we ever extend the json.decode API to take a callback,
+	// a distinguished, private failure type prevents the possibility of
+	// json.decode with "default" becoming abused as a try-catch mechanism.
 	type failure string
 	fail := func(format string, args ...interface{}) {
 		panic(failure(fmt.Sprintf(format, args...)))
@@ -496,18 +512,22 @@ func decode(thread *starlark.Thread, b *starlark.Builtin, args starlark.Tuple, k
 		x := recover()
 		switch x := x.(type) {
 		case failure:
-			err = fmt.Errorf("json.decode: at offset %d, %s", i, x)
+			if d != nil {
+				v = d
+			} else {
+				err = fmt.Errorf("json.decode: at offset %d, %s", i, x)
+			}
 		case nil:
 			// nop
 		default:
 			panic(x) // unexpected panic
 		}
 	}()
-	x := parse()
+	v = parse()
 	if skipSpace() {
 		fail("unexpected character %q after value", s[i])
 	}
-	return x, nil
+	return v, nil
 }
 
 func isdigit(b byte) bool {


Test patch:
diff --git a/starlark/testdata/json.star b/starlark/testdata/json.star
index 4f66637..62a457b 100644
--- a/starlark/testdata/json.star
+++ b/starlark/testdata/json.star
@@ -111,6 +111,20 @@ decode_error('{"one": 1,', "unexpected end of file")
 decode_error('{"one": 1, }', "unexpected character '}'")
 decode_error('{"one": 1]', "in object, got ']', want ',' or '}'")
 
+## json.decode with default specified
+
+assert.eq(json.decode('{"valid": "json"}', default = "default value"), {"valid": "json"})
+assert.eq(json.decode('{"valid": "json"}', "default value"), {"valid": "json"})
+assert.eq(json.decode('{"invalid": "json"', default = "default value"), "default value")
+assert.eq(json.decode('{"invalid": "json"', "default value"), "default value")
+assert.eq(json.decode('{"invalid": "json"', default = None), None)
+assert.eq(json.decode('{"invalid": "json"', None), None)
+
+assert.fails(
+    lambda: json.decode(x = '{"invalid": "json"', default = "default value"),
+    "unexpected keyword argument x"
+)
+
 def codec(x):
     return json.decode(json.encode(x))