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

Rustdoc Json: Add tests for Reexports, and improve jsondocck #82571

Merged
merged 2 commits into from
Mar 12, 2021

Conversation

aDotInTheVoid
Copy link
Member

The two changes are orthognal, so you can land just one if you want, but the improved errors realy helped write the tests.

Notably does not have the case from #80664, but I want to have all the ajacent cases tested before starting work on that to ensure I dont break anything.

Improves #81359

cc @CraftSpider

r? @jyn514

@rustbot modify labels: +A-testsuite +T-rustdoc +A-rustdoc-json

@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend A-testsuite Area: The testsuite used to check the correctness of rustc T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Feb 26, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 26, 2021
Copy link
Contributor

@CraftSpider CraftSpider left a comment

Choose a reason for hiding this comment

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

Looks good, I appreciate any improved output. I marked my one nit, which is stylistic not functional.

src/test/rustdoc-json/reexport/glob_extern.rs Outdated Show resolved Hide resolved
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Left a bunch of nits, but overall this looks great :)

Comment on lines +16 to +17
// @has - "$.index[*][?(@.name=='glob_extern')].inner.items[*]" $public_fn_id
pub use mod1::*;
Copy link
Member

Choose a reason for hiding this comment

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

This re-export is inlined, so the JSON shows the items at the crate root?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure I understand the question, but yes, it shows up in the crate root

Output
{
  "crate_version": null,
  "external_crates": {},
  "format_version": 4,
  "includes_private": false,
  "index": {
    "0:0": {
      "attrs": ["#![no_core]", "#![feature(no_core)]"],
      "crate_id": 0,
      "deprecation": null,
      "docs": null,
      "id": "0:0",
      "inner": {"is_crate": true, "items": ["0:3"]},
      "kind": "module",
      "links": {},
      "name": "glob_extern",
      "source": {"begin": [3, 0], "end": [13, 16], "filename": "glob_extern.rs"},
      "visibility": "public"
    },
    "0:3": {
      "attrs": [],
      "crate_id": 0,
      "deprecation": null,
      "docs": null,
      "id": "0:3",
      "inner": {
        "abi": "\"C\"",
        "decl": {"c_variadic": false, "inputs": [], "output": null},
        "generics": {"params": [], "where_predicates": []},
        "header": ["unsafe"]
      },
      "kind": "function",
      "links": {},
      "name": "public_fn",
      "source": {"begin": [8, 8], "end": [8, 27], "filename": "glob_extern.rs"},
      "visibility": "public"
    }
  },
  "paths": {
    "0:0": {"crate_id": 0, "kind": "module", "path": ["glob_extern"]},
    "0:3": {"crate_id": 0, "kind": "function", "path": ["glob_extern", "public_fn"]}
  },
  "root": "0:0"
}

src/test/rustdoc-json/reexport/glob_private.rs Outdated Show resolved Hide resolved
src/tools/jsondocck/src/main.rs Outdated Show resolved Hide resolved
src/tools/jsondocck/src/main.rs Outdated Show resolved Hide resolved
@jyn514
Copy link
Member

jyn514 commented Mar 10, 2021

@bors delegate=CraftSpider

@bors
Copy link
Contributor

bors commented Mar 10, 2021

✌️ @CraftSpider can now approve this pull request

@aDotInTheVoid
Copy link
Member Author

@rustbot modify labels: -S-waiting-on-review +S-waiting-on-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 Mar 10, 2021
@aDotInTheVoid
Copy link
Member Author

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

@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 Mar 10, 2021
@CraftSpider
Copy link
Contributor

Looks nice, thanks for the improvements!
@bors r+

@bors
Copy link
Contributor

bors commented Mar 11, 2021

📌 Commit 5f24798 has been approved by CraftSpider

@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 11, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 11, 2021
…ftSpider

Rustdoc Json: Add tests for Reexports, and improve jsondocck

The two changes are orthognal, so you can land just one if you want, but the improved errors realy helped write the tests.

Notably does not have the case from rust-lang#80664, but I want to have all the ajacent cases tested before starting work on that to ensure I dont break anything.

Improves rust-lang#81359

cc `@CraftSpider`

r? `@jyn514`

`@rustbot` modify labels: +A-testsuite +T-rustdoc +A-rustdoc-json
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 11, 2021
…ftSpider

Rustdoc Json: Add tests for Reexports, and improve jsondocck

The two changes are orthognal, so you can land just one if you want, but the improved errors realy helped write the tests.

Notably does not have the case from rust-lang#80664, but I want to have all the ajacent cases tested before starting work on that to ensure I dont break anything.

Improves rust-lang#81359

cc ``@CraftSpider``

r? ``@jyn514``

``@rustbot`` modify labels: +A-testsuite +T-rustdoc +A-rustdoc-json
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 12, 2021
Rollup of 11 pull requests

Successful merges:

 - rust-lang#80385 (Clarify what `Cell::replace` returns)
 - rust-lang#82571 (Rustdoc Json: Add tests for Reexports, and improve jsondocck)
 - rust-lang#82860 (Add `-Z unpretty` flag for the THIR)
 - rust-lang#82950 (convert slice doc link to intra-doc links)
 - rust-lang#82965 (Add spirv extension handling in compiletest)
 - rust-lang#82966 (update MSYS2 link in README)
 - rust-lang#82979 (Fix "run" button position in error index)
 - rust-lang#83001 (Ignore Vim swap files)
 - rust-lang#83003 (rustdoc: tweak the search index format)
 - rust-lang#83013 (Adjust some `#[cfg]`s to take non-Unix non-Windows operating systems into account)
 - rust-lang#83018 (Reintroduce accidentally deleted assertions.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Mar 12, 2021

⌛ Testing commit 5f24798 with merge 77b996e...

@bors bors merged commit a98dc9b into rust-lang:master Mar 12, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 12, 2021
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 A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

7 participants