Skip to content

jq directives for jsondocck#158269

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
kohsine:add-jq
Jul 11, 2026
Merged

jq directives for jsondocck#158269
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
kohsine:add-jq

Conversation

@kohsine

@kohsine kohsine commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

View all comments

Closes #142479.

Adds jq directives for jsondocck.

I decided to add jq instead of replacing jsonpath entirely so that migration could happen incrementally. In theory, it should be possible to replace every jsonpath test case with the new jq directives. Moving forward, this might put more burden on reviewers as test case writers would have the option to use jq or jsonpath or even both. Note that you cannot use jq_set with jsonpath directives or set with jq directives. Diff should be reviewed without whitespace.

r? @aDotInTheVoid

@rustbot

rustbot commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging.

cc @davidtwco, @wesleywiser

compiletest directives have been modified. Please add or update docs for the
new or modified directive in src/doc/rustc-dev-guide/.

Some changes occurred in src/tools/compiletest

cc @jieyouxu

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.

These commits modify tests/rustdoc-json.
rustdoc-json is a public (but unstable) interface.

Please ensure that if you've changed the output:

  • It's intentional.
  • The FORMAT_VERSION in src/librustdoc-json-types is bumped if necessary.

cc @aDotInTheVoid, @obi1kenobi

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-rustdoc-json Area: Rustdoc JSON backend A-testsuite Area: The testsuite used to check the correctness of rustc labels Jun 22, 2026
@rustbot

rustbot commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Failed to set assignee to aDotInTheVoid: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@rustbot rustbot added A-tidy Area: The tidy tool S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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. labels Jun 22, 2026

@aDotInTheVoid aDotInTheVoid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for working on this! This is a really good start.


This should also update the docs in src/doc/rustc-dev-guide/src/rustdoc-internals/rustdoc-json-test-suite.md

Also, I know we don't have them at the moment, but it would be nice to have unit-tests for this. (Grep for jsondoclint for details about how to wire them into the overall build/ci system).

Finally, I can't review/approve this (rust-lang/team#2523), someone still on the rustdoc team will have to.

View changes since this review

Comment thread src/tools/jsondocck/src/directive.rs Outdated
Comment thread src/tools/jsondocck/src/directive.rs Outdated
Comment thread src/tools/jsondocck/src/directive.rs Outdated
@aDotInTheVoid

Copy link
Copy Markdown
Member

r? @lolbinarycat (Discussed this at all-hands, feel free to re-assign).

CC @fmease (Also discussed vague ideas about rustdoc testing)

@rustbot

rustbot commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

lolbinarycat is currently at their maximum review capacity.
They may take a while to respond.

@kohsine

kohsine commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Mirrored the jsonpath directives for jq except for has <path> and !has <path> which are useless because jq returns null for both non-existent paths and existent paths with null values.

Conversions from has "$.foo.bar" will probably look like jq_is '.foo | has("bar")' true.

Conversions from !has "$.foo.bar" will probably look like jq_is '.foo | has("bar")' false.

Unfortunately this means that we lose error messages from !has <path>. has <path> didn't have good error messages in the first place. We still get good error messages from all the other directives though.

@kohsine kohsine requested a review from aDotInTheVoid June 23, 2026 20:02
@kohsine kohsine changed the title jq and jqset directives jq directives for jsondocck Jun 23, 2026

@davidtwco davidtwco left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

License/dependency changes look fine

View changes since this review

@lolbinarycat lolbinarycat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One big design concern (the two variable namespaces), and a bunch of potential readability and diagnostic improvements.

Would be nice if the new directives could be added to rustc-dev-guide under rustdoc-json-test-suite, but that can also be done as a followup.

View changes since this review

Comment thread src/tools/jsondocck/src/cache.rs Outdated
Comment thread src/tools/jsondocck/src/directive.rs Outdated
Comment thread src/tools/jsondocck/src/directive.rs
Comment thread src/tools/jsondocck/src/directive.rs Outdated
Comment thread src/tools/jsondocck/src/directive.rs Outdated
Comment thread src/tools/jsondocck/src/directive.rs Outdated
Comment thread src/tools/jsondocck/src/directive.rs Outdated
Comment thread src/tools/jsondocck/src/cache.rs
Comment thread src/tools/jsondocck/src/directive.rs Outdated
Comment thread src/tools/jsondocck/src/directive.rs Outdated
@lolbinarycat

Copy link
Copy Markdown
Contributor

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 7, 2026
@rustbot

rustbot commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rust-log-analyzer

This comment has been minimized.

@lolbinarycat

Copy link
Copy Markdown
Contributor

looking good! just merge the variable namespaces, then squash the commits down to something more reasonable, and it should be good to merge.

Co-authored-by: lolbinarycat <dogedoge61+github@gmail.com>
@kohsine

kohsine commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 8, 2026
@lolbinarycat

Copy link
Copy Markdown
Contributor

Thanks!

@bors r+

@rust-bors

rust-bors Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 3ed6acb has been approved by lolbinarycat

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 Jul 10, 2026
rust-bors Bot pushed a commit that referenced this pull request Jul 11, 2026
Rollup of 15 pull requests

