Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in 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-lint 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-lint 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!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint 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