rustdoc: Make "rust code block is empty" and "could not parse code block" warnings a lint (INVALID_RUST_CODEBLOCKS) by jyn514 · Pull Request #84587 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation31 Commits5 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Fixes #79792. This already went through FCP in #79816, so it only needs final review.

This is mostly a rebase of #79816 - thank you @poliorcetics for doing most of the work!

jyn514 added T-rustdoc

Relevant to the rustdoc team, which will review and decide on the PR/issue.

A-lints

Area: Lints (warnings about flaws in source code) such as unused_mut.

labels

Apr 26, 2021

r? @CraftSpider

(rust-highfive has picked a reviewer for you, use r? to override)

jyn514 changed the titleMake "rust code block is empty" and "could not parse code block" warnings a lint rustdoc: Make "rust code block is empty" and "could not parse code block" warnings a lint

Apr 26, 2021

I don't see a test for the actual empty block case, could you add one? Otherwise looks generally good, given that I'm not super familiar with the lint system.

jyn514 added T-rustdoc and removed T-rustdoc

Relevant to the rustdoc team, which will review and decide on the PR/issue.

labels

May 1, 2021

@rfcbot fcp merge

This adds a new lint INVALID_RUST_CODEBLOCKS. These warnings were already being emitting, but as a hard warning and not a lint, so they couldn't be allowed or denied.

I'm open to renaming the lint, but I do think changing these from a warning to a lint is the right decision.

Team member @jyn514 has proposed to merge this. The next step is review by the rest of the tagged team members:

• @CraftSpider
• @GuillaumeGomez
• @Manishearth
• @Nemo157
• @camelid
• @jsha
• @jyn514
• @ollie27

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

jyn514 changed the titlerustdoc: Make "rust code block is empty" and "could not parse code block" warnings a lint rustdoc: Make "rust code block is empty" and "could not parse code block" warnings a lint (INVALID_RUST_CODEBLOCKS)

May 1, 2021

jyn514 added S-waiting-on-team

Status: Awaiting decision from the relevant subteam (see the T- label).

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

May 1, 2021

Choose a reason for hiding this comment

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

Is there an existing issue about this not pointing to the span that contains the error? Would be nice to fix in the future.

Choose a reason for hiding this comment

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

Why was this behavior changed in this PR?

Choose a reason for hiding this comment

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

The note just moved below the help, it wasn't attached to the correct span before either.

Choose a reason for hiding this comment

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

The existing issue is #67857.

Choose a reason for hiding this comment

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

it wasn't attached to the correct span before either.

Ah, I see 👍

Choose a reason for hiding this comment

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

Why was this behavior changed in this PR?

@rfcbot reviewed

I agree with making this a lint, but why was #84587 (comment) changed in this PR? I would prefer it to be a separate change.

🔔 This is now entering its final comment period, as per the review above. 🔔

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

May 15, 2021

…pider

rustdoc: Make "rust code block is empty" and "could not parse code block" warnings a lint (INVALID_RUST_CODEBLOCKS)

Fixes rust-lang#79792. This already went through FCP in rust-lang#79816, so it only needs final review.

This is mostly a rebase of rust-lang#79816 - thank you @poliorcetics for doing most of the work!

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

May 15, 2021

…pider

rustdoc: Make "rust code block is empty" and "could not parse code block" warnings a lint (INVALID_RUST_CODEBLOCKS)

Fixes rust-lang#79792. This already went through FCP in rust-lang#79816, so it only needs final review.

This is mostly a rebase of rust-lang#79816 - thank you @poliorcetics for doing most of the work!

I think it'll need to be rebased:

    Checking tracing-subscriber v0.2.16
    Checking tracing-tree v0.1.9
    Checking rustdoc-json-types v0.1.0 (/checkout/src/rustdoc-json-types)
    Checking rustdoc v0.0.0 (/checkout/src/librustdoc)
error[E0599]: no method named `as_local` found for enum `types::FakeDefId` in the current scope
  --> src/librustdoc/passes/check_code_block_syntax.rs:56:42
   |
56 |         let local_id = match item.def_id.as_local() {
   |                                          ^^^^^^^^ help: there is an associated function with a similar name: `is_local`
  ::: src/librustdoc/clean/types.rs:54:1
   |
   |
54 | crate enum FakeDefId {
   | -------------------- method `as_local` not found for this
error: aborting due to previous error

For more information about this error, try `rustc --explain E0599`.
error: could not compile `rustdoc`

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 15, 2021

This adds a new lint to rustc that is used in rustdoc when a code block is empty or cannot be parsed as valid Rust code.

Previously this was unconditionally a warning. As such some documentation comments were (unknowingly) abusing this to pass despite the -Dwarnings used when compiling rustc, this should not be the case anymore.

This also gives a better error message when a span is missing.

• Simplify boolean expression
• Give an example of invalid syntax
• Remove explanation of why code block is text

This comment has been minimized.

📌 Commit 587c504 has been approved by CraftSpider

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 18, 2021

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

May 18, 2021

…pider

rustdoc: Make "rust code block is empty" and "could not parse code block" warnings a lint (INVALID_RUST_CODEBLOCKS)

Fixes rust-lang#79792. This already went through FCP in rust-lang#79816, so it only needs final review.

This is mostly a rebase of rust-lang#79816 - thank you @poliorcetics for doing most of the work!

bors added a commit to rust-lang-ci/rust that referenced this pull request

May 18, 2021

…laumeGomez

Rollup of 7 pull requests

Successful merges:

• rust-lang#84587 (rustdoc: Make "rust code block is empty" and "could not parse code block" warnings a lint (INVALID_RUST_CODEBLOCKS))
• rust-lang#85280 (Toggle-wrap items differently than top-doc.)
• rust-lang#85338 (Implement more Iterator methods on core::iter::Repeat)
• rust-lang#85339 (Report an error if a lang item has the wrong number of generic arguments)
• rust-lang#85369 (Suggest borrowing if a trait implementation is found for &/&mut )
• rust-lang#85393 (Suppress spurious errors inside async fn)
• rust-lang#85415 (Clean up remnants of BorrowOfPackedField)

Failed merges:

r? @ghost @rustbot modify labels: rollup

jyn514 deleted the rustdoc-lint-block branch

May 18, 2021 18:18

jyn514 added the T-rustdoc

Relevant to the rustdoc team, which will review and decide on the PR/issue.

label

Jun 22, 2021

Labels

A-lints

Area: Lints (warnings about flaws in source code) such as unused_mut.

disposition-merge

This issue / PR is in PFCP or FCP with a disposition to merge it.

finished-final-comment-period

The final comment period is finished for this PR / Issue.

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.