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

New rustdoc lint to respect -Dwarnings correctly #80456

Closed
wants to merge 1 commit into from

Conversation

poliorcetics
Copy link
Contributor

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.

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.
  • Refactor things.
  • Squash into one commit.

Previously was #79816 but I broke the submodules on my side and it was less trouble fixing it with a new PR that cherry-picked the changes.

Fix #79792.

@rustbot label T-rustdoc A-lint
r? @jyn514

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.
@rustbot 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 28, 2020
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 28, 2020
@petrochenkov
Copy link
Contributor

petrochenkov commented Dec 28, 2020

Can lints reported by rustdoc be defined entirely in rustdoc without touching rustc_lint and other compiler crates?
Clippy does this somehow.

I've been seeing numerous lints of uncertain quality being added to rustdoc in the recent year without any process from the compiler or lang team side, while in theory new lints require lang team RFCs or FCPs.
If they are added by rustdoc team, can they at least be offloaded entirely into rustdoc?

@poliorcetics
Copy link
Contributor Author

It would certainly be both easier to maintain and less buggy to have everything concentrated in rustdoc but I don't know if it is feasible, I'm especially not knowledgeable about how lints work in the compiler to tell.

I'll try to make something work and report my findings.

@jyn514
Copy link
Member

jyn514 commented Dec 28, 2020

@poliorcetics please don't make that change here, I would prefer to do it consistently and not piecemeal.

Can lints reported by rustdoc be defined entirely in rustdoc without touching rustc_lint and other compiler crates?

I would like to do that but haven't gotten around to it. The main technical issue is that the old lint names will have to be deprecated and renamed; they can't be deleted, because it's not possible to register built-in lints from out-of-tree (in other words, the new lints will have to be named rustdoc::lint, not lint directly).

There's also some debate about which lints should belong to rustdoc and which should be part of rustc: #77364

in theory new lints require lang team RFCs or FCPs.

I didn't know that. Is that written down somewhere?

@poliorcetics
Copy link
Contributor Author

Closing this in favour of #79816, see there for more infos.

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. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustdoc -D warnings doesn't fail for all warnings
5 participants