Repository: jsonata-js/jsonata (js)

Issue:
Binding syntax
If you evaluate the following JSONata expression, you get what I would argue is an unintuitive result:

```
($a := [1,2]; $a[1]=3; $a;)
```

One might expect the answer to be `[1,3]`.  Or at least that if you evaluate:

```
($a := [1,2]; $a[1]=3; $a[1];)
```

you would expect to get `3`.

It seems to me that the middle statement in both blocks should either be a syntax error (LHS must be a pure variable expression) or at least a semantic error (binding to non-variable expressions is not allowed).  But this doesn't trigger either.  Clearly, the middle statement doesn't seem to be having any effect since the answer remains `[1,2]`.  To be honest, I haven't dug deeply enough into this to know exactly what effect the middle statement is having, but it seems to me this is irrelevant since the bigger question is how should that statement be evaluated?

Personally, I think it should be an error.  It seems reasonably easy to detect.

Comments?

Pull request:
move validation of LHS of := from evaluator to parser
The left hand side of the binding operator `:=` must be a variable.  This PR moves the validation of this rule from the evaluator (which throws a dynamic error) to the parser (throws a syntax error).  This is also more robust since it is checking against the token type rather than a node in the AST.

Resolves #148 which was ignoring the predicate attached to the variable in the AST.

Patch:
diff --git a/jsonata.js b/jsonata.js
index 3f56822..69d166c 100644
--- a/jsonata.js
+++ b/jsonata.js
@@ -783,14 +783,8 @@ var jsonata = (function() {
         // <expression> <operator> <expression>
         // right associative
         var infixr = function (id, bp, led) {
-            var bindingPower = bp || operators[id];
-            var s = symbol(id, bindingPower);
-            s.led = led || function (left) {
-                this.lhs = left;
-                this.rhs = expression(bindingPower - 1); // subtract 1 from bindingPower for right associative operators
-                this.type = "binary";
-                return this;
-            };
+            var s = symbol(id, bp);
+            s.led = led;
             return s;
         };
 
@@ -836,7 +830,6 @@ var jsonata = (function() {
         terminal("and"); // the 'keywords' can also be used as terminals (field names)
         terminal("or"); //
         terminal("in"); //
-        infixr(":="); // bind variable
         prefix("-"); // unary numeric negation
         infix("~>"); // function application
 
@@ -1056,6 +1049,23 @@ var jsonata = (function() {
         // object grouping
         infix("{", operators['{'], objectParser);
 
+        // bind variable
+        infixr(":=", operators[':='], function(left) {
+            if(left.type !== 'variable') {
+                return handleError( {
+                    code: "S0212",
+                    stack: (new Error()).stack,
+                    position: left.position,
+                    token: left.value
+                });
+            }
+            this.lhs = left;
+            this.rhs = expression(operators[':='] - 1); // subtract 1 from bindingPower for right associative operators
+            this.type = "binary";
+            return this;
+
+        });
+
         // if/then/else ternary operator ?:
         infix("?", operators['?'], function (left) {
             this.type = 'condition';
@@ -2270,17 +2280,8 @@ var jsonata = (function() {
      */
     function* evaluateBindExpression(expr, input, environment) {
         // The RHS is the expression to evaluate
-        // The LHS is the name of the variable to bind to - should be a VARIABLE token
+        // The LHS is the name of the variable to bind to - should be a VARIABLE token (enforced by parser)
         var value = yield * evaluate(expr.rhs, input, environment);
-        if (expr.lhs.type !== 'variable') {
-            throw {
-                code: "D2005",
-                stack: (new Error()).stack,
-                position: expr.position,
-                token: expr.value,
-                value: expr.lhs.type === 'path' ? expr.lhs.steps[0].value : expr.lhs.value
-            };
-        }
         environment.bind(expr.lhs.value, value);
         return value;
     }
@@ -4803,6 +4804,7 @@ var jsonata = (function() {
         "S0209": "A predicate cannot follow a grouping expression in a step",
         "S0210": "Each step can only have one grouping expression",
         "S0211": "The symbol {{token}} cannot be used as a unary operator",
+        "S0212": "The left side of := must be a variable name (start with $)",
         "S0301": "Empty regular expressions are not allowed",
         "S0302": "No terminating / in regular expression",
         "S0402": "Choice groups containing parameterized types are not supported",
@@ -4823,7 +4825,7 @@ var jsonata = (function() {
         "T2002": "The right side of the {{token}} operator must evaluate to a number",
         "T2003": "The left side of the range operator (..) must evaluate to an integer",
         "T2004": "The right side of the range operator (..) must evaluate to an integer",
-        "D2005": "The left side of := must be a variable name (start with $)",
+        "D2005": "The left side of := must be a variable name (start with $)",  // defunct - replaced by S0212 parser error
         "T2006": "The right side of the function application operator ~> must be a function",
         "T2007": "Type mismatch when comparing values {{value}} and {{value2}} in order-by clause",
         "T2008": "The expressions within an order-by clause must evaluate to numeric or string values",


Test patch:
diff --git a/test/test-suite/groups/errors/case017.json b/test/test-suite/groups/errors/case017.json
index ffbe4d0..6e54f82 100644
--- a/test/test-suite/groups/errors/case017.json
+++ b/test/test-suite/groups/errors/case017.json
@@ -2,5 +2,5 @@
     "expr": "x:=1",
     "dataset": null,
     "bindings": {},
-    "code": "D2005"
+    "code": "S0212"
 }
\ No newline at end of file
diff --git a/test/test-suite/groups/errors/case018.json b/test/test-suite/groups/errors/case018.json
index 247ae99..9dd7515 100644
--- a/test/test-suite/groups/errors/case018.json
+++ b/test/test-suite/groups/errors/case018.json
@@ -2,5 +2,5 @@
     "expr": "2:=1",
     "dataset": null,
     "bindings": {},
-    "code": "D2005"
+    "code": "S0212"
 }
\ No newline at end of file
diff --git a/test/test-suite/groups/variables/case012.json b/test/test-suite/groups/variables/case012.json
new file mode 100644
index 0000000..f8ce00c
--- /dev/null
+++ b/test/test-suite/groups/variables/case012.json
@@ -0,0 +1,6 @@
+{
+    "expr": "($a := [1,2]; $a[1]:=3; $a)",
+    "dataset": null,
+    "bindings": {},
+    "code": "S0212"
+}
\ No newline at end of file