Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Binding syntax #148

Closed
xogeny opened this issue Feb 14, 2018 · 4 comments
Closed

Binding syntax #148

xogeny opened this issue Feb 14, 2018 · 4 comments

Comments

@xogeny
Copy link
Contributor

xogeny commented Feb 14, 2018

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?

@andrew-coleman
Copy link
Member

The middle statement, $a[1]=3 is an equality comparison which in this case will evaluate to false (although it's irrelevant what it evaluates to because the block as a whole just returns $a, which is [1,2]). I presume you meant the middle expression to be $a[1]:=3 in an attempt to mutate the array. That should throw an error, because the LHS of the binding operator should be just a variable. However, it looks like there is a bug in the parser that is allowing this (it's seeing that the LHS is a variable token, but ignoring the predicate).

At the moment, JSONata does allow a variable to be re-bound within a block. E.g. ($a := [1,2]; $a:=[1,3]; $a) will return [1,3]. Whether that should be allowed is open to debate.

@xogeny
Copy link
Contributor Author

xogeny commented Feb 14, 2018

Ugh! Yes, I meant $a[1] := 3.

Yes, I see that in evaluateBindExpression, it checks to see if it is a variable (but ignores the predicates). So this is treated as a run-time semantic error. That seems fine to me.

As for re-assignment, I don't have any issue with that. I'll leave this issue open so it can be addressed in the 1.5.x code base. But I'll ensure that it is resolved in the version I'm working with.

@andrew-coleman
Copy link
Member

Interesting. I can't imagine why I left this to be caught by the evaluator when it could be picked up by the parser as a syntax error. I'll change that.

@xogeny
Copy link
Contributor Author

xogeny commented Feb 15, 2018

@andrew-coleman Yes, I was wondering about that. I implemented in the evaluator just to mimic 1.5.x, but I'll move it to the parser. Throwing it as a syntax error is preferable because you'll get immediate feedback that the expression itself is invalid.

mtiller added a commit to mtiller/jsonata that referenced this issue Mar 7, 2018
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

No branches or pull requests

2 participants