Declare a new lint to properly deny warnings in rustdoc by poliorcetics · Pull Request #79816 · rust-lang/rust

poliorcetics · 2020-12-07T23:49:32Z

This declares a new lint: INVALID_RUST_CODEBLOCK that is used by rustdoc to properly follow -D warnings instead of unconditionally emitting a warning.

Todo List

Declare lint.
Document lint (file: src/doc/rustdoc/src/lints.md).
Use lint in rustdoc (file: src/librustdoc/passes/check_code_block_syntax.rs, maybe others).
Write tests.
- Note: one for the behaviour of the new lint when the error is in a dependency, not the crate being tested (Declare a new lint to properly deny warnings in rustdoc #79816 (comment))
Refactor things.
Wait on Make rustdoc lints a tool lint instead of built-in #80527.

Fix #79792.

@rustbot label T-rustdoc A-lint

rust-highfive · 2020-12-07T23:49:35Z

r? @matthewjasper

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

jyn514 · 2020-12-12T01:36:23Z

r? @jyn514

jyn514

This looks about like what I imagined, thanks :)

compiler/rustc_lint_defs/src/builtin.rs

src/doc/rustdoc/src/lints.md

jyn514 · 2020-12-12T01:43:53Z

src/librustdoc/passes/check_code_block_syntax.rs

It is not always local (e.g. if this is a re-export), I think we found why it wasn't a lint before. Fortunately, we don't need to check the syntax of other crates, so you can just return without doing anything if it's non-local.

See #77230 for more discussion; it would also be good to add a test, since if the behavior changes it should be intentional.

Could you add a test for the behavior?

poliorcetics · 2020-12-14T11:44:39Z

@rustbot label +S-waiting-on-review -S-waiting-on-author

jyn514

Thanks, LGTM. Can you squash the commits before merging?

src/doc/rustdoc/src/lints.md

src/librustdoc/passes/check_code_block_syntax.rs

jyn514 · 2020-12-14T13:25:21Z

Also you need to update src/test/rustdoc-ui:

Some tests failed in compiletest suite=rustdoc-ui mode=ui host=x86_64-unknown-linux-gnu target=x86_64-unknown-linux-gnu
failures:

---- [ui] rustdoc-ui/invalid-syntax.rs stdout ----
diff of stderr:

7	LL | | /// ```
8	   | |_______^
9	   |
+	   = note: `#[warn(invalid_rust_codeblock)]` on by default
10	   = note: error from rustc: unknown start of token: \
11	   = note: error from rustc: unknown start of token: \
12	   = note: error from rustc: unknown start of token: \

poliorcetics · 2020-12-15T18:21:07Z

Added the changes you proposed and squashed the whole thing

Edit: I forgot to ran fmt first, made the change and squashed again

jyn514 · 2020-12-16T00:21:13Z

@bors r+

Looks great, thanks!

bors · 2020-12-16T00:21:14Z

📌 Commit 0e0ae47 has been approved by jyn514

…r=jyn514 Declare a new lint to properly deny warnings in rustdoc This declares a new lint: `INVALID_RUST_CODEBLOCK` that is used by `rustdoc` to properly follow `-D warnings` instead of unconditionally emitting a warning. ## Todo List - [x] Declare lint. - [x] Document lint (file: `src/doc/rustdoc/src/lints.md`). - [x] Use lint in `rustdoc` (file: `src/librustdoc/passes/check_code_block_syntax.rs`, maybe others). - [x] Write tests. - [x] Note: one for the behaviour of the new lint when the error is in a dependency, not the crate being tested (rust-lang#79816 (comment)) - [x] Refactor things. ## Questions - How/where are lints tested ? - Where are lints for `rustdoc` tested ? Fix rust-lang#79792. `@rustbot` label T-rustdoc A-lint

…r=jyn514 Declare a new lint to properly deny warnings in rustdoc This declares a new lint: `INVALID_RUST_CODEBLOCK` that is used by `rustdoc` to properly follow `-D warnings` instead of unconditionally emitting a warning. ## Todo List - [x] Declare lint. - [x] Document lint (file: `src/doc/rustdoc/src/lints.md`). - [x] Use lint in `rustdoc` (file: `src/librustdoc/passes/check_code_block_syntax.rs`, maybe others). - [x] Write tests. - [x] Note: one for the behaviour of the new lint when the error is in a dependency, not the crate being tested (rust-lang#79816 (comment)) - [x] Refactor things. ## Questions - How/where are lints tested ? - Where are lints for `rustdoc` tested ? Fix rust-lang#79792. ``@rustbot`` label T-rustdoc A-lint

