Skip to content

Commit 6375ab4

Browse files
Fix singleton unwrapping for $each, $spread, and $match (RecoLabs#3)
These three functions returned []any instead of *Sequence, preventing CollapseSequence from unwrapping single-element results. This caused parity divergence from jsonata-js: - $each({"a":1}, fn) returned ["result"] instead of "result" - $spread({"a":1}) returned [{"a":1}] instead of {"a":1} - $match("hello", /^h/) returned [{...}] instead of {...} Return *Sequence from all three so evalFunction properly singleton-unwraps single-element results. Also introduces CollapseAndKeep helper in value.go (with shallow struct copy for alias safety) to centralize KeepArray/singleton logic, and collapses intermediate results in function composition to prevent raw *Sequence leaking between composed functions. Signed-off-by: NirBarak-RecoLabs <nirb@recolabs.ai>
1 parent 47994a4 commit 6375ab4

16 files changed

Lines changed: 189 additions & 45 deletions

File tree

evaluator_test.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,79 @@ func TestRegressionExpr(t *testing.T) {
495495
}
496496
}
497497

498+
// TestSingletonUnwrapEdgeCases covers scenarios the JSON suite cannot:
499+
// error expectations and chain operator (~>) paths.
500+
func TestSingletonUnwrapEdgeCases(t *testing.T) {
501+
tests := []struct {
502+
name string
503+
expr string
504+
data string
505+
want any
506+
wantError bool
507+
}{
508+
{
509+
name: "join_nested_each_single_key_errors",
510+
expr: `$join($each({"x": {"p": "hi", "q": "lo"}},` +
511+
` function($obj, $name) { $each($obj,` +
512+
` function($v, $k) { $name & "." & $k & "=" & $v }) })[], ', ')`,
513+
data: `{}`,
514+
wantError: true,
515+
},
516+
{
517+
name: "chain_each_single_key",
518+
expr: `{"a": 1} ~> $each(function($v, $k) { $k })`,
519+
data: `{}`,
520+
want: "a",
521+
},
522+
{
523+
name: "chain_spread_single_key",
524+
expr: `{"a": 1} ~> $spread()`,
525+
data: `{}`,
526+
want: map[string]any{"a": float64(1)},
527+
},
528+
{
529+
name: "chain_bare_spread_single_key",
530+
expr: `{"a": 1} ~> $spread`,
531+
data: `{}`,
532+
want: map[string]any{"a": float64(1)},
533+
},
534+
{
535+
name: "composition_spread_merge",
536+
expr: `($spread ~> $merge)({"a": 1, "b": 2})`,
537+
data: `{}`,
538+
want: map[string]any{"a": float64(1), "b": float64(2)},
539+
},
540+
}
541+
542+
for _, tt := range tests {
543+
t.Run(tt.name, func(t *testing.T) {
544+
var data any
545+
if err := json.Unmarshal([]byte(tt.data), &data); err != nil {
546+
t.Fatalf("unmarshal: %v", err)
547+
}
548+
e, err := gnata.Compile(tt.expr)
549+
if err != nil {
550+
t.Fatalf("compile %q: %v", tt.expr, err)
551+
}
552+
result, err := e.Eval(context.Background(), data)
553+
if tt.wantError {
554+
if err == nil {
555+
t.Fatalf("expected error, got %v", result)
556+
}
557+
return
558+
}
559+
if err != nil {
560+
t.Fatalf("eval %q: %v", tt.expr, err)
561+
}
562+
if tt.want != nil && !gnata.DeepEqual(result, tt.want) {
563+
got, _ := json.Marshal(result)
564+
want, _ := json.Marshal(tt.want)
565+
t.Fatalf("got %s, want %s", got, want)
566+
}
567+
})
568+
}
569+
}
570+
498571
// TestRegressionBytes covers regressions requiring direct EvalBytes
499572
// access (e.g. large integers that exceed float64 precision).
500573
func TestRegressionBytes(t *testing.T) {

functions/object_funcs.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func fnSpread(args []any, _ any) (any, error) {
113113
return result
114114
}
115115
if evaluator.IsMap(args[0]) {
116-
return spreadOne(args[0]), nil
116+
return &evaluator.Sequence{Values: spreadOne(args[0])}, nil
117117
}
118118
if arr, ok := args[0].([]any); ok {
119119
var result []any
@@ -241,18 +241,18 @@ func makeFnEach(evalFn EvalFn) evaluator.EnvAwareBuiltin {
241241
}
242242

243243
keys := evaluator.MapKeys(objVal)
244-
result := make([]any, 0, len(keys))
244+
seq := evaluator.CreateSequence()
245245
for _, ks := range keys {
246246
val, _ := evaluator.MapGet(objVal, ks)
247247
res, err := evalFn(fn, []any{val, ks}, focus, env)
248248
if err != nil {
249249
return nil, err
250250
}
251251
if res != nil {
252-
result = append(result, res)
252+
seq.Values = append(seq.Values, res)
253253
}
254254
}
255-
return result, nil
255+
return seq, nil
256256
}
257257
}
258258

