Rustdoc: JSON backend experimental impl, with new tests. #79539

aDotInTheVoid · 2020-11-29T20:53:24Z

The first commit is all of #75114, but squased to 1 commit, as that was much easier to rebase onto master.

The git history is a mess, but I think I'll edit it after review, so it's obvious whats new.

Still to do

Update docs.
Add bless option to tests.
Add test option for multiple files in same crate.
Decide if the tests should check for json to be equal or subset.
Go through the rest of the review for the original pr. (This is open because the test system is done(ish), but stuff like not using a hashmap and using CRATE_DEF_INDEX hasn't)

I'm also sure how many of these we need to do before landing on nightly, as it would be nice to get this in tree, so it isn't effected by churn like #79125, #79041, #79061

r? @jyn514

jyn514

I'm also sure how many of these we need to do before landing on nightly, as it would be nice to get this in tree, so it isn't effected by churn like #79125, #79041, #79061

Most of it can wait until later I think (although it should be done before stabilization). The things I'd consider blockers are:

~~Fixing the test suite, which currently does nothing. This should be as simple as reverting a66a97a.~~ I see you already did, thank you! ❤️
Anything changing rustdoc's normal behavior, such as https://github.com/rust-lang/rust/pull/75114/files/a66a97ac588f26350cab886904ddbe70ce17d339#r519470764. It doesn't necessarily have to be reverted, but if not there should be an explanation for why it changed. I understand you're not the one who changed it and it makes things more difficult - maybe revert it, run the tests, and see what breaks?
Stripped items should not be included by default: JSON backend experimental impl #75114 (comment). This is totally against the stability guarantees of rust - the behavior around --document-private-items is a little unclear and we can have that discussion before stabilization, but including private items by default seems very wrong to me.

Other than that, I think this has been in limbo long enough and should get merged ASAP.

cc @tmandry, all is not lost 😆

src/tools/compiletest/src/runtest.rs

jyn514 · 2020-11-30T01:30:15Z

src/test/rustdoc-json/compare.py

+            if expected_type == str and actual.startswith(base_dir):
+                if actual.replace(base_dir + "/", "") != expected:
+                    raise SubsetException(
+                        "expected `{}`, got: `{}`".format(
+                            expected, actual.replace(base_dir + "/", "")


What is this doing?

This is a hack to get around the real output being "filename": "/home/nixon/upstreams/rust/rust/src/test/rustdoc-json/structs.rs", but the test is "filename": "structs.rs",.

Long term, it's probably usefull to do something like $TEST_BUILD_DIR, like the ui tests have.

Ok, can you leave those examples as a comment in the source? Feel free to change the absolute path to something else as long as it's still clear it's absolute and not relative.

src/librustdoc/json/conversions.rs

aDotInTheVoid · 2020-11-30T22:23:21Z

Anything changing rustdoc's normal behavior, such as https://github.com/rust-lang/rust/pull/75114/files/a66a97ac588f26350cab886904ddbe70ce17d339#r519470764. It doesn't necessarily have to be reverted, but if not there should be an explanation for why it changed. I understand you're not the one who changed it and it makes things more difficult - maybe revert it, run the tests, and see what breaks?

Reverting it seems fine, so I've done that.

Stripped items should not be included by default: #75114 (comment). This is totally against the stability guarantees of rust - the behavior around --document-private-items is a little unclear and we can have that discussion before stabilization, but including private items by default seems very wrong to me.

Done

aDotInTheVoid · 2020-11-30T22:24:47Z

@rustbot modify labels: -S-waiting-on-author +S-waiting-on-review

What do you want done with the git history

jyn514

What do you want done with the git history

It's such a mess by now I don't know if it's worth messing with more 😅 up to you if you want to squash or not.

This looks way better than before :) thanks so much for taking this on! I'm really excited to see what people will build with this.

src/librustdoc/json/conversions.rs

jyn514 · 2020-11-30T22:47:28Z

r=me unless @GuillaumeGomez or @tmandry want to take a look - I have more things I'd like to change, but I'd much rather get this on nightly so it's not stuck in limbo.

GuillaumeGomez · 2020-12-01T09:20:34Z

Looks good to me as well, thanks a lot!

@bors: r=jyn514,GuillaumeGomez

bors · 2020-12-01T09:20:36Z

📌 Commit 97011eb has been approved by jyn514,GuillaumeGomez

…illaumeGomez Rustdoc: JSON backend experimental impl, with new tests. Based on rust-lang#75114 by `@P1n3appl3` The first commit is all of rust-lang#75114, but squased to 1 commit, as that was much easier to rebase onto master. The git history is a mess, but I think I'll edit it after review, so it's obvious whats new. ## Still to do - [ ] Update docs. - [ ] Add bless option to tests. - [ ] Add test option for multiple files in same crate. - [ ] Decide if the tests should check for json to be equal or subset. - [ ] Go through the rest of the review for the original pr. (This is open because the test system is done(ish), but stuff like [not using a hashmap](rust-lang#75114 (comment)) and [using `CRATE_DEF_INDEX` ](rust-lang#75114 (comment)) hasn't) I'm also sure how many of these we need to do before landing on nightly, as it would be nice to get this in tree, so it isn't effected by churn like rust-lang#79125, rust-lang#79041, rust-lang#79061 r? `@jyn514`

src/librustdoc/json/conversions.rs

Respond to comments and start adding tests Fix re-exports and extern-crate Add expected output tests Add restricted paths Format Fix: associated methods missing in output Ignore stripped items Mark stripped items removing them creates dangling references Fix tests and update conversions Don't panic if JSON backend fails to create a file Fix attribute error in JSON testsuite Move rustdoc-json to rustdoc/ This way it doesn't have to build rustc twice. Eventually it should probably get its own suite, like rustdoc-js, but that can be fixed in a follow-up. Small cleanups Don't prettify json before printing This fully halves the size of the emitted JSON. Add comments [BREAKING CHANGE] rename version -> crate_version [BREAKING CHANGE] rename source -> span Use exhaustive matches Don't qualify imports for DefId Rename clean::{ItemEnum -> ItemKind}, clean::Item::{inner -> kind} Fix Visibility conversion clean::Visability was changed here: https://github.com/rust-lang/rust/pull/77820/files#diff-df9f90aae0b7769e1eb6ea6f1d73c1c3f649e1ac48a20e169662a8930eb427beL1467-R1509 More churn catchup Format

Move rustdoc/rustdoc-json to rustdoc-json Scaffold rustdoc-json test mode Implement run_rustdoc_json_test Fix up python Make tidy happy

Go back to CRATE_DEF_INDEX Minor niceness improvements Don't output hidden items Remove striped items from fields Add $TEST_BASE_DIR Small catch

jyn514 · 2020-12-01T19:06:40Z

@bors r=jyn514,GuillaumeGomez

bors · 2020-12-01T19:06:41Z

📌 Commit 6018200 has been approved by jyn514,GuillaumeGomez

bors · 2020-12-02T06:30:06Z

⌛ Testing commit 6018200 with merge 86c4e2f69b2692667f369170cb8a34a740ef16c9...

bors · 2020-12-02T07:37:35Z

💔 Test failed - checks-actions

aDotInTheVoid · 2020-12-02T08:45:12Z

Windows path separator problem.

__main__.SubsetException: ['source', 'filename']: expected `$TEST_BASE_DIR/structs.rs`, got: `$TEST_BASE_DIR\structs.rs`

src/test/rustdoc-json/compare.py

jyn514 · 2020-12-02T20:51:31Z

@bors r+ rollup=iffy

bors · 2020-12-02T20:51:33Z

📌 Commit 824d5ce57320cc4c1a5d0a343e44650211e7cd12 has been approved by jyn514

jyn514 · 2020-12-02T22:14:15Z

@bors r+ rollup=iffy

bors · 2020-12-02T22:14:16Z

📌 Commit d619271 has been approved by jyn514

bors · 2020-12-02T23:18:50Z

⌛ Testing commit d619271 with merge 7dc1e85...

bors · 2020-12-03T02:00:38Z

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing 7dc1e85 to master...

aDotInTheVoid · 2021-01-24T18:54:54Z

@rustbot modify labels +A-rustdoc-json

rust-highfive assigned jyn514 Nov 29, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 29, 2020

aDotInTheVoid marked this pull request as draft November 29, 2020 21:23

jyn514 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Nov 30, 2020

jyn514 reviewed Nov 30, 2020

View reviewed changes

src/librustdoc/json/conversions.rs Outdated Show resolved Hide resolved

jyn514 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 Nov 30, 2020

aDotInTheVoid marked this pull request as ready for review November 30, 2020 22:24

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 Nov 30, 2020

jyn514 reviewed Nov 30, 2020

View reviewed changes

src/librustdoc/json/conversions.rs Outdated Show resolved Hide resolved

aDotInTheVoid force-pushed the json-mvp branch from 6fcde34 to 97011eb Compare November 30, 2020 22:43

jyn514 mentioned this pull request Nov 30, 2020

JSON backend experimental impl #75114

Closed

jyn514 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 30, 2020

GuillaumeGomez approved these changes Dec 1, 2020

View reviewed changes

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-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Dec 1, 2020

m-ou-se mentioned this pull request Dec 1, 2020

Rollup of 12 pull requests #79595

Closed

m-ou-se reviewed Dec 1, 2020

View reviewed changes

src/librustdoc/json/conversions.rs Outdated Show resolved Hide resolved

P1n3appl3 and others added 3 commits December 1, 2020 18:34

Add tests for rustdoc json

1098cce

Move rustdoc/rustdoc-json to rustdoc-json Scaffold rustdoc-json test mode Implement run_rustdoc_json_test Fix up python Make tidy happy

Address review comments.

40b5470

Go back to CRATE_DEF_INDEX Minor niceness improvements Don't output hidden items Remove striped items from fields Add $TEST_BASE_DIR Small catch

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 2, 2020

jyn514 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 Dec 2, 2020

jyn514 reviewed Dec 2, 2020

View reviewed changes

src/test/rustdoc-json/compare.py Outdated Show resolved Hide resolved

aDotInTheVoid force-pushed the json-mvp branch from ae5e485 to 824d5ce Compare December 2, 2020 20:50

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 2, 2020

Normalize windows path seperators.

d619271

aDotInTheVoid force-pushed the json-mvp branch from 824d5ce to d619271 Compare December 2, 2020 21:48

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 3, 2020

bors merged commit 7dc1e85 into rust-lang:master Dec 3, 2020

rustbot added this to the 1.50.0 milestone Dec 3, 2020

bors mentioned this pull request Dec 3, 2020

Render Markdown in search results #77686

Merged

aDotInTheVoid mentioned this pull request Dec 3, 2020

Tracking issue for RFC 2963: rustdoc JSON backend #76578

Open

5 tasks

aDotInTheVoid deleted the json-mvp branch January 6, 2021 12:27

rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Jan 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rustdoc: JSON backend experimental impl, with new tests. #79539

Rustdoc: JSON backend experimental impl, with new tests. #79539

aDotInTheVoid commented Nov 29, 2020 •

edited

Loading

jyn514 left a comment

jyn514 Nov 30, 2020

aDotInTheVoid Nov 30, 2020

jyn514 Nov 30, 2020

aDotInTheVoid commented Nov 30, 2020

aDotInTheVoid commented Nov 30, 2020

jyn514 left a comment

jyn514 commented Nov 30, 2020

GuillaumeGomez commented Dec 1, 2020

bors commented Dec 1, 2020

jyn514 commented Dec 1, 2020

bors commented Dec 1, 2020

bors commented Dec 2, 2020

bors commented Dec 2, 2020

aDotInTheVoid commented Dec 2, 2020

jyn514 commented Dec 2, 2020

bors commented Dec 2, 2020

jyn514 commented Dec 2, 2020

bors commented Dec 2, 2020

bors commented Dec 2, 2020

bors commented Dec 3, 2020

aDotInTheVoid commented Jan 24, 2021

Rustdoc: JSON backend experimental impl, with new tests. #79539

Rustdoc: JSON backend experimental impl, with new tests. #79539

Conversation

aDotInTheVoid commented Nov 29, 2020 • edited Loading

Still to do

jyn514 left a comment

Choose a reason for hiding this comment

jyn514 Nov 30, 2020

Choose a reason for hiding this comment

aDotInTheVoid Nov 30, 2020

Choose a reason for hiding this comment

jyn514 Nov 30, 2020

Choose a reason for hiding this comment

aDotInTheVoid commented Nov 30, 2020

aDotInTheVoid commented Nov 30, 2020

jyn514 left a comment

Choose a reason for hiding this comment

jyn514 commented Nov 30, 2020

GuillaumeGomez commented Dec 1, 2020

bors commented Dec 1, 2020

jyn514 commented Dec 1, 2020

bors commented Dec 1, 2020

bors commented Dec 2, 2020

bors commented Dec 2, 2020

aDotInTheVoid commented Dec 2, 2020

jyn514 commented Dec 2, 2020

bors commented Dec 2, 2020

jyn514 commented Dec 2, 2020

bors commented Dec 2, 2020

bors commented Dec 2, 2020

bors commented Dec 3, 2020

aDotInTheVoid commented Jan 24, 2021

aDotInTheVoid commented Nov 29, 2020 •

edited

Loading