fix: null handling (#5)

* test: add some null handling test cases

* fix: correct support of null

Author: ddebrunner

callable.go

@@ -518,7 +518,6 @@ func (f *lambdaCallable) validateArgTypes(argv []reflect.Value) ([]reflect.Value
 	paramCount := len(f.params)
 
 	for i, arg := range argv {
-
 		// Don't type check undefined arguments.
 		if arg == undefined {
 			continue

callable_test.go

@@ -560,6 +560,10 @@ func testGoCallable(t *testing.T, tests []goCallableTest) {
 		if argc := len(test.Args); argc > 0 {
 			argv = make([]reflect.Value, argc)
 			for i := range argv {
+				if test.Args[i] == nil {
+					argv[i] = nullValue()
+					continue
+				}
 				argv[i] = reflect.ValueOf(test.Args[i])
 			}
 		}
@@ -1402,7 +1406,7 @@ func TestLambdaCallable(t *testing.T) {
 				},
 			},
 			Args: []interface{}{
-				null,
+				nil,
 			},
 			Error: &ArgTypeError{
 				Func:  "boolean2",
@@ -1811,48 +1815,55 @@ func testLambdaCallable(t *testing.T, tests []lambdaCallableTest) {
 
 	for i, test := range tests {
 
-		env := newEnvironment(nil, len(test.Vars))
-		for name, v := range test.Vars {
-			env.bind(name, reflect.ValueOf(v))
-		}
+		t.Run(test.Name, func(t *testing.T) {
 
-		f := &lambdaCallable{
-			callableName: callableName{
-				name: test.Name,
-			},
-			body:       test.Body,
-			paramNames: test.ParamNames,
-			typed:      test.Typed,
-			params:     test.Params,
-			env:        env,
-			context:    reflect.ValueOf(test.Context),
-		}
+			env := newEnvironment(nil, len(test.Vars))
+			for name, v := range test.Vars {
+				env.bind(name, reflect.ValueOf(v))
+			}
 
-		var args []reflect.Value
-		for _, arg := range test.Args {
-			args = append(args, reflect.ValueOf(arg))
-		}
+			f := &lambdaCallable{
+				callableName: callableName{
+					name: test.Name,
+				},
+				body:       test.Body,
+				paramNames: test.ParamNames,
+				typed:      test.Typed,
+				params:     test.Params,
+				env:        env,
+				context:    reflect.ValueOf(test.Context),
+			}
 
-		v, err := f.Call(args)
+			var args []reflect.Value
+			for _, arg := range test.Args {
+				if arg == nil {
+					args = append(args, nullValue())
+					continue
+				}
+				args = append(args, reflect.ValueOf(arg))
+			}
 
-		var output interface{}
-		if v.IsValid() && v.CanInterface() {
-			output = v.Interface()
-		}
+			v, err := f.Call(args)
 
-		if test.Undefined {
-			if v != undefined {
-				t.Errorf("lambda %d: expected undefined, got %v", i+1, v)
+			var output interface{}
+			if v.IsValid() && v.CanInterface() {
+				output = v.Interface()
 			}
-		} else {
-			if !reflect.DeepEqual(test.Output, output) {
-				t.Errorf("lambda %d: expected %v, got %v", i+1, test.Output, output)
+
+			if test.Undefined {
+				if v != undefined {
+					t.Errorf("lambda %d: expected undefined, got %v", i+1, v)
+				}
+			} else {
+				if !reflect.DeepEqual(test.Output, output) {
+					t.Errorf("lambda %d: expected %v, got %v", i+1, test.Output, output)
+				}
 			}
-		}
 
-		if !reflect.DeepEqual(test.Error, err) {
-			t.Errorf("lambda %d: expected error %v, got %v", i+1, test.Error, err)
-		}
+			if !reflect.DeepEqual(test.Error, err) {
+				t.Errorf("lambda %d: expected error %v, got %v", i+1, test.Error, err)
+			}
+		})
 	}
 }
 

eval.go

@@ -111,10 +111,13 @@ func evalBoolean(node *jparse.BooleanNode, data reflect.Value, env *environment)
 	return reflect.ValueOf(node.Value), nil
 }
 
-var null *interface{}
+// return a value that corresponds to any(nil).
+func nullValue() reflect.Value {
+	return reflect.ValueOf([]any{nil}).Index(0)
+}
 
 func evalNull(node *jparse.NullNode, data reflect.Value, env *environment) (reflect.Value, error) {
-	return reflect.ValueOf(null), nil
+	return nullValue(), nil
 }
 
 func evalRegex(node *jparse.RegexNode, data reflect.Value, env *environment) (reflect.Value, error) {
@@ -288,7 +291,7 @@ func evalOverSequence(node jparse.Node, seq *sequence, env *environment) ([]refl
 
 	for i, N := 0, len(seq.values); i < N; i++ {
 
-		res, err := eval(node, reflect.ValueOf(seq.values[i]), env)
+		res, err := eval(node, seq.valueAt(i), env)
 		if err != nil {
 			return nil, err
 		}
@@ -1128,12 +1131,19 @@ func eq(lhs, rhs reflect.Value) bool {
 		return reflect.DeepEqual(lhs.Interface(), rhs.Interface())
 	}
 
-	// All other types (e.g. functions) are
+	// All other valid and comparable values (e.g. functions) are
 	// compared directly. Two functions with the same contents
 	// are not considered equal unless they're the same
 	// physical object in memory.
 
-	return lhs == rhs
+	if !lhs.IsValid() || !rhs.IsValid() {
+		return false
+	}
+
+	if !lhs.CanInterface() || !rhs.CanInterface() {
+		return false
+	}
+	return lhs.Interface() == rhs.Interface()
 }
 
 func lt(lhs, rhs reflect.Value) bool {
@@ -1335,12 +1345,20 @@ func (s sequence) Value() reflect.Value {
 	case n == 0:
 		return undefined
 	case n == 1 && !s.keepSingletons:
-		return reflect.ValueOf(s.values[0])
+		return s.valueAt(0)
 	default:
 		return reflect.ValueOf(s.values)
 	}
 }
 
+func (s sequence) valueAt(idx int) reflect.Value {
+	v := s.values[idx]
+	if v == nil {
+		return nullValue()
+	}
+	return reflect.ValueOf(v)
+}
+
 var (
 	typeSequence    = reflect.TypeOf((*sequence)(nil)).Elem()
 	typeSequencePtr = reflect.PtrTo(typeSequence)

eval_test.go

@@ -75,7 +75,7 @@ func TestEvalNull(t *testing.T) {
 	testEvalTestCases(t, []evalTestCase{
 		{
 			Input:  &jparse.NullNode{},
-			Output: null,
+			Output: nil,
 		},
 	})
 }
@@ -624,7 +624,7 @@ func TestEvalArray(t *testing.T) {
 				float64(1),
 				"two",
 				true,
-				null,
+				nil,
 			},
 		},
 		{
@@ -654,7 +654,7 @@ func TestEvalArray(t *testing.T) {
 					"two",
 					true,
 				},
-				null,
+				nil,
 			},
 		},
 		{

go.mod

@@ -2,4 +2,13 @@ module github.com/stepzen-dev/jsonata-go
 
 go 1.20
 
-require golang.org/x/text v0.9.0
+require (
+	github.com/stretchr/testify v1.8.4
+	golang.org/x/text v0.9.0
+)
+
+require (
+	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
+)

go.sum

@@ -1,2 +1,12 @@
+github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
+github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
+github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
+github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
 golang.org/x/text v0.9.0 h1:2sjJmO8cDvYveuX97RDLsxlyUxLl+GHoLxBiRdHllBE=
 golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
+gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

jsonata_test.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/stepzen-dev/jsonata-go/jparse"
 	"github.com/stepzen-dev/jsonata-go/jtypes"
+	"github.com/stretchr/testify/require"
 )
 
 type testCase struct {
@@ -285,12 +286,27 @@ func TestPaths(t *testing.T) {
 }
 
 func TestPaths2(t *testing.T) {
-
 	runTestCases(t, testdata.address, []*testCase{
 		{
 			Expression: "Other.Misc",
 			Output:     nil,
-			Skip:       true, // returns ErrUndefined
+		},
+		{
+			Expression: "Other.XXX", // no such key
+			Output:     nil,
+			Error:      ErrUndefined,
+		},
+		{
+			Expression: `{"misc": Other.Misc}`,
+			Output:     map[string]any{"misc": nil},
+		},
+		{
+			Expression: `["abc", null, Other.Misc]`,
+			Output:     []any{"abc", nil, nil},
+		},
+		{
+			Expression: `null`,
+			Output:     nil,
 		},
 	})
 }
@@ -2785,15 +2801,13 @@ func TestNullExpressions(t *testing.T) {
 			Output: []interface{}{
 				nil,
 			},
-			Skip: true, // uses the wrong kind of nil (*interface{}(nil) instead of plain nil)?
 		},
 		{
 			Expression: "[null, null]",
 			Output: []interface{}{
 				nil,
 				nil,
 			},
-			Skip: true, // uses the wrong kind of nil (*interface{}(nil) instead of plain nil)?
 		},
 		{
 			Expression: "$not(null)",
@@ -2814,7 +2828,6 @@ func TestNullExpressions(t *testing.T) {
 				"false": false,
 				"null":  nil,
 			},
-			Skip: true, // uses the wrong kind of nil (*interface{}(nil) instead of plain nil)?
 		},
 	})
 }
@@ -8105,3 +8118,11 @@ func readJSON(filename string) interface{} {
 
 	return dest
 }
+
+func TestNullValue(t *testing.T) {
+	nv := nullValue()
+	require.True(t, nv.IsValid())
+	require.True(t, nv.IsNil())
+	require.True(t, nv.CanInterface())
+	require.True(t, nv.Interface() == nil)
+}