Declare a new lint to properly deny warnings in rustdoc #79816

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

-
-                diag
+                // NOTE(poliorcetics): is 'item' always local here ? Or will this panic one day ?
+                let hir_id = self.cx.tcx.hir().local_def_id_to_hir_id(item.def_id.expect_local());


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

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

@@ -7,6 +7,7 @@ LL | | /// \__________pkt->size___________/          \_result->size_/ \__pkt->si
 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.

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

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

rust-highfive assigned matthewjasper Dec 7, 2020

rustbot added A-lint 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 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

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

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

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 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 Show resolved Hide resolved

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 Mar 6, 2021

This comment has been minimized.

Sign in to view

poliorcetics force-pushed the rustdoc-fail-on-deny branch from c067fae to 4f4b2f5 Compare March 13, 2021 17:15

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 Mar 13, 2021

jyn514 reviewed Mar 19, 2021

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 Mar 19, 2021

JohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 4, 2021

camelid 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 23, 2021

jyn514 closed this Apr 26, 2021

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Declare a new lint to properly deny warnings in rustdoc #79816

Declare a new lint to properly deny warnings in rustdoc #79816

poliorcetics commented Dec 7, 2020 •

edited by jyn514

Loading

rust-highfive commented Dec 7, 2020

jyn514 commented Dec 12, 2020

jyn514 left a comment

jyn514 Dec 12, 2020

jyn514 Mar 19, 2021

poliorcetics commented Dec 14, 2020

jyn514 left a comment

jyn514 commented Dec 14, 2020

poliorcetics commented Dec 15, 2020 •

edited

Loading

jyn514 commented Dec 16, 2020

bors commented Dec 16, 2020

poliorcetics commented Mar 6, 2021 •

edited

Loading

This comment has been minimized.

jyn514 commented Mar 19, 2021

jyn514 Mar 19, 2021

JohnCSimon commented Apr 4, 2021

jyn514 commented Apr 23, 2021

jyn514 commented Apr 26, 2021

Declare a new lint to properly deny warnings in rustdoc #79816

Declare a new lint to properly deny warnings in rustdoc #79816

Conversation

poliorcetics commented Dec 7, 2020 • edited by jyn514 Loading

Todo List

rust-highfive commented Dec 7, 2020

jyn514 commented Dec 12, 2020

jyn514 left a comment

Choose a reason for hiding this comment

jyn514 Dec 12, 2020

Choose a reason for hiding this comment

jyn514 Mar 19, 2021

Choose a reason for hiding this comment

poliorcetics commented Dec 14, 2020

jyn514 left a comment

Choose a reason for hiding this comment

jyn514 commented Dec 14, 2020

poliorcetics commented Dec 15, 2020 • edited Loading

jyn514 commented Dec 16, 2020

bors commented Dec 16, 2020

poliorcetics commented Mar 6, 2021 • edited Loading

This comment has been minimized.

jyn514 commented Mar 19, 2021

jyn514 Mar 19, 2021

Choose a reason for hiding this comment

JohnCSimon commented Apr 4, 2021

jyn514 commented Apr 23, 2021

jyn514 commented Apr 26, 2021

poliorcetics commented Dec 7, 2020 •

edited by jyn514

Loading

poliorcetics commented Dec 15, 2020 •

edited

Loading

poliorcetics commented Mar 6, 2021 •

edited

Loading