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: replace most (e)println! statements with structured warnings/errors #50541

Merged
merged 3 commits into from
May 16, 2018

Conversation

QuietMisdreavus
Copy link
Member

Turns out, the rustc diagnostic handler doesn't need a whole lot of setup that we weren't already doing. For errors that occur outside a "dealing with source code" context, we can just use the format/color config we were already parsing and make up a Handler that we can emit structured warnings/errors from. So i did that. This will make it way easier to test things with rustdoc-ui tests, since those require the JSON error output. (In fact, this PR is a yak shave for a different one where i was trying to do just that. >_>)

@rust-highfive
Copy link
Collaborator

r? @GuillaumeGomez

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 8, 2018
@QuietMisdreavus
Copy link
Member Author

Fixing the merge conflict, just double-checking that it still builds before i finish the rebase.

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
/usr/local/lib/python2.7/dist-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
/usr/local/lib/python2.7/dist-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
  Downloading https://files.pythonhosted.org/packages/c9/6c/72524c483cd871e1d55fb3c3fa029605d17182c83de6e27ca143aed635a8/awscli-1.15.16-py2.py3-none-any.whl (1.3MB)
    0% |▎                               | 10kB 10.2MB/s eta 0:00:01
    1% |▌                               | 20kB 1.2MB/s eta 0:00:02
    2% |▉                               | 30kB 1.4MB/s eta 0:00:01
    3% |█                               | 40kB 1.3MB/s eta 0:00:01
---

[00:06:12] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:06:13] tidy error: /checkout/src/librustdoc/lib.rs:717: line longer than 100 chars
[00:06:14] some tidy checks failed
[00:06:14] 
[00:06:14] 
[00:06:14] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:06:14] 
[00:06:14] 
[00:06:14] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:06:14] Build completed unsuccessfully in 0:02:37
[00:06:14] Build completed unsuccessfully in 0:02:37
[00:06:14] make: *** [tidy] Error 1
[00:06:14] Makefile:79: recipe for target 'tidy' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:1a6966ff
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@GuillaumeGomez
Copy link
Member

Oh nice, you improved my code quite a lot! Thanks a lot!

@bors: r+

@bors
Copy link
Contributor

bors commented May 9, 2018

📌 Commit b7328e1 has been approved by GuillaumeGomez

@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 May 9, 2018
@bors
Copy link
Contributor

bors commented May 12, 2018

⌛ Testing commit b7328e1b54a4f066e8202b655522528c4e830594 with merge 13a3971464f0503d2f2cc63aa6788147ee66717c...

@bors
Copy link
Contributor

bors commented May 12, 2018

💔 Test failed - status-travis

@bors 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 May 12, 2018
@rust-highfive
Copy link
Collaborator

