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

jsondocck: Replace jsonpath_lib with jsonpath-rust #138763

Merged
merged 4 commits into from
Mar 22, 2025

Conversation

aDotInTheVoid
Copy link
Member

The current jsonpath implementation we use isn't spec-compliant, and is buggy. See freestrings/jsonpath#91

To solve it, it's replaced with https://github.com/besok/jsonpath-rust. This is spec-compiant, and doesn't have a really awkward bug we need to always dance around.

Unfortunately, this requires rewriting almost every test, as the behaviour of [?(@, which is extremely common was changed. (But the new behaviour makes way more sense, and isn't buggy with tripply nested selectors)

Unblocks #110406. Makes #100515 much easier as we don't need to explain the broken JSONPath implementation

Best reviewed commit-by-commit. The first does the replacement. The next two rewrite the test-suite mechanically. The last rewrites the test-suite by hand.

r? @GuillaumeGomez

@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 21, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 21, 2025

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Mar 21, 2025
@GuillaumeGomez
Copy link
Member

Nice, thanks!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 21, 2025

📌 Commit 13335e3 has been approved by GuillaumeGomez

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 21, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 21, 2025
…uillaumeGomez

jsondocck: Replace `jsonpath_lib` with `jsonpath-rust`

The current jsonpath implementation we use isn't spec-compliant, and is buggy. See freestrings/jsonpath#91

To solve it, it's replaced with https://github.com/besok/jsonpath-rust. This is spec-compiant, and doesn't have a really awkward bug we need to always dance around.

Unfortunately, this requires rewriting almost every test, as the behaviour of `[?(`@`,` which is *extremely* common was changed. (But the new behaviour makes way more sense, and isn't buggy with tripply nested selectors)

Unblocks rust-lang#110406. Makes rust-lang#100515 much easier as we don't need to explain the broken JSONPath implementation

Best reviewed commit-by-commit. The first does the replacement. The next two rewrite the test-suite mechanically. The last rewrites the test-suite by hand.

r? `@GuillaumeGomez`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 21, 2025
…uillaumeGomez

jsondocck: Replace `jsonpath_lib` with `jsonpath-rust`

The current jsonpath implementation we use isn't spec-compliant, and is buggy. See freestrings/jsonpath#91

To solve it, it's replaced with https://github.com/besok/jsonpath-rust. This is spec-compiant, and doesn't have a really awkward bug we need to always dance around.

Unfortunately, this requires rewriting almost every test, as the behaviour of `[?(``@`,`` which is *extremely* common was changed. (But the new behaviour makes way more sense, and isn't buggy with tripply nested selectors)

Unblocks rust-lang#110406. Makes rust-lang#100515 much easier as we don't need to explain the broken JSONPath implementation

Best reviewed commit-by-commit. The first does the replacement. The next two rewrite the test-suite mechanically. The last rewrites the test-suite by hand.

r? ``@GuillaumeGomez``
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 22, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#136769 (Provide optional `Read`/`Write` methods for stdio)
 - rust-lang#138410 (Couple mir building cleanups)
 - rust-lang#138490 (Forward `stream_position` in `Arc<File>` as well)
 - rust-lang#138535 (Cleanup `LangString::parse`)
 - rust-lang#138536 (stable_mir: Add `MutMirVisitor`)
 - rust-lang#138673 (Fix build failure on Trusty)
 - rust-lang#138750 (Make `crate_hash` not iterate over `hir_crate` owners anymore)
 - rust-lang#138763 (jsondocck: Replace `jsonpath_lib` with `jsonpath-rust`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 22, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#138410 (Couple mir building cleanups)
 - rust-lang#138490 (Forward `stream_position` in `Arc<File>` as well)
 - rust-lang#138535 (Cleanup `LangString::parse`)
 - rust-lang#138536 (stable_mir: Add `MutMirVisitor`)
 - rust-lang#138673 (Fix build failure on Trusty)
 - rust-lang#138750 (Make `crate_hash` not iterate over `hir_crate` owners anymore)
 - rust-lang#138763 (jsondocck: Replace `jsonpath_lib` with `jsonpath-rust`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a4c0173 into rust-lang:master Mar 22, 2025
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 22, 2025
Rollup merge of rust-lang#138763 - aDotInTheVoid:two-years-later, r=GuillaumeGomez

jsondocck: Replace `jsonpath_lib` with `jsonpath-rust`

The current jsonpath implementation we use isn't spec-compliant, and is buggy. See freestrings/jsonpath#91

To solve it, it's replaced with https://github.com/besok/jsonpath-rust. This is spec-compiant, and doesn't have a really awkward bug we need to always dance around.

Unfortunately, this requires rewriting almost every test, as the behaviour of `[?(```@`,``` which is *extremely* common was changed. (But the new behaviour makes way more sense, and isn't buggy with tripply nested selectors)

Unblocks rust-lang#110406. Makes rust-lang#100515 much easier as we don't need to explain the broken JSONPath implementation

Best reviewed commit-by-commit. The first does the replacement. The next two rewrite the test-suite mechanically. The last rewrites the test-suite by hand.

r? ```@GuillaumeGomez```
@rustbot rustbot added this to the 1.87.0 milestone Mar 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants