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.

Sign up for GitHub

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

Jump to bottom

rustdoc: Use diagnostics for error when including sources #82993

Merged
merged 1 commit into from
Mar 27, 2021
Merged

rustdoc: Use diagnostics for error when including sources #82993

merged 1 commit into from
Mar 27, 2021

Conversation

camelid
Copy link
Member

@camelid camelid commented Mar 10, 2021 •
edited
Loading

This error probably almost never happens, but we should still use the
diagnostic infrastructure. My guess is that the error was added back
before rustdoc used the rustc diagnostic infrastructure (it was all
println! and eprintln! back then!) and since it likely rarely occurs
and this code doesn't change that much, no one thought to transition it
to using diagnostics.

Note that the old error was actually a warning (it didn't stop the rest
of doc building). It seems very unlikely that this would fail without
the rest of the doc build failing, so it makes more sense for it to be a
hard error.

The error looks like this:

error: failed to render source code for `src/test/rustdoc/smart-punct.rs`: "bar": foo
  --> src/test/rustdoc/smart-punct.rs:3:1
   |
3  | / #![crate_name = "foo"]
4  | |
5  | | //! This is the "start" of the 'document'! How'd you know that "it's" ...
6  | | //!
...  |
22 | | //! I say "don't smart-punct me -- please!"
23 | | //! ```
   | |_______^

I wasn't sure how to trigger the error, so to create that message I
temporarily made rustdoc always emit it. That's also why it says "bar"
and "foo" instead of a real error message.

Note that the span of the diagnostic starts at line 3 because line 1 of
that file is a (non-doc) comment and line 2 is a blank line.

@camelid camelid added A-diagnostics Area: Messages for errors, warnings, and lints C-cleanup Category: PRs that clean code up or issues documenting cleanup. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 10, 2021
@rust-highfive
Copy link
Collaborator

r? @jyn514