These were the warnings previously: ``` warning: could not parse code block as Rust code --> crates/stdx/src/lib.rs:137:9 | 137 | /// ∀ x in slice[..idx]: pred(x) | _________^ 138 | | /// && ∀ x in slice[idx..]: !pred(x) | |____^ | = note: error from rustc: unknown start of token: \u{2200} warning: 1 warning emitted warning: unresolved link to `package` --> crates/base_db/src/input.rs:181:15 | 181 | /// it's [package].name, can be different for other project types or even | ^^^^^^^ no item named `package` in scope | = note: `#[warn(broken_intra_doc_links)]` on by default = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `package` --> crates/base_db/src/input.rs:181:15 | 181 | /// it's [package].name, can be different for other project types or even | ^^^^^^^ no item named `package` in scope | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: 2 warnings emitted warning: unresolved link to `package` --> crates/base_db/src/input.rs:181:15 | 181 | /// it's [package].name, can be different for other project types or even | ^^^^^^^ no item named `package` in scope | = note: `#[warn(broken_intra_doc_links)]` on by default = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: unresolved link to `package` --> crates/base_db/src/input.rs:181:15 | 181 | /// it's [package].name, can be different for other project types or even | ^^^^^^^ no item named `package` in scope | = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` warning: 2 warnings emitted ``` This does *not* fix the following warning, because it is actually rust code and rustdoc is being over eager: ``` warning: Rust code block is empty --> crates/parser/src/grammar.rs:16:5 | 16 | //! ``` | _____^ 17 | | //! // test function_with_zero_parameters 18 | | //! // fn foo() {} 19 | | //! ``` | |_______^ | help: mark blocks that do not contain Rust code as text | 16 | //! ```text | ^^^^^^^ ``` rust-lang/rust#79816 should make this configurable so the warning can be `allow`ed.