The job dist-i686-linux of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[00:09:23]    Compiling proc_macro v0.0.0 (file:///checkout/src/libproc_macro)
[00:09:44] [RUSTC-TIMING] proc_macro test:false 20.693
[00:09:44]    Compiling syntax_ext v0.0.0 (file:///checkout/src/libsyntax_ext)
[00:10:17] [RUSTC-TIMING] syntax_ext test:false 33.390
No output has been received in the last 30m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

1 similar comment
@rust-highfive
Copy link
Collaborator

The job dist-i686-linux of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[00:09:23]    Compiling proc_macro v0.0.0 (file:///checkout/src/libproc_macro)
[00:09:44] [RUSTC-TIMING] proc_macro test:false 20.693
[00:09:44]    Compiling syntax_ext v0.0.0 (file:///checkout/src/libsyntax_ext)
[00:10:17] [RUSTC-TIMING] syntax_ext test:false 33.390
No output has been received in the last 30m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@kennytm
Copy link
Member

kennytm commented May 12, 2018

@bors retry

@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 May 12, 2018
@bors
Copy link
Contributor

bors commented May 13, 2018

🔒 Merge conflict

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 13, 2018
@bors
Copy link
Contributor

bors commented May 13, 2018

☔ The latest upstream changes (presumably #50235) made this pull request unmergeable. Please resolve the merge conflicts.

@QuietMisdreavus
Copy link
Member Author

@bors r=GuillaumeGomez

@bors
Copy link
Contributor

bors commented May 14, 2018

📌 Commit c3fd12f has been approved by GuillaumeGomez

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 14, 2018
@kennytm
Copy link
Member

kennytm commented May 15, 2018

@bors p=1

Needed to unblock #50669.

kennytm added a commit to kennytm/rust that referenced this pull request May 15, 2018
…uillaumeGomez

rustdoc: replace most (e)println! statements with structured warnings/errors

Turns out, the rustc diagnostic handler doesn't need a whole lot of setup that we weren't already doing. For errors that occur outside a "dealing with source code" context, we can just use the format/color config we were already parsing and make up a `Handler` that we can emit structured warnings/errors from. So i did that. This will make it way easier to test things with `rustdoc-ui` tests, since those require the JSON error output. (In fact, this PR is a yak shave for a different one where i was trying to do just that. `>_>`)
@bors
Copy link
Contributor

bors commented May 16, 2018

⌛ Testing commit c3fd12f with merge 3c31e17...

bors added a commit that referenced this pull request May 16, 2018
rustdoc: replace most (e)println! statements with structured warnings/errors

Turns out, the rustc diagnostic handler doesn't need a whole lot of setup that we weren't already doing. For errors that occur outside a "dealing with source code" context, we can just use the format/color config we were already parsing and make up a `Handler` that we can emit structured warnings/errors from. So i did that. This will make it way easier to test things with `rustdoc-ui` tests, since those require the JSON error output. (In fact, this PR is a yak shave for a different one where i was trying to do just that. `>_>`)
@bors
Copy link
Contributor

bors commented May 16, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: GuillaumeGomez
Pushing 3c31e17 to master...

@bors bors merged commit c3fd12f into rust-lang:master May 16, 2018
kennytm added a commit to kennytm/rust that referenced this pull request May 16, 2018
…=GuillaumeGomez

rustdoc: deprecate `#![doc(passes, plugins, no_default_passes)]`

Closes rust-lang#48164

Blocked on rust-lang#50541 - this includes those changes, which were necessary to create the UI test

cc rust-lang#44136

Turns out, there were special attributes to mess with rustdoc passes and plugins! Who knew! Since we deprecated the CLI flags for this functionality, it makes sense that we do the same for the attributes.

This PR also introduces a `#![doc(document_private_items)]` attribute, to match the `--document-private-items` flag introduced in rust-lang#44138 when the passes/plugins flags were deprecated.

I haven't done a search to see whether these attributes are being used at all, but if the flags were any indication, i don't expect to see any users of these.
@QuietMisdreavus QuietMisdreavus deleted the rustdoc-errors branch May 17, 2018 21:36
@jyn514 jyn514 mentioned this pull request Mar 1, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 6, 2021
…eGomez

Cleanup rustdoc warnings

## Clean up error reporting for deprecated passes

Using `error!` here goes all the way back to the original commit, rust-lang#8540. I don't see any reason to use logging; rustdoc should use diagnostics wherever possible. See rust-lang#81932 (comment) for further context.

- Use spans for deprecated attributes
- Use a proper diagnostic for unknown passes, instead of error logging
- Add tests for unknown passes
- Improve some wording in diagnostics

##  Report that `doc(plugins)` doesn't work using diagnostics instead of `eprintln!`

This also adds a test for the output.

This was added in rust-lang#52194. I don't see any particular reason not to use diagnostics here, I think it was just missed in rust-lang#50541.
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 6, 2021
…eGomez

Cleanup rustdoc warnings

## Clean up error reporting for deprecated passes

Using `error!` here goes all the way back to the original commit, rust-lang#8540. I don't see any reason to use logging; rustdoc should use diagnostics wherever possible. See rust-lang#81932 (comment) for further context.

- Use spans for deprecated attributes
- Use a proper diagnostic for unknown passes, instead of error logging
- Add tests for unknown passes
- Improve some wording in diagnostics

##  Report that `doc(plugins)` doesn't work using diagnostics instead of `eprintln!`

This also adds a test for the output.

This was added in rust-lang#52194. I don't see any particular reason not to use diagnostics here, I think it was just missed in rust-lang#50541.
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Mar 6, 2021
…eGomez

Cleanup rustdoc warnings

## Clean up error reporting for deprecated passes

Using `error!` here goes all the way back to the original commit, rust-lang#8540. I don't see any reason to use logging; rustdoc should use diagnostics wherever possible. See rust-lang#81932 (comment) for further context.

- Use spans for deprecated attributes
- Use a proper diagnostic for unknown passes, instead of error logging
- Add tests for unknown passes
- Improve some wording in diagnostics

##  Report that `doc(plugins)` doesn't work using diagnostics instead of `eprintln!`

This also adds a test for the output.

This was added in rust-lang#52194. I don't see any particular reason not to use diagnostics here, I think it was just missed in rust-lang#50541.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 6, 2021
…eGomez

Cleanup rustdoc warnings

## Clean up error reporting for deprecated passes

Using `error!` here goes all the way back to the original commit, rust-lang#8540. I don't see any reason to use logging; rustdoc should use diagnostics wherever possible. See rust-lang#81932 (comment) for further context.

- Use spans for deprecated attributes
- Use a proper diagnostic for unknown passes, instead of error logging
- Add tests for unknown passes
- Improve some wording in diagnostics

##  Report that `doc(plugins)` doesn't work using diagnostics instead of `eprintln!`

This also adds a test for the output.

This was added in rust-lang#52194. I don't see any particular reason not to use diagnostics here, I think it was just missed in rust-lang#50541.
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 7, 2021
…eGomez

Cleanup rustdoc warnings

## Clean up error reporting for deprecated passes

Using `error!` here goes all the way back to the original commit, rust-lang#8540. I don't see any reason to use logging; rustdoc should use diagnostics wherever possible. See rust-lang#81932 (comment) for further context.

- Use spans for deprecated attributes
- Use a proper diagnostic for unknown passes, instead of error logging
- Add tests for unknown passes
- Improve some wording in diagnostics

##  Report that `doc(plugins)` doesn't work using diagnostics instead of `eprintln!`

This also adds a test for the output.

This was added in rust-lang#52194. I don't see any particular reason not to use diagnostics here, I think it was just missed in rust-lang#50541.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants