-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Add possibility to generate rustdoc JSON output to stdout #128963
Conversation
Why not use the standard |
Because I didn't know about it. It's much better! |
3868ad5
to
caf30e5
Compare
--output-to-stdout
option to rustdoc
Applied your suggestion @Urgau. Thanks a lot! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is worth supporting, but if --output -
is made to work, then so should --output foo.json
src/librustdoc/config.rs
Outdated
(Some(_), Some(_)) => { | ||
dcx.fatal("cannot use both 'out-dir' and 'output' at once"); | ||
(Some(_), Some(output)) => { | ||
if output == "-" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the purpose of this logic? rustdoc --out-dir something --output -
should be an error, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because --out-dir
is used for all rustdoc-ui tests, preventing this option to be tested. I thought about whether I should add a new flag to tests to allow removing this argument or to allow to have --out-dir
and --output -
used at the same time and considering it doesn't generate a file, I thought it would be ok. The only thing that I'm not clear about is whether or not it should a warning.
src/librustdoc/config.rs
Outdated
dcx.fatal("cannot use both 'out-dir' and 'output' at once"); | ||
(Some(_), Some(output)) => { | ||
if output == "-" { | ||
output_to_stdout = output == "-"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: This is always true by the condition above:
output_to_stdout = output == "-"; | |
output_to_stdout = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oof, silly me. Thanks!
tests/rustdoc-ui/json-stdout.stdout
Outdated
@@ -0,0 +1 @@ | |||
{"root":"0:0:2381","crate_version":null,"includes_private":false,"index":{"0:0:2381":{"id":"0:0:2381","crate_id":0,"name":"json_stdout","span":{"filename":"$DIR/json-stdout.rs","begin":[6,0],"end":[9,15]},"visibility":"public","docs":null,"links":{},"attrs":["#![feature(no_core)]","#![no_core]"],"deprecation":null,"inner":{"module":{"is_crate":true,"items":["0:1:2380"],"is_stripped":false}}},"0:1:2380":{"id":"0:1:2380","crate_id":0,"name":"Foo","span":{"filename":"$DIR/json-stdout.rs","begin":[9,0],"end":[9,15]},"visibility":"public","docs":null,"links":{},"attrs":[],"deprecation":null,"inner":{"struct":{"kind":"unit","generics":{"params":[],"where_predicates":[]},"impls":[]}}}},"paths":{"0:0:2381":{"crate_id":0,"path":["json_stdout"],"kind":"module"},"0:1:2380":{"crate_id":0,"path":["json_stdout","Foo"],"kind":"struct"}},"external_crates":{},"format_version":33} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is gonna be super fragile. I think we should use a run-make
test instead, that just checks that rustdoc passes, and prints JSON to stdout (maybe with format_version
, but no other fields).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a great idea and will also solve the --out-dir
and --output -
options special case.
@@ -37,7 +37,7 @@ pub(crate) struct JsonRenderer<'tcx> { | |||
/// level field of the JSON blob. | |||
index: Rc<RefCell<FxHashMap<types::Id, types::Item>>>, | |||
/// The directory where the blob will be written to. | |||
out_path: PathBuf, | |||
out_path: Option<PathBuf>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be renamed to out_dir
, as it's not the path to the JSON file, but the path to the directory that contains the JSON file. (But also, maybe we should do this logic earlier, see my later comments)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly, as we discovered, output
and out-dir
are actually the same option and output
is deprecated. I think it'll require a bigger discussion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remark (peanut gallery): that is very surprising, rustc
's --output
and --out-dir
means (I understand this is rustdoc not rustc):
The output filename can be set with the
-o
flag. A suffix may be added to the filename with the-C extra-filename
flag.
Output files are written to the current directory unless the--out-dir
flag is used.
(rustc's cli docs is kinda inaccurate here because it's actually output path for -o
not just filename)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
Ok(()) | ||
let mut p = out_dir; | ||
p.push(output.index.get(&output.root).unwrap().name.clone().unwrap()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also respect the --output
flag here, even if it's not for a file. Currently --output
is ignored for JSON, but if --output -
works, then --output some_file.json
should also work to rename the JSON (instead of using the crate name).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, from what I can see, --out-dir
and --output
are kinda considered the same by rustdoc and --output
is deprecated. So currently we don't really have a way to say I want JSON output to be generated into "this file".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remark (peanut gallery): I would've thought --output=PATH
would cause the json output to be dumped to this specific file at PATH
, and that if --output
is specified, --out-dir
would be ignored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it's the whole problem here. The --output
option was created when the HTML format was the only format, making it useless and why we created --out-dir
. Currently, both options are actually the same, which is pretty bad.
But also, this is complicated by the fact that |
Unless I'm missing something, it's already working. |
Nevermind, it's not. I'll rewrite the whole PR to make this behaviour better. |
caf30e5
to
2b41fbe
Compare
The run-make-support library was changed cc @jieyouxu This PR modifies cc @jieyouxu |
(Some(_), Some(_)) => { | ||
dcx.fatal("cannot use both 'out-dir' and 'output' at once"); | ||
} | ||
(Some(out_dir), None) => out_dir, | ||
(None, Some(output)) => output, | ||
(Some(out_dir), None) | (None, Some(out_dir)) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we discovered they're the same option, it simplifies a lot the handling of this new option.
2b41fbe
to
c79c592
Compare
@@ -37,7 +37,7 @@ pub(crate) struct JsonRenderer<'tcx> { | |||
/// level field of the JSON blob. | |||
index: Rc<RefCell<FxHashMap<types::Id, types::Item>>>, | |||
/// The directory where the blob will be written to. | |||
out_path: PathBuf, | |||
out_path: Option<PathBuf>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remark (peanut gallery): that is very surprising, rustc
's --output
and --out-dir
means (I understand this is rustdoc not rustc):
The output filename can be set with the
-o
flag. A suffix may be added to the filename with the-C extra-filename
flag.
Output files are written to the current directory unless the--out-dir
flag is used.
(rustc's cli docs is kinda inaccurate here because it's actually output path for -o
not just filename)
|
||
Ok(()) | ||
let mut p = out_dir; | ||
p.push(output.index.get(&output.root).unwrap().name.clone().unwrap()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remark (peanut gallery): I would've thought --output=PATH
would cause the json output to be dumped to this specific file at PATH
, and that if --output
is specified, --out-dir
would be ignored.
c79c592
to
63ab7b5
Compare
@bors r+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The run-make test and library changes LGTM
.arg("-Zunstable-options") | ||
.output_format("json") | ||
.run() | ||
.assert_stdout_contains("{\""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remark: I suppose this is a reasonable heuristic.
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#125970 (CommandExt::before_exec: deprecate safety in edition 2024) - rust-lang#127905 (Add powerpc-unknown-linux-muslspe compile target) - rust-lang#128925 (derive(SmartPointer): register helper attributes) - rust-lang#128946 (Hash Ipv*Addr as an integer) - rust-lang#128963 (Add possibility to generate rustdoc JSON output to stdout) - rust-lang#129015 (Update books) - rust-lang#129067 (Use `append` instead of `extend(drain(..))`) - rust-lang#129100 (Fix dependencies cron job) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#125970 (CommandExt::before_exec: deprecate safety in edition 2024) - rust-lang#127905 (Add powerpc-unknown-linux-muslspe compile target) - rust-lang#128925 (derive(SmartPointer): register helper attributes) - rust-lang#128946 (Hash Ipv*Addr as an integer) - rust-lang#128963 (Add possibility to generate rustdoc JSON output to stdout) - rust-lang#129015 (Update books) - rust-lang#129067 (Use `append` instead of `extend(drain(..))`) - rust-lang#129100 (Fix dependencies cron job) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#128963 - GuillaumeGomez:output-to-stdout, r=aDotInTheVoid Add possibility to generate rustdoc JSON output to stdout Fixes rust-lang#127165. I think it's likely common to want to get rustdoc json output directly instead of reading it from a file so I added this option to allow it. It's unstable and only works with `--output-format=json`. r? `@aDotInTheVoid`
…r=GuillaumeGomez rustdoc-json: Clean up serialization and printing. Somewhat a followup to rust-lang#128963, but makes sense regardless. - Renames `out_path` to `out_dir` because it's not the path to the JSON, but the directory - Also adds a comment explaining `None` - Renames `write` to `serialize_and_write` because it does both. - Also renames the self-profile activity name to be clear this measures both IO cost and serialization CPU cost - Expands the timer to cover flushing - Renames `output` to `output_crate`, to emphasize it's the contents, not the `--output` flag. r? `@GuillaumeGomez`
Rollup merge of rust-lang#129191 - aDotInTheVoid:rdj-serial-cleanup, r=GuillaumeGomez rustdoc-json: Clean up serialization and printing. Somewhat a followup to rust-lang#128963, but makes sense regardless. - Renames `out_path` to `out_dir` because it's not the path to the JSON, but the directory - Also adds a comment explaining `None` - Renames `write` to `serialize_and_write` because it does both. - Also renames the self-profile activity name to be clear this measures both IO cost and serialization CPU cost - Expands the timer to cover flushing - Renames `output` to `output_crate`, to emphasize it's the contents, not the `--output` flag. r? `@GuillaumeGomez`
…jieyouxu Actually parse stdout json, instead of using hacky contains logic. Fixes up the test added in rust-lang#128963, to actually parse the stdout to JSON, instead of just checking that it contains `{"`. CC `@GuillaumeGomez` r? `@jieyouxu`
…jieyouxu Actually parse stdout json, instead of using hacky contains logic. Fixes up the test added in rust-lang#128963, to actually parse the stdout to JSON, instead of just checking that it contains `{"`. CC ``@GuillaumeGomez`` r? ``@jieyouxu``
…jieyouxu Actually parse stdout json, instead of using hacky contains logic. Fixes up the test added in rust-lang#128963, to actually parse the stdout to JSON, instead of just checking that it contains `{"`. CC ```@GuillaumeGomez``` r? ```@jieyouxu```
Rollup merge of rust-lang#129837 - aDotInTheVoid:test-better-json, r=jieyouxu Actually parse stdout json, instead of using hacky contains logic. Fixes up the test added in rust-lang#128963, to actually parse the stdout to JSON, instead of just checking that it contains `{"`. CC ``@GuillaumeGomez`` r? ``@jieyouxu``
@Urgau So how does this work exactly, how can I get the JSON output to stdout? 🙂 When I do
|
|
@Urgau Thanks for the fast response! But how can I run > rustdoc -- --output-format json -Z unstable-options -o -
error: too many file operands
For more information about this error, try `rustc --explain E0670`. Why "error: too many file operands"? 😅 Then I tried: > rustdoc src/main.rs --edition 2021 --output-format json -Z unstable-options
error[E0432]: unresolved import `google_tasks1`
--> src/main.rs:1:5
|
1 | use google_tasks1 as tasks1;
| ^^^^^^^^^^^^^^^^^^^^^^^ no external crate `google_tasks1`
error[E0432]: unresolved import `serde`
--> src/main.rs:2:5
|
2 | use serde::{Deserialize, Serialize};
| ^^^^^ use of undeclared crate or module `serde`
error[E0433]: failed to resolve: use of undeclared crate or module `yup_oauth2`
--> src/main.rs:9:5
|
9 | use yup_oauth2::{storage::TokenInfo, InstalledFlowAuthenticator, InstalledFlowReturnMethod};
| ^^^^^^^^^^ use of undeclared crate or module `yup_oauth2`
error[E0432]: unresolved import `serde_json`
--> src/main.rs:3:5
|
3 | use serde_json::json;
| ^^^^^^^^^^ use of undeclared crate or module `serde_json`
error[E0432]: unresolved import `yup_oauth2`
--> src/main.rs:9:5
|
9 | use yup_oauth2::{storage::TokenInfo, InstalledFlowAuthenticator, InstalledFlowReturnMethod};
| ^^^^^^^^^^ use of undeclared crate or module `yup_oauth2`
error[E0433]: failed to resolve: use of undeclared crate or module `tokio`
--> src/main.rs:51:3
|
51 | #[tokio::main]
| ^^^^^ use of undeclared crate or module `tokio`
error: aborting due to 6 previous errors
Some errors have detailed explanations: E0432, E0433.
For more information about an error, try `rustc --explain E0432`. but it complains about unresolved external imports. |
I'm sorry but I'm not the author of this change, I don't have time and this discussion is off-topic. |
Fixes #127165.
I think it's likely common to want to get rustdoc json output directly instead of reading it from a file so I added this option to allow it. It's unstable and only works with
--output-format=json
.r? @aDotInTheVoid