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

jyn514 · 2021-04-26T13:13:55Z

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!

rust-highfive · 2021-04-26T13:13:58Z

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

src/doc/rustdoc/src/lints.md

CraftSpider · 2021-04-27T23:04:19Z

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 · 2021-05-01T01:20:13Z

I don't see a test for the actual empty block case, could you add one?

It already exists: https://github.com/rust-lang/rust/blob/7f4d7327da6dbec5958d144a12e2e4709ebcbfc1/src/test/rustdoc-ui/invalid-syntax.stderr#L105-L111

Err ... actually I don't think there was ever an FCP in #79816. Also the lint name should be plural, not singular. I'll start FCP once I rename it.

jyn514 · 2021-05-01T01:31:11Z

@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.

rfcbot · 2021-05-01T01:31:12Z

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

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.

Nemo157 · 2021-05-01T09:57:40Z

src/test/rustdoc-ui/ignore-block-help.stderr

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

Why was this behavior changed in this PR?

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

The existing issue is #67857.

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

Ah, I see 👍

src/doc/rustdoc/src/lints.md

camelid · 2021-05-01T23:30:11Z

src/test/rustdoc-ui/ignore-block-help.stderr

Why was this behavior changed in this PR?

camelid · 2021-05-01T23:31:11Z

@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.

compiler/rustc_mir/src/borrow_check/region_infer/mod.rs

src/librustdoc/passes/check_code_block_syntax.rs

rfcbot · 2021-05-02T12:50:10Z

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

…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!

…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 · 2021-05-15T11:27:48Z

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`

jyn514 · 2021-05-15T13:21:46Z

@bors r-

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

jyn514 · 2021-05-18T05:07:23Z

@bors r=CraftSpider

bors · 2021-05-18T05:07:24Z

📌 Commit 587c504 has been approved by CraftSpider

…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!

…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 <type>) - 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 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

rust-highfive assigned CraftSpider Apr 26, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 26, 2021

jyn514 changed the title ~~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 Apr 26, 2021

jyn514 force-pushed the rustdoc-lint-block branch from ec10728 to 6d22485 Compare April 26, 2021 13:16

syvb reviewed Apr 26, 2021

View reviewed changes

src/doc/rustdoc/src/lints.md Outdated Show resolved Hide resolved

jyn514 force-pushed the rustdoc-lint-block branch from 6d22485 to 7f4d732 Compare April 26, 2021 13:45

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 added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels May 1, 2021

jyn514 changed the title ~~rustdoc: 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 and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 1, 2021

Nemo157 reviewed May 1, 2021

View reviewed changes

camelid reviewed May 1, 2021

View reviewed changes

GuillaumeGomez reviewed May 2, 2021

View reviewed changes

compiler/rustc_mir/src/borrow_check/region_infer/mod.rs Outdated Show resolved Hide resolved

GuillaumeGomez reviewed May 2, 2021

View reviewed changes

src/librustdoc/passes/check_code_block_syntax.rs Outdated Show resolved Hide resolved

GuillaumeGomez reviewed May 2, 2021

View reviewed changes

src/librustdoc/passes/check_code_block_syntax.rs Outdated Show resolved Hide resolved

rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels May 2, 2021

jyn514 force-pushed the rustdoc-lint-block branch from f23a7db to 0511ba4 Compare May 4, 2021 02:20

GuillaumeGomez mentioned this pull request May 15, 2021

Rollup of 13 pull requests #85325

Closed

GuillaumeGomez mentioned this pull request May 15, 2021

Rollup of 12 pull requests #85327

Closed

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

poliorcetics and others added 4 commits May 17, 2021 21:31

Add back missing help for ignore blocks

4120f75

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

Rename INVALID_RUST_CODEBLOCK{,S}

b885d81

Address review comments

18f6922

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

jyn514 force-pushed the rustdoc-lint-block branch from 0511ba4 to 6b92f2e Compare May 18, 2021 03:10

This comment has been minimized.

Sign in to view

Fix rebase conflicts

587c504

jyn514 force-pushed the rustdoc-lint-block branch from 6b92f2e to 587c504 Compare May 18, 2021 03:33

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 mentioned this pull request May 18, 2021

Rollup of 7 pull requests #85431

Closed

GuillaumeGomez mentioned this pull request May 18, 2021

Rollup of 7 pull requests #85437

Merged

bors merged commit 07d11cf into rust-lang:master May 18, 2021

rustbot added this to the 1.54.0 milestone May 18, 2021

jyn514 deleted the rustdoc-lint-block branch May 18, 2021 18:18

apiraino removed the to-announce Announce this issue on triage meeting label May 20, 2021

jyn514 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jun 22, 2021

Uh oh!

Conversation

jyn514 commented Apr 26, 2021

Uh oh!

rust-highfive commented Apr 26, 2021

Uh oh!

Uh oh!

CraftSpider commented Apr 27, 2021

Uh oh!

jyn514 commented May 1, 2021

Uh oh!

jyn514 commented May 1, 2021

Uh oh!

rfcbot commented May 1, 2021 • edited by GuillaumeGomez Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nemo157 May 1, 2021

Choose a reason for hiding this comment

Uh oh!

camelid May 1, 2021

Choose a reason for hiding this comment

Uh oh!

Nemo157 May 2, 2021

Choose a reason for hiding this comment

Uh oh!

jyn514 May 4, 2021

Choose a reason for hiding this comment

Uh oh!

camelid May 5, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

camelid May 1, 2021

Choose a reason for hiding this comment

Uh oh!

camelid commented May 1, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rfcbot commented May 2, 2021

Uh oh!

GuillaumeGomez commented May 15, 2021

Uh oh!

jyn514 commented May 15, 2021

Uh oh!

This comment has been minimized.

jyn514 commented May 18, 2021

Uh oh!

bors commented May 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

rfcbot commented May 1, 2021 •

edited by GuillaumeGomez

Loading