functions/string_match_replace.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,15 @@ func makeFnMatch(evalFn EvalFn) evaluator.EnvAwareBuiltin {
7373
return nil, &evaluator.JSONataError{Code: "D3137", Message: fmt.Sprintf("regex error: %v", matchErr)}
7474
}
7575
}
76-
if len(result) == 0 {
77-
return nil, nil
78-
}
79-
return result, nil
76+
return matchResultSeq(result), nil
77+
}
78+
}
79+
80+
func matchResultSeq(result []any) any {
81+
if len(result) == 0 {
82+
return nil
8083
}
84+
return &evaluator.Sequence{Values: result}
8185
}
8286

8387
func matchWithCustomMatcher(s string, matcherFn any, limit int, evalFn EvalFn, env *evaluator.Environment) (any, error) {
@@ -119,10 +123,7 @@ func matchWithCustomMatcher(s string, matcherFn any, limit int, evalFn EvalFn, e
119123
}
120124
}
121125

122-
if len(result) == 0 {
123-
return nil, nil
124-
}
125-
return result, nil
126+
return matchResultSeq(result), nil
126127
}
127128

128129
// ── $replace ──────────────────────────────────────────────────────────────────

internal/evaluator/eval_chain.go

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,7 @@ func evalChain(right *parser.Node, piped, input any, env *Environment) (any, err
3838
if err != nil {
3939
return nil, err
4040
}
41-
if right.KeepArray {
42-
if seq, ok := result.(*Sequence); ok {
43-
result = CollapseSequence(seq)
44-
}
45-
switch result.(type) {
46-
case []any:
47-
return result, nil
48-
case nil:
49-
return nil, nil
50-
default:
51-
return []any{result}, nil
52-
}
53-
}
54-
return result, nil
41+
return CollapseAndKeep(result, right.KeepArray), nil
5542
}
5643
// Right is a function reference or other expression.
5744
fn, err := Eval(right, input, env)
@@ -87,10 +74,19 @@ func evalChain(right *parser.Node, piped, input any, env *Environment) (any, err
8774
if err != nil {
8875
return nil, err
8976
}
90-
return callFunction(fn, []any{intermediate}, focus, env)
77+
intermediate = CollapseAndKeep(intermediate, false)
78+
res, err := callFunction(fn, []any{intermediate}, focus, env)
79+
if err != nil {
80+
return nil, err
81+
}
82+
return CollapseAndKeep(res, false), nil
9183
}), nil
9284
}
93-
return callFunction(fn, []any{piped}, input, env)
85+
result, err := callFunction(fn, []any{piped}, input, env)
86+
if err != nil {
87+
return nil, err
88+
}
89+
return CollapseAndKeep(result, false), nil
9490
}
9591

9692
func evalBlock(node *parser.Node, input any, env *Environment) (any, error) {

internal/evaluator/eval_function.go

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -76,23 +76,7 @@ func evalFunction(node *parser.Node, input any, env *Environment) (any, error) {
7676
if err != nil {
7777
return nil, err
7878
}
79-
// Normalize Sequence results: jsonata-js collapses Sequence values
80-
// after every function call. Single-element → scalar, multi → []any.
81-
// Functions returning *Sequence ($keys, $distinct) rely on this.
82-
if seq, ok := result.(*Sequence); ok {
83-
result = CollapseSequence(seq)
84-
}
85-
if node.KeepArray {
86-
switch result.(type) {
87-
case []any:
88-
return result, nil
89-
case nil:
90-
return nil, nil
91-
default:
92-
return []any{result}, nil
93-
}
94-
}
95-
return result, nil
79+
return CollapseAndKeep(result, node.KeepArray), nil
9680
}
9781

9882
func evalLambda(node *parser.Node, input any, env *Environment) (any, error) {

internal/evaluator/value.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,32 @@ func CollapseSequence(s *Sequence) any {
6565
}
6666
}
6767

68+
// CollapseAndKeep normalizes a function call result for callers that need
69+
// KeepArray (the [] suffix) support. Builtins returning *Sequence rely on
70+
// CollapseSequence; when keepArray is true, singletons are preserved as
71+
// one-element arrays instead of being unwrapped.
72+
func CollapseAndKeep(result any, keepArray bool) any {
73+
if seq, ok := result.(*Sequence); ok {
74+
if keepArray {
75+
s := *seq
76+
s.KeepSingleton = true
77+
seq = &s
78+
}
79+
result = CollapseSequence(seq)
80+
}
81+
if keepArray {
82+
switch result.(type) {
83+
case []any:
84+
return result
85+
case nil:
86+
return nil
87+
default:
88+
return []any{result}
89+
}
90+
}
91+
return result
92+
}
93+
6894
// IsArray returns true for []any values (not *Sequence).
6995
func IsArray(v any) bool {
7096
_, ok := v.([]any)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"expr": "$each({'a': 1}, function($v, $k) { $k })",
3+
"data": {},
4+
"bindings": {},
5+
"result": "a"
6+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"expr": "$each({'a': 1}, function($v, $k) { [1, 2, 3] })",
3+
"data": {},
4+
"bindings": {},
5+
"result": [1, 2, 3]
6+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"expr": "$each({'a': 1}, function($v, $k) { $k })[]",
3+
"data": {},
4+
"bindings": {},
5+
"result": ["a"]
6+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"expr": "$each({'x': {'p': 1, 'q': 2}}, function($v, $k) { $each($v, function($val, $key) { $k & '.' & $key }) })",
3+
"data": {},
4+
"bindings": {},
5+
"result": ["x.p", "x.q"],
6+
"unordered": true
7+
}

0 commit comments

Comments
 (0)