Skip to content

feat: implement support for operator precedence #17

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

Merged
merged 51 commits into from
Apr 25, 2025

Conversation

josdejong
Copy link
Collaborator

@josdejong josdejong commented Mar 3, 2025

Fixes #16

This is a work in progress to see where we end up when implementing full support for operators and operator precedence.

Changes

  • Makes the parentheses around operators optional and introduces operator precedence, so instead of:
    filter(((.type == "customer") and (.age >= 18)) and (.age <= 65))
    
    You can enter:
    filter(.type == "customer" and .age >= 18 and .age <= 65)
    
  • Changes the functions and, and or to support a variable number of arguments, like and(true, true, true).

Breaking changes

  • The operators option changes from an object like { aboutEq: '~=', ... } into an array with custom operators with name and precedence, like [{ name: 'aboutEq', op: '~=', at: '==', vararg: false, leftAssociative: true }, ...].

To do

  • Come up with a good solution to allow users to implement custom operators, being able to specify their precedence
  • Implement support for operators with more than two arguments in the JSON format, like ["add", 1, 2, 3]. Make pipe a regular operator.
  • Work out a solution for operator associativity
  • Write extensive unit tests for checking all operator precedence
  • Move the tests with custom operators into parse.test.json and stringify.test.json?
  • Update documentation and examples

Copy link

@Hughp135 Hughp135 left a comment

Choose a reason for hiding this comment

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

Nice work, but main concern is if the breaking changes are really necessary?

@josdejong
Copy link
Collaborator Author

Ok I've thought about it and I think it's best to go for the user friendly API (see #17 (comment)), I've now merged the feat/extend-operators branch into this PR, and I've worked out the docs.

So now there is only a breaking change in the operators, not in functions anymore, and both options extend the existing functions and operators, they don't replace them.

@josdejong josdejong marked this pull request as ready for review March 11, 2025 11:30
@josdejong josdejong changed the title feat: implement support for operator precedence (WIP) feat: implement support for operator precedence Mar 11, 2025
@josdejong
Copy link
Collaborator Author

@lateapexearlyspeed I would love to hear your opinion on this PR improving operator support, since you've implemented a version of JSON Query in .Net. What do you think?

@lateapexearlyspeed
Copy link

@josdejong I agree to support operator precedence in json query language, the operators chain is easier to be used by user, thanks .

Just personal opinion:

I think all operators should support "chain", not only those "+-*/ and or ..." operators. It will give user max freedom to write as general program languages, for example user can also write 1 ^ 2 ^ 3. So eventually all necessary concepts for all operators are only precedence and association. Conceptually, 1 ^ 2 ^ 3 can be parsed into AST as (1 ^ 2) ^ 3 and ^ operator model is still Binary Operators.

The 'var arg' concept in specific operators is beneficial to json format and model only, it should not limit other operators' chain in language level, like 1 ^ 2 ^ 3.

Also I notice the pipe operator is defined with highest precedence now. I just feel it should be lowest precedence because it is used as result transition :)

@josdejong
Copy link
Collaborator Author

Thanks for your feedback, very helpful! I'm glad you see this as a valuable improvement too :)

About chaining support: I have been thinking about this. Thing is, for operators like ==, chaining doesn't make sense. What does a>b>c mean? It would be evaluated as (a>b)>c but then you compare a boolean against c, which is odd. If you really have such an expression, I think it doesn't do harm to require explicit parenthesis in that case. Then, there are left associative and right associative operators, which can be confusing. For example ^ is right associative in many programming languages, so 1 ^ 2 ^ 3 is evaluated as 1 ^ (2 ^ 3). I find this confusing however. So I thought: let's not support right associative operators for now, and instead require the user to use parenthesis. That will clear any possible confusion and misreading, so it may be a good thing in general. That's my idea behind only supporting left associative operators for now, and require parenthesis in other cases. What do you think?

Pipe operator precedence: wow, you're right, it should be lowest indeed. Thanks for spotting this. Fixed via 4a6ad69

@lateapexearlyspeed
Copy link

Got your point now, thanks.

# Conflicts:
#	README.md
#	reference/functions.md
@lateapexearlyspeed
Copy link

@josdejong Just confirm: because 1 < 2 < 3 is not allowed, so 1 < 2 * 1 < 3 is also not allowed. (note precedence of * is higher than <)
Is my understand correct ?

@josdejong
Copy link
Collaborator Author

@josdejong Just confirm: because 1 < 2 < 3 is not allowed, so 1 < 2 * 1 < 3 is also not allowed. (note precedence of * is higher than <) Is my understand correct ?

That is correct, 1 < 2 * 1 < 3 would be parsed as 1 < (2 * 1) < 3, but then you have again a chain of a < b < c, which is not supported. So to do this operation, you would have to do (1 < 2 * 1) < 3 or 1 < (2 * 1 < 3).

@josdejong
Copy link
Collaborator Author

On a side note, some programming languages support a < b <= c which is interpreted as a < b and b <= c. That is an interesting one, but let's park that as a future idea.

@josdejong
Copy link
Collaborator Author

@josdejong josdejong merged commit 0d39aeb into develop Apr 25, 2025
4 checks passed
@josdejong josdejong deleted the feat/extend-functions-and-or branch April 25, 2025 06:53
Copy link

🎉 This PR is included in version 5.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for chaining multiple 'or' / 'and' operators
3 participants