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

Complete JSONPath parser #116

Open
V0ldek opened this issue Mar 29, 2023 · 0 comments
Open

Complete JSONPath parser #116

V0ldek opened this issue Mar 29, 2023 · 0 comments
Labels
acceptance: go ahead Reviewed, implementation can start area: app Improvements in overall CLI app usability help wanted External contributions welcome type: feature New feature or request
Milestone

Comments

@V0ldek
Copy link
Member

V0ldek commented Mar 29, 2023

Is your feature request related to a problem? Please describe.
Currently our approach was to bundle parsing JSONPath selectors a part of enabling support for them entirely, as the first step in the process.

Pros of the current approach

The pros of that approach is that we're rather immune to any syntax changes that could be proposed in further refinements of the JSONPath RFC, and the parser is relatively small.

Cons of the current approach

The cons are threefold, engineering-wise, UX-wise, and from the usability of rsonpath-lib as an API.

UX

As of this writing we don't have descendant wildcards yet (#68), but we do have child wildcards. What happens if a user tries to query for $.data[*]..*.value?

Error: 0: Could not parse JSONPath query. 1: One or more syntax errors occurred. Parse error: $.data[*]..*.value ^ invalid tokens

That's technically correct, but utterly useless. Without that one dot token it compiles, yes, but the true issue here is that this is valid JSONPath, we just don't support the ..* construct yet. Writing proper error messages for this would require the parser to understand ..*, and then an error from the compiler saying "we can't handle this yet".

Engineering

We need to basically maintain a second JSONPath grammar. While it's essentially a subset, we need to document the differences and any additions will result in an effective change of the public API and the CLI experience. It's also hard to maintain the parser's error messages consistent, since what is now an "invalid token" with no further explanation could be a valid token later.

Library

The query and engine modules are separate, and query has a clearly designated compiler part (automaton). We strive to be a comprehensive JSONPath library, which means that the query module should recognise as much of JSONPath as it can without relying on the engine to understand that. That way we make extensibility easier – say we wanted to roll out a DOM-based engine for all of JSONPath tomorrow, because it's vastly easier than a streaming one and still useful. It makes no modular sense to restrict the query API based on what the consumer of that API, automaton and engine, support at the moment.

Describe the solution you'd like

  1. Take the newest RFC proposal and produce a syntax file. Unfortunately the grammar is scattered around the doc and needs to be assembled together. We want the explicit EBNF grammar as part of our docs.

  2. Implement the parser to fulfill that grammar and extend JsonPathQuery to support all constructs. Extensive testing is needed here.

  3. Make error messages nice.

Describe alternatives you've considered
We can just do nothing and take these in stride as new selectors come, but with all the cons it entails (described above).

@V0ldek V0ldek added the type: feature New feature or request label Mar 29, 2023
@V0ldek V0ldek added this to the Future milestone Mar 29, 2023
@V0ldek V0ldek added help wanted External contributions welcome acceptance: go ahead Reviewed, implementation can start mod: parser area: app Improvements in overall CLI app usability labels Mar 29, 2023
@V0ldek V0ldek removed the mod: parser label Oct 4, 2023
@V0ldek V0ldek mentioned this issue Jan 15, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acceptance: go ahead Reviewed, implementation can start area: app Improvements in overall CLI app usability help wanted External contributions welcome type: feature New feature or request
Projects
Development

No branches or pull requests

1 participant