Make rustdoc lints a tool lint instead of built-in - Rename `broken_intra_doc_links` to `rustdoc::broken_intra_doc_links` (and similar for other rustdoc lints; I don't expect any others to be used frequently, though). - Ensure that the old lint names still work and give deprecation errors - Register lints even when running doctests - Move lint machinery into a separate file - Add `declare_rustdoc_lint!` macro Unblocks rust-lang#80300, rust-lang#79816, rust-lang#80965. Makes the strangeness in rust-lang#77364 more apparent to the end user (note that `missing_docs` is *not* moved to rustdoc in this PR). ## Current status This is waiting on FCP: rust-lang#80527 (comment)

Make rustdoc lints a tool lint instead of built-in - Rename `broken_intra_doc_links` to `rustdoc::broken_intra_doc_links` (and similar for other rustdoc lints; I don't expect any others to be used frequently, though). - Ensure that the old lint names still work and give deprecation errors - Register lints even when running doctests - Move lint machinery into a separate file - Add `declare_rustdoc_lint!` macro Unblocks rust-lang#80300, rust-lang#79816, rust-lang#80965. Makes the strangeness in rust-lang#77364 more apparent to the end user (note that `missing_docs` is *not* moved to rustdoc in this PR). Closes rust-lang#78786. ## Current status This is blocked on rust-lang#82620 (see rust-lang#80527 (comment))

jyn514 · 2021-03-04T14:38:06Z

#80527 was merged, so this is no longer blocked :)

@rustbot label: -S-blocked +S-waiting-on-author

poliorcetics · 2021-03-06T16:51:37Z

@rustbot label: +S-waiting-on-review -S-waiting-on-author

compiler/rustc_lint_defs/src/builtin.rs

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.

jyn514 · 2021-03-19T23:39:49Z

This is still not correct. Any new lints should be added to librustdoc/lint.rs. rustc_lint_defs shouldn't be modified at all, that's for builtin lints, not tool lints.

jyn514 · 2021-03-19T23:42:27Z

src/test/rustdoc-ui/invalid-syntax.stderr

 LL | | /// ```
   | |_______^
   |
+   = note: `#[warn(invalid_rust_codeblock)]` on by default


Could you add some tests for when ignore is present (and for all the rest of the branching you have for the diagnostics)?

JohnCSimon · 2021-04-04T16:17:43Z

@poliorcetics - Ping from triage: can you please resolve the merge conflict and set it to S-waiting-on-review ?

jyn514 · 2021-04-23T03:55:31Z

Hi @poliorcetics, any updates on this? It's really close to being done :/

jyn514 · 2021-04-26T12:09:16Z

I'm going to close this - feel free to re-open if you have time to work on this again.

rust-highfive assigned matthewjasper Dec 7, 2020

rustbot added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Dec 7, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 7, 2020

poliorcetics changed the title ~~Declare the new lint to properly deny warnings in rustdoc~~ Declare a new lint to properly deny warnings in rustdoc Dec 7, 2020

rust-highfive assigned jyn514 and unassigned matthewjasper Dec 12, 2020

jyn514 reviewed Dec 12, 2020

View reviewed changes

jyn514 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 12, 2020

rustbot 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 Dec 14, 2020

jyn514 approved these changes Dec 14, 2020

View reviewed changes

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

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

poliorcetics force-pushed the rustdoc-fail-on-deny branch from 7158e11 to d271e41 Compare December 15, 2020 18:20

poliorcetics force-pushed the rustdoc-fail-on-deny branch from d271e41 to 0e0ae47 Compare December 15, 2020 18:22

poliorcetics marked this pull request as ready for review December 15, 2020 18:27

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 Dec 16, 2020

Dylan-DPC-zz mentioned this pull request Dec 16, 2020

Rollup of 10 pull requests #80078

Closed

Dylan-DPC-zz mentioned this pull request Dec 16, 2020

Rollup of 10 pull requests #80084

Closed

poliorcetics force-pushed the rustdoc-fail-on-deny branch from e5a1d97 to 03f64fc Compare January 31, 2021 16:23

This comment has been minimized.

Sign in to view

poliorcetics force-pushed the rustdoc-fail-on-deny branch from 03f64fc to 2c33488 Compare January 31, 2021 17:57

jyn514 mentioned this pull request Feb 15, 2021

Accept additional user-defined classes in fenced code blocks #79454

Closed

7 tasks

jturner314 mentioned this pull request Feb 18, 2021

Add docs to CI rust-ndarray/ndarray#925

Merged

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Mar 4, 2021

poliorcetics force-pushed the rustdoc-fail-on-deny branch from 2c33488 to c067fae Compare March 6, 2021 16:50

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 6, 2021

jyn514 reviewed Mar 6, 2021

View reviewed changes

compiler/rustc_lint_defs/src/builtin.rs Outdated Show resolved Hide resolved

compiler/rustc_lint_defs/src/builtin.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

jyn514 reviewed Mar 19, 2021

View reviewed changes

jyn514 mentioned this pull request Apr 26, 2021

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

Merged

Uh oh!

Conversation

poliorcetics commented Dec 7, 2020 • edited by jyn514 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Todo List

Uh oh!

rust-highfive commented Dec 7, 2020

Uh oh!

jyn514 commented Dec 12, 2020

Uh oh!

jyn514 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jyn514 Dec 12, 2020

Choose a reason for hiding this comment

Uh oh!

jyn514 Mar 19, 2021

Choose a reason for hiding this comment

Uh oh!

poliorcetics commented Dec 14, 2020

Uh oh!

jyn514 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jyn514 commented Dec 14, 2020

Uh oh!

poliorcetics commented Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jyn514 commented Dec 16, 2020

Uh oh!

bors commented Dec 16, 2020

Uh oh!

This comment has been minimized.

This comment has been minimized.

jyn514 commented Mar 4, 2021

Uh oh!

poliorcetics commented Mar 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

jyn514 commented Mar 19, 2021

Uh oh!

jyn514 Mar 19, 2021

Choose a reason for hiding this comment

Uh oh!

JohnCSimon commented Apr 4, 2021

Uh oh!

jyn514 commented Apr 23, 2021

Uh oh!

jyn514 commented Apr 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

poliorcetics commented Dec 7, 2020 •

edited by jyn514

Loading

poliorcetics commented Dec 15, 2020 •

edited

Loading

poliorcetics commented Mar 6, 2021 •

edited

Loading