Successful merges:

 - #159106 (Update LLVM)
 - #157690 (codegen_ssa: pack small const aggregates into immediate stores)
 - #159005 (Use `as_lang_item` instead of repeatedly matching)
 - #156735 (Move NativeLib::filename to the rmeta-link archive member)
 - #157153 (allow `Allocator`s to be used as `#[global_allocator]`s)
 - #158269 (jq directives for jsondocck)
 - #158767 (merge DefKind::InlineConst into AnonConst)
 - #159099 (Stabilize String::from_utf8_lossy_owned)
 - #158930 (Reorganize `tests/ui/issues` [20/N])
 - #158965 (Add codegen test for Result is_ok unwrap)
 - #158979 (Reorganize `tests/ui/issues` [21/N])
 - #159050 (assert only opaques with sub unified hidden infer are non-rigid)
 - #159062 (Remove unused WEAK_ONLY_LANG_ITEMS static)
 - #159070 (Add `io::ErrorKind::InputOutputError`)
 - #159092 (make volatile operations const)
rust-bors Bot pushed a commit that referenced this pull request Jul 11, 2026
Rollup of 15 pull requests

Successful merges:

 - #159106 (Update LLVM)
 - #157690 (codegen_ssa: pack small const aggregates into immediate stores)
 - #159005 (Use `as_lang_item` instead of repeatedly matching)
 - #156735 (Move NativeLib::filename to the rmeta-link archive member)
 - #157153 (allow `Allocator`s to be used as `#[global_allocator]`s)
 - #158269 (jq directives for jsondocck)
 - #158767 (merge DefKind::InlineConst into AnonConst)
 - #159099 (Stabilize String::from_utf8_lossy_owned)
 - #158930 (Reorganize `tests/ui/issues` [20/N])
 - #158965 (Add codegen test for Result is_ok unwrap)
 - #158979 (Reorganize `tests/ui/issues` [21/N])
 - #159050 (assert only opaques with sub unified hidden infer are non-rigid)
 - #159062 (Remove unused WEAK_ONLY_LANG_ITEMS static)
 - #159070 (Add `io::ErrorKind::InputOutputError`)
 - #159092 (make volatile operations const)
@rust-bors rust-bors Bot merged commit 5b0d5c2 into rust-lang:main Jul 11, 2026
13 checks passed
@rustbot rustbot added this to the 1.99.0 milestone Jul 11, 2026
rust-timer added a commit that referenced this pull request Jul 11, 2026
Rollup merge of #158269 - kohsine:add-jq, r=lolbinarycat

jq directives for jsondocck

Closes #142479.

Adds `jq` directives for `jsondocck`.

I decided to add jq instead of replacing jsonpath entirely so that migration could happen incrementally. In theory, it should be possible to replace every jsonpath test case with the new `jq` directives. Moving forward, this might put more burden on reviewers as test case writers would have the option to use jq or jsonpath or even both. ~~Note that you cannot use `jq_set` with jsonpath directives or `set`  with jq directives~~. Diff should be reviewed without whitespace.

r? @aDotInTheVoid
@lolbinarycat

Copy link
Copy Markdown
Contributor

hey, I was working on writing the docs for this when I noticed some more issues:

  1. jq_has does not support the single argument mode
  2. !count and !jq_count do not exist, but are listed in KNOWN_JSONDOCCK_DIRECTIVE_NAMES

@kohsine

kohsine commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

jq_has does not support the single argument mode

jq_has <filter> is useless because jq returns [null] on both existent path with value null and non-existent path. Unlike jsonpath that returns [null] on existent path with value null and returns [] on non-existent path. i.e. with jq the code can't tell whether the path exists and has null value or doesn't exist at all but the whole point of the has directive is to check for path existence.

Test writers will have to do something like jq_is '.foo | has("bar")' true to check for path existence. Unfortunately this means error messages will get swallowed by jq for this directive.

!count and !jq_count do not exist, but are listed in KNOWN_JSONDOCCK_DIRECTIVE_NAMES

Sorry, I think I just copy and pasted the jsonpath directives without really checking. It does seem that !count and !jq_count are not being used.

pull Bot pushed a commit to xtqqczze/rust-lang-miri that referenced this pull request Jul 12, 2026
Rollup of 15 pull requests

Successful merges:

 - rust-lang/rust#159106 (Update LLVM)
 - rust-lang/rust#157690 (codegen_ssa: pack small const aggregates into immediate stores)
 - rust-lang/rust#159005 (Use `as_lang_item` instead of repeatedly matching)
 - rust-lang/rust#156735 (Move NativeLib::filename to the rmeta-link archive member)
 - rust-lang/rust#157153 (allow `Allocator`s to be used as `#[global_allocator]`s)
 - rust-lang/rust#158269 (jq directives for jsondocck)
 - rust-lang/rust#158767 (merge DefKind::InlineConst into AnonConst)
 - rust-lang/rust#159099 (Stabilize String::from_utf8_lossy_owned)
 - rust-lang/rust#158930 (Reorganize `tests/ui/issues` [20/N])
 - rust-lang/rust#158965 (Add codegen test for Result is_ok unwrap)
 - rust-lang/rust#158979 (Reorganize `tests/ui/issues` [21/N])
 - rust-lang/rust#159050 (assert only opaques with sub unified hidden infer are non-rigid)
 - rust-lang/rust#159062 (Remove unused WEAK_ONLY_LANG_ITEMS static)
 - rust-lang/rust#159070 (Add `io::ErrorKind::InputOutputError`)
 - rust-lang/rust#159092 (make volatile operations const)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-compiletest Area: The compiletest test runner A-rustdoc-json Area: Rustdoc JSON backend A-testsuite Area: The testsuite used to check the correctness of rustc A-tidy Area: The tidy tool 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.

jsondocck: replace JSONPath with jq as json query language.

6 participants