(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 Mar 10, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@camelid
Copy link
Member Author

camelid commented Mar 10, 2021

This will likely conflict with #82994 so probably one of these should be merged before the other.

@camelid camelid mentioned this pull request Mar 10, 2021
Merged
jyn514
jyn514 reviewed Mar 22, 2021
View reviewed changes
src/librustdoc/html/sources.rs Outdated Show resolved Hide resolved
src/librustdoc/html/sources.rs Outdated Show resolved Hide resolved
@camelid camelid added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 23, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@camelid
Copy link
Member Author

camelid commented Mar 24, 2021

Time for a rebase!

@jyn514 jyn514 removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 24, 2021
@camelid camelid 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 24, 2021
@camelid
Copy link
Member Author

camelid commented Mar 24, 2021

Okay, should be ready for review now!

@camelid
rustdoc: Use diagnostics for error when including sources
3d8ce0a 
This error probably almost never happens, but we should still use the
diagnostic infrastructure. My guess is that the error was added back
before rustdoc used the rustc diagnostic infrastructure (it was all
`println!` and `eprintln!` back then!) and since it likely rarely occurs
and this code doesn't change that much, no one thought to transition it
to using diagnostics.

Note that the old error was actually a warning (it didn't stop the rest
of doc building). It seems very unlikely that this would fail without
the rest of the doc build failing, so it makes more sense for it to be a
hard error.

The error looks like this:

    error: failed to render source code for `src/test/rustdoc/smart-punct.rs`: "bar": foo
      --> src/test/rustdoc/smart-punct.rs:3:1
       |
    3  | / #![crate_name = "foo"]
    4  | |
    5  | | //! This is the "start" of the 'document'! How'd you know that "it's" ...
    6  | | //!
    ...  |
    22 | | //! I say "don't smart-punct me -- please!"
    23 | | //! ```
       | |_______^

I wasn't sure how to trigger the error, so to create that message I
temporarily made rustdoc always emit it. That's also why it says "bar"
and "foo" instead of a real error message.

Note that the span of the diagnostic starts at line 3 because line 1 of
that file is a (non-doc) comment and line 2 is a blank line.
@camelid
Copy link
Member Author

camelid commented Mar 24, 2021

Squashed.

@camelid camelid changed the title rustdoc: Use diagnostics for warning about error with including sources rustdoc: Use diagnostics for error when including sources Mar 24, 2021
@jyn514
Copy link
Member

jyn514 commented Mar 24, 2021

r=me with CI passing

@camelid
Copy link
Member Author

camelid commented Mar 26, 2021

@bors r=jyn514

@bors
Copy link
Contributor

bors commented Mar 26, 2021

📌 Commit 3d8ce0a has been approved by jyn514

@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 26, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 26, 2021
@Dylan-DPC
Rollup merge of rust-lang#82993 - camelid:source-use-diag, r=jyn514
bfd56fa 
rustdoc: Use diagnostics for error when including sources

This error probably almost never happens, but we should still use the
diagnostic infrastructure. My guess is that the error was added back
before rustdoc used the rustc diagnostic infrastructure (it was all
`println!` and `eprintln!` back then!) and since it likely rarely occurs
and this code doesn't change that much, no one thought to transition it
to using diagnostics.

Note that the old error was actually a warning (it didn't stop the rest
of doc building). It seems very unlikely that this would fail without
the rest of the doc build failing, so it makes more sense for it to be a
hard error.

The error looks like this:

    error: failed to render source code for `src/test/rustdoc/smart-punct.rs`: "bar": foo
      --> src/test/rustdoc/smart-punct.rs:3:1
       |
    3  | / #![crate_name = "foo"]
    4  | |
    5  | | //! This is the "start" of the 'document'! How'd you know that "it's" ...
    6  | | //!
    ...  |
    22 | | //! I say "don't smart-punct me -- please!"
    23 | | //! ```
       | |_______^

I wasn't sure how to trigger the error, so to create that message I
temporarily made rustdoc always emit it. That's also why it says "bar"
and "foo" instead of a real error message.

Note that the span of the diagnostic starts at line 3 because line 1 of
that file is a (non-doc) comment and line 2 is a blank line.
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Mar 26, 2021
Closed
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 27, 2021
@Dylan-DPC
Rollup merge of rust-lang#82993 - camelid:source-use-diag, r=jyn514
c1dc467 
rustdoc: Use diagnostics for error when including sources

This error probably almost never happens, but we should still use the
diagnostic infrastructure. My guess is that the error was added back
before rustdoc used the rustc diagnostic infrastructure (it was all
`println!` and `eprintln!` back then!) and since it likely rarely occurs
and this code doesn't change that much, no one thought to transition it
to using diagnostics.

Note that the old error was actually a warning (it didn't stop the rest
of doc building). It seems very unlikely that this would fail without
the rest of the doc build failing, so it makes more sense for it to be a
hard error.

The error looks like this:

    error: failed to render source code for `src/test/rustdoc/smart-punct.rs`: "bar": foo
      --> src/test/rustdoc/smart-punct.rs:3:1
       |
    3  | / #![crate_name = "foo"]
    4  | |
    5  | | //! This is the "start" of the 'document'! How'd you know that "it's" ...
    6  | | //!
    ...  |
    22 | | //! I say "don't smart-punct me -- please!"
    23 | | //! ```
       | |_______^

I wasn't sure how to trigger the error, so to create that message I
temporarily made rustdoc always emit it. That's also why it says "bar"
and "foo" instead of a real error message.

Note that the span of the diagnostic starts at line 3 because line 1 of
that file is a (non-doc) comment and line 2 is a blank line.
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Mar 27, 2021
Closed
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 27, 2021
@Dylan-DPC
Rollup merge of rust-lang#82993 - camelid:source-use-diag, r=jyn514
0a8441f 
rustdoc: Use diagnostics for error when including sources

This error probably almost never happens, but we should still use the
diagnostic infrastructure. My guess is that the error was added back
before rustdoc used the rustc diagnostic infrastructure (it was all
`println!` and `eprintln!` back then!) and since it likely rarely occurs
and this code doesn't change that much, no one thought to transition it
to using diagnostics.

Note that the old error was actually a warning (it didn't stop the rest
of doc building). It seems very unlikely that this would fail without
the rest of the doc build failing, so it makes more sense for it to be a
hard error.

The error looks like this:

    error: failed to render source code for `src/test/rustdoc/smart-punct.rs`: "bar": foo
      --> src/test/rustdoc/smart-punct.rs:3:1
       |
    3  | / #![crate_name = "foo"]
    4  | |
    5  | | //! This is the "start" of the 'document'! How'd you know that "it's" ...
    6  | | //!
    ...  |
    22 | | //! I say "don't smart-punct me -- please!"
    23 | | //! ```
       | |_______^

I wasn't sure how to trigger the error, so to create that message I
temporarily made rustdoc always emit it. That's also why it says "bar"
and "foo" instead of a real error message.

Note that the span of the diagnostic starts at line 3 because line 1 of
that file is a (non-doc) comment and line 2 is a blank line.
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Mar 27, 2021
Closed
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Mar 27, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 27, 2021
@bors
Auto merge of rust-lang#83580 - Dylan-DPC:rollup-1zod4p7, r=Dylan-DPC
9b0edb7 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#81351 (combine: stop eagerly evaluating consts)
 - rust-lang#82525 (make unaligned_references future-incompat lint warn-by-default)
 - rust-lang#82626 (update array missing `IntoIterator` msg)
 - rust-lang#82917 (Add function core::iter::zip)
 - rust-lang#82993 (rustdoc: Use diagnostics for error when including sources)
 - rust-lang#83522 (Improve fs error open_from unix)
 - rust-lang#83548 (Always preserve `None`-delimited groups in a captured `TokenStream`)
 - rust-lang#83555 (Add #[inline] to io::Error methods)

Failed merges:

 - rust-lang#83130 (escape_ascii take 2)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f665e5a into rust-lang:master Mar 27, 2021
@rustbot rustbot added this to the 1.53.0 milestone Mar 27, 2021
@camelid camelid deleted the source-use-diag branch March 28, 2021 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyn514 jyn514 jyn514 left review comments

Assignees

@jyn514 jyn514

Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-cleanup Category: PRs that clean code up or issues documenting cleanup. C-enhancement Category: An issue proposing an enhancement or a PR with one. 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
Milestone
1.53.0
Development

Successfully merging this pull request may close these issues.
6 participants
@camelid @rust-highfive @rust-log-analyzer @jyn514 @bors @rustbot