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

--message-format json doesn't apply to rustdoc coverage output #75135

Open
Nemo157 opened this issue Aug 4, 2020 · 8 comments
Open

--message-format json doesn't apply to rustdoc coverage output #75135

Nemo157 opened this issue Aug 4, 2020 · 8 comments
Labels
A-CLI Area: Command-line interface (CLI) to the compiler A-doc-coverage Area: Calculating how much of a crate has documentation C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@Nemo157
Copy link
Member

Nemo157 commented Aug 4, 2020

I tried this:

RUSTDOCFLAGS='--show-coverage -Z unstable-options' cargo doc --message-format json --no-deps --all-features

I expected to get a json message containing the coverage information, but instead I still got a table.

Then I tried this:

RUSTDOCFLAGS='--show-coverage -Z unstable-options --output-format json' cargo doc --no-deps --all-features

and it did output a json message, but that message is missing a reason key to identify it within the greater message stream when combined with --message-format json.

rustdoc 1.47.0-nightly (d8cbd9cac 2020-08-03)
@Nemo157 Nemo157 added the C-bug Category: This is a bug. label Aug 4, 2020
@est31
Copy link
Member

est31 commented Aug 4, 2020

Yeah it's part of a larger problem. For example, rustdoc also doesn't pass on json params to the sub compiler invocations it performs when doing doctests. And it has a bunch of eprintln! sprinkled across the codebase, all not abiding the json format.

Edit: wait, my comment was about --error-format, something separate... was confused a bit.

@GuillaumeGomez
Copy link
Member

--message-format isn't the option you're looking for. You want --output-format json.

@Nemo157
Copy link
Member Author

Nemo157 commented Aug 4, 2020

@est31 well, uh, it's a bit complicated now, --message-format json was translated into --error-format json previously; but for a while now cargo has been unconditionally running the compiler with something like --error-format json --json=diagnostic-rendered-ansi and then using --message-format json to determine whether to just re-emit those json messages after stripping the rendered-ansi message from them, or to emit the rendered-ansi message itself.

@jyn514
Copy link
Member

jyn514 commented Feb 20, 2021

RUSTDOCFLAGS='--show-coverage -Z unstable-options' cargo doc --message-format json --no-deps --all-features

This seems like a cargo bug, it should propagate the flag to rustdoc.

and it did output a json message, but that message is missing a reason key to identify it within the greater message stream when combined with --message-format json.

I'm not sure what reason is or what it has to do with rustdoc. Is this a cargo field? If so I think this should be moved to rust-lang/cargo.

@Nemo157
Copy link
Member Author

Nemo157 commented Feb 20, 2021

This seems like a cargo bug, it should propagate the flag to rustdoc.

It does, but as @GuillaumeGomez mentioned (and expanded on on discord iirc) the coverage output is treated as an "output artifact" not an "error message" so obeys the --output-format flag instead of the --error-format flag.

I'm not sure what reason is or what it has to do with rustdoc. Is this a cargo field? If so I think this should be moved to rust-lang/cargo.

reason is a field added to every json message by cargo to allow distinguishing them, it takes the --error-format=json message from the compiler and re-emits it with reason = "compiler-message" injected. This may not actually be necessary since the coverage output goes to stdout instead of stderr as all other messages do.

Overall this issue might be a "works as designed", users should just be using cargo rustdoc --message-format=json -- --show-coverage --output-format=json, but maybe the coverage data should be considered a "message" rather than "output" to avoid that double-specification of the format.

@jyn514
Copy link
Member

jyn514 commented Feb 21, 2021

It does, but as @GuillaumeGomez mentioned (and expanded on on discord iirc) the coverage output is treated as an "output artifact" not an "error message" so obeys the --output-format flag instead of the --error-format flag.

Ok, but that seems to me like cargo should also be propagating --output-format=json to rustdoc? Unless you're asking to combine --error-format and --output-format into the same flag, which seems less flexible.

maybe the coverage data should be considered a "message" rather than "output" to avoid that double-specification of the format.

That doesn't seem right - when you pass --show-coverage, rustdoc doesn't generate any HTML output, which means the coverage output is the only thing it emits. So I think it should be considered output rather than a diagnostic.

@Nemo157
Copy link
Member Author

Nemo157 commented Feb 21, 2021

Ok, but that seems to me like cargo should also be propagating --output-format=json to rustdoc?

That wouldn't be appropriate for other outputs of cargo rustdoc, if you run cargo rustdoc --message-format=json you would expect the docs built to still be in html format; --show-coverage feels different because it is output as a "message".


Does rustdoc or rustc have any other similar user facing output via stdout that can be configured to use json? The closest I can think of is rustc --print cfg and other --print flags, but those don't have the option to toggle to json; but they do ignore the --error-format flag like --show-coverage.

@jyn514
Copy link
Member

jyn514 commented Feb 21, 2021

Does rustdoc or rustc have any other similar user facing output via stdout that can be configured to use json? The closest I can think of is rustc --print cfg and other --print flags, but those don't have the option to toggle to json; but they do ignore the --error-format flag like --show-coverage.

The closest I found was rustc --print=target-spec-json, and there is no corresponding --print=target-spec. I guess rustdoc could add --show-coverage-json, but it seems confusing to have both that and --output-format.

Overall this issue might be a "works as designed", users should just be using cargo rustdoc --message-format=json -- --show-coverage --output-format=json, but maybe the coverage data should be considered a "message" rather than "output" to avoid that double-specification of the format.

It seems reasonable to have one but not the other though - show the coverage as JSON, but if there's an error, pretty-print it for the user. I did something like that with --message-format=json-render-diagnostics for cargo-metadata: oli-obk/cargo_metadata#147.

Actually now that I say that, I think the right behavior is for cargo to pass --output-format=json --error-format=json when it gets passed --message-format=json, and to pass --output-format=json when passed message-format=json-render-diagnostics. What do you think?

@ChrisDenton ChrisDenton added the needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. label Jul 16, 2023
@fmease fmease added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-doc-coverage Area: Calculating how much of a crate has documentation A-CLI Area: Command-line interface (CLI) to the compiler and removed needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. labels Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: Command-line interface (CLI) to the compiler A-doc-coverage Area: Calculating how much of a crate has documentation C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants