Skip to content

Fix ?: operator returning wrong result when LHS has array predicate#780

Merged
andrew-coleman merged 1 commit intojsonata-js:masterfrom
DevLeoko:fix/elvis-operator-array-predicate
Apr 29, 2026
Merged

Fix ?: operator returning wrong result when LHS has array predicate#780
andrew-coleman merged 1 commit intojsonata-js:masterfrom
DevLeoko:fix/elvis-operator-array-predicate

Conversation

@DevLeoko
Copy link
Copy Markdown

@DevLeoko DevLeoko commented Apr 23, 2026

Quick Note

We are using JSONata quite extensively over at Ambivio and love it!
But we have noticed some issues like this one.

Please review these changes carefully as they have been AI-assisted and I don't have a deep understanding of JSONatas parsing logic.

Summary

The ?: (elvis/default) operator returns the wrong value whenever its left-hand side is an array predicate or a negative numeric literal. For example:

Expression Expected Actual (before fix)
[true][-1] ?: "no" true "no"
[1,2,3][-1] ?: "no" 3 2
array[1] ?: "no" (with array=[10,20]) 20 "no"
-5 ?: 99 -5 5

Root cause

In src/parser.js, the ?: infix handler stored the same left AST node reference into both condition and then:

infix("?:", operators['?:'], function (left) {
    this.type = 'condition';
    this.condition = left;
    this.then = left;        // SAME REFERENCE
    this.else = expression(0);
    return this;
});

The post-parse processAST step then walks both branches, but processAST mutates the AST it receives:

  • The [ (predicate) handler pushes a filter stage onto the LHS step. Run twice on the same node, you get two filters — so array[1] becomes effectively array[1][1].
  • The unary-- handler folds -N into the underlying number literal by mutating its value in place. Run twice, the negation is undone — so -5 becomes 5.

This is the exact same class of bug that was fixed for the ?? operator in #774 / commit fd47061 (closing #773). The fix was never propagated to ?:.

Fix

Deep-clone left for the condition branch so the two branches have independent AST nodes — mirroring the ?? fix:

infix("?:", operators['?:'], function (left) {
    this.type = 'condition';
    this.condition = JSON.parse(JSON.stringify(left));
    this.then = left;
    this.else = expression(0);
    return this;
});

Tests

Added 5 new test cases under test/test-suite/groups/default-operator/:

  • case014.json — array predicate on LHS exists, returns LHS value
  • case015.json — array predicate on LHS does not exist, returns RHS value
  • case016.json — negative array index on multi-element array
  • case017.json — negative array index on single-element array
  • case018.json — unary minus on number literal LHS is not double-negated

Full suite passes locally: 1768 passing, 100% coverage maintained.

Test plan

  • npm test passes (1768 tests)
  • 100% coverage maintained
  • All cases from the linked issue return correct values
  • Existing default-operator tests still pass

Signed-off-by: Leo Garbe leo@respark.dev

The ?: infix handler reused the same `left` AST node reference for both
the `condition` and the `then` branch. During AST post-processing, this
caused the same node to be mutated twice — filter stages were pushed
onto the shared node twice (turning e.g. `array[1]` into `array[1][1]`)
and the unary-minus folding on number literals was applied twice
(turning e.g. `-5` back into `5`).

Deep-clone `left` for the condition argument so the two branches have
independent AST nodes. This mirrors the fix applied to the `??`
operator in jsonata-js#774 / commit fd47061 (closing jsonata-js#773).

Includes new test cases under default-operator covering array
predicates (positive index, out-of-range, negative index on
multi-/single-element arrays) and unary-minus literal LHS.

Signed-off-by: Leo Garbe <leo@respark.dev>
Made-with: Cursor
Copy link
Copy Markdown
Member

@andrew-coleman andrew-coleman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thank you!

@andrew-coleman andrew-coleman merged commit accd4d3 into jsonata-js:master Apr 29, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants