Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

rustdoc -D warnings doesn't fail for all warnings #79792

Closed
cmichi opened this issue Dec 7, 2020 · 2 comments · Fixed by #84587
Closed

rustdoc -D warnings doesn't fail for all warnings #79792

cmichi opened this issue Dec 7, 2020 · 2 comments · Fixed by #84587
Assignees
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@cmichi
Copy link
Contributor

cmichi commented Dec 7, 2020

I tried this code:

/// See [`foo:Foo`].
/// ```rust
fn main() {}

I expected to see this happen:

The env variable -Dwarnings is supposed to make rustdoc treat warnings as errors. For export RUSTDOCFLAGS="" the run below outputs only warnings, no errors. For RUSTDOCFLAGS="-Dwarnings" I would have expected both warnings to now be errors.

$ export RUSTDOCFLAGS="-Dwarnings"
$ cargo doc
 Documenting rustdoc-bug v0.1.0 (/tmp/bug/rustdoc-bug)
error: unresolved link to `foo:Foo`
 --> src/main.rs:1:10
  |
1 | /// See [`foo:Foo`].
  |          ^^^^^^^^^ no item named `foo:Foo` in scope
  |
  = note: `-D broken-intra-doc-links` implied by `-D warnings`
  = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

error: Rust code block is empty
 --> src/main.rs:2:5
  |
2 | /// ```rust
  |     ^^^^^^^

error: aborting due to previous error; 1 warning emitted```

Instead, this happened:

Only one of the warnings is switched to an error.

$ export RUSTDOCFLAGS="-Dwarnings"
$ cargo doc
 Documenting rustdoc-bug v0.1.0 (/tmp/bug/rustdoc-bug)
error: unresolved link to `foo:Foo`
 --> src/main.rs:1:10
  |
1 | /// See [`foo:Foo`].
  |          ^^^^^^^^^ no item named `foo:Foo` in scope
  |
  = note: `-D broken-intra-doc-links` implied by `-D warnings`
  = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

warning: Rust code block is empty
 --> src/main.rs:2:5
  |
2 | /// ```rust
  |     ^^^^^^^

error: aborting due to previous error; 1 warning emitted```

This is also problematic because for warnings the exit code is still 0, so a CI setup might not detect the failure:

$ cat src/main.rs 
/// ```rust
fn main() {}
$ RUSTDOCFLAGS="-Dwarnings" cargo doc
warning: Rust code block is empty
 --> src/main.rs:1:5
  |
1 | /// ```rust
  |     ^^^^^^^

warning: 1 warning emitted

    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
$ echo $?
0

Meta

rustc --version --verbose:

rustc 1.50.0-nightly (1c389ffef 2020-11-24)
@cmichi cmichi added the C-bug Category: This is a bug. label Dec 7, 2020
@cmichi cmichi changed the title rustdoc -D warnings doesn't work for all warnings rustdoc -D warnings doesn't fail for all warnings Dec 7, 2020
@jonas-schievink jonas-schievink added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Dec 7, 2020
@jyn514
Copy link
Member

jyn514 commented Dec 7, 2020

This happens because the rustdoc warning isn't emitted as a lint, so it isn't subject to lint levels. I don't think there was any reason for not making it a lint, so the fix is probably to use struct_span_lint instead of the current hard-coded warning:

let mut diag = self.cx.sess().struct_span_warn(sp, warning_message);

@jyn514 jyn514 added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Dec 7, 2020
@poliorcetics
Copy link
Contributor

@rustbot claim

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 16, 2020
…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
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 16, 2020
…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
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 22, 2020
…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
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 22, 2020
…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
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 22, 2020
…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
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Dec 23, 2020
…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
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 23, 2020
…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
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Dec 24, 2020
…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
@jyn514 jyn514 assigned jyn514 and unassigned poliorcetics Apr 26, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue 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 issue 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 issue 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!
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
5 participants