Skip to content

Compile time path #101

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

thehiddenwaffle
Copy link
Contributor

Here's the big one, lots of design choices to be made. I tried to keep everything as simple as possible for now but in the future I could definitely remove a couple hundred lines(ToTokens impls mostly) with declarative macros and once it stabilizes and has a full test suite I can work on better error reporting from the macro side and the FromPest side

I have tried to list out all the options that I have thought of, as well as the one that I think is best but I'm certainly not going to make those choices without presenting them to you and ensuring you agree. These are just my suggestions and I'm happy to expand on my thought process but ultimately it's your choice of course.

In this PR I have not integrated anything I created with the main functionality of this repository because there are still design choices to make, because its a 2600 line addition, and because it's barely tested.

Summary:

  • Added a new crate with a common ast called jsonpath-ast
    • Common structures shared between syn and pest, allowing them to parse into the same things
    • All syn related implementations are locked behind a crate feature "compiled-path", ensuring that users who do not need compiled paths via the json_path! macro will not suffer longer build times
      • Work could still be done to completely eliminate syn and other macro related dependencies when "compiled-path" is toggled off, currently not possible
    • Includes some helper types(PestIgnoredPunctuated, PestLiteralWithoutRule) for when pest checks that punctuation exist but doesn't parse it, so syn still needs to check that it exists, and FromPest needs to ignore it(because it already guaranteed it) during pest::Rules -> AST conversion
    • Current ToTokens implementation has each struct generate itself as tokens, ie: Main writes out "Main::new(....inners)"
      • Could be changed to be anything at all(see Macro Output below)
  • Added new proc macro crate jsonpath-rust-impl
    • Provides the json_path! proc macro, depends on jsonpath-ast with feature "compiled-path" enabled
    • Has a test suite I generated using the basic.json from the rfc and trybuild. I think this would be a good approach, but I'm open to better ideas and therefore only did basic.json not the whole RFC
      • Even with trybuild there are still choices, see Testing below
    • Not everything can be parsed by syn at compile time, see Macro Limitations below

Macro Limitations:

  • single quote strings break the macro parser because in rust only chars are single quoted
    • rust has r"# #" strings so in my opinion this is no impact, we can provide a sed regex?
  • due to constraints rust has put on it's identifiers, emoji in member_name_shorthand is not allowed(works in both string literals and bracket field accessjson_query!( $["☺"] ))
  • we could solve these if we changed the input to a string literal, however:
    • this would make error messages and syntax highlighting severely more complex to implement, and currently syn is giving them to us absolutely free(in my opinion the limitations are acceptable and we should not have the macro take strings because any path is still buildable with some tweaks like changing to bracket field access or converting strings)

Macro Output:

  • Currently, the macro just outputs AST literals, though of course it could be made to output anything, so either:
    • We implement Query for the AST nodes and just delete the existing types
    • Provide impls like From<JPQueryAST> for JPQuery for all existing structs then just have the macro call .into()
    • We have the macro output the current types(my least favorite, probably error prone and somewhat redundant)

Testing:

  • If using trybuild, we can either:
    • Create files dynamically from rfc test case file and then delete afterward to ensure accuracy and account for changes in the suite?
    • Just statically build the suite(I made a quick script), how often could it even change?(to me this feels like the answer)

Introduces `jsonpath-rust-impl` crate as a procedural macro for JSONPath parsing and validation. Includes AST definitions, Pest-based grammar, and integration with the main library. Starts refining error handling and debugging within parsing logic.
Introduce `parse_terminated_nonempty` to enforce nonempty parsing for `PestIgnoredPunctuated`. Updated relevant AST nodes to use this stricter parsing function, ensuring better validation and error handling for empty input cases.
These tests validate that the `json_query!` macro correctly rejects various malformed JSONPath expressions. The added cases cover scenarios like empty segments, unexpected tokens, and improper syntax for selectors.
@thehiddenwaffle
Copy link
Contributor Author

thehiddenwaffle commented May 20, 2025

And a topical meme to brighten your day:
meme

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

Successfully merging this pull request may close these issues.

1 participant