Repository: jsonata-js/jsonata (js)

Issue:
Proposal:  Add short-circuit evaluation for conditionals
Consider the following input:
`{"foo":""}`
and the following JSONata expression:
`foo != "" and $number(foo) > 0`
If short-circuit conditionals existed in JSONata, then the result would be `false`.  However, because these do not exist in JSONata, the following error is produced:
`Unable to cast value to a number: ""`

However, currently a work around is possible with `foo != "" ? $number(foo) > 0 : false` 

Pull request description:
Make boolean operators short-circuit
My first ever PR to an open-source project. Woo!

Fixes #338 and #505.

Defers execution of the right-hand side of boolean expressions. Uses the natural short-circuiting behavior of Javascript's `||` and `&&` operators to evaluate only when needed.

Patch:
diff --git a/src/jsonata.js b/src/jsonata.js
index a91eb0f..e1d6092 100644
--- a/src/jsonata.js
+++ b/src/jsonata.js
@@ -457,9 +457,21 @@ var jsonata = (function() {
     function * evaluateBinary(expr, input, environment) {
         var result;
         var lhs = yield * evaluate(expr.lhs, input, environment);
-        var rhs = yield * evaluate(expr.rhs, input, environment);
         var op = expr.value;
 
+        //defer evaluation of RHS to allow short-circuiting
+        var evalrhs = function*(){return yield * evaluate(expr.rhs, input, environment);};
+        if (op === "and" || op === "or") {
+            try {
+                return yield * evaluateBooleanExpression(lhs, evalrhs, op);
+            } catch(err) {
+                err.position = expr.position;
+                err.token = op;
+                throw err;
+            }
+        }
+
+        var rhs = yield * evalrhs();
         try {
             switch (op) {
                 case '+':
@@ -482,10 +494,6 @@ var jsonata = (function() {
                 case '&':
                     result = evaluateStringConcat(lhs, rhs);
                     break;
-                case 'and':
-                case 'or':
-                    result = evaluateBooleanExpression(lhs, rhs, op);
-                    break;
                 case '..':
                     result = evaluateRangeExpression(lhs, rhs);
                     break;
@@ -836,35 +844,31 @@ var jsonata = (function() {
     /**
      * Evaluate boolean expression against input data
      * @param {Object} lhs - LHS value
-     * @param {Object} rhs - RHS value
+     * @param {Function} evalrhs - function to evaluate RHS value
      * @param {Object} op - opcode
      * @returns {*} Result
      */
-    function evaluateBooleanExpression(lhs, rhs, op) {
+    function * evaluateBooleanExpression(lhs, evalrhs, op) {
         var result;
 
-        var lBool = fn.boolean(lhs);
-        var rBool = fn.boolean(rhs);
-
-        if (typeof  lBool === 'undefined') {
-            lBool = false;
-        }
-
-        if (typeof  rBool === 'undefined') {
-            rBool = false;
-        }
+        var lBool = boolize(lhs);
 
         switch (op) {
             case 'and':
-                result = lBool && rBool;
+                result = lBool && boolize(yield * evalrhs());
                 break;
             case 'or':
-                result = lBool || rBool;
+                result = lBool || boolize(yield * evalrhs());
                 break;
         }
         return result;
     }
 
+    function boolize(value) {
+        var booledValue = fn.boolean(value);
+        return typeof booledValue === 'undefined' ? false : booledValue;
+    }
+
     /**
      * Evaluate string concatenation against input data
      * @param {Object} lhs - LHS value


Test patch:
diff --git a/test/test-suite/groups/boolean-expresssions/case028.json b/test/test-suite/groups/boolean-expresssions/case028.json
new file mode 100644
index 0000000..7731457
--- /dev/null
+++ b/test/test-suite/groups/boolean-expresssions/case028.json
@@ -0,0 +1,8 @@
+{
+    "description": "will not evaluate rhs (which would error) because lhs is true",
+    "expr": "foo = '' or $number(foo) = 0",
+    "data": {
+        "foo":""
+    },
+    "result": true
+}
diff --git a/test/test-suite/groups/boolean-expresssions/case029.json b/test/test-suite/groups/boolean-expresssions/case029.json
new file mode 100644
index 0000000..f75f8f8
--- /dev/null
+++ b/test/test-suite/groups/boolean-expresssions/case029.json
@@ -0,0 +1,8 @@
+{
+    "description": "will not evaluate rhs (which would error) because lhs is false",
+    "expr": "$type(data) = 'number' and data > 10",
+    "data": {
+        "data":"15"
+    },
+    "result": false
+}
diff --git a/test/test-suite/groups/boolean-expresssions/case030.json b/test/test-suite/groups/boolean-expresssions/case030.json
new file mode 100644
index 0000000..84d63cc
--- /dev/null
+++ b/test/test-suite/groups/boolean-expresssions/case030.json
@@ -0,0 +1,8 @@
+{
+    "description": "throws error on right side of boolean expression (for CC of a catch)",
+    "expr": "$type(age) = 'number' or $number(age) > 0",
+    "data": {
+        "age":"33 1/2"
+    },
+    "code": "D3030"
+}