Auto-generate lint documentation.

[ehuss]

This adds a tool which will generate the lint documentation in the rustc book automatically. This is motivated by keeping the documentation up-to-date, and consistently formatted. It also ensures the examples are correct and that they actually generate the expected lint. The lint groups table is also auto-generated. See rust-lang/compiler-team#349 for the original proposal.

An outline of how this works:

• The declare_lint! macro now accepts a doc comment where the documentation is written. This is inspired by how clippy works.
• A new tool src/tools/lint-docs scrapes the documentation and adds it to the rustc book during the build.
  • It runs each example and verifies its output and embeds the output in the book.
  • It does a few formatting checks.
  • It verifies that every lint is documented.
• Groups are collected from rustc -W help.

I updated the documentation for all the missing lints. I have also added an "Explanation" section to each lint providing a reason for the lint and suggestions on how to resolve it.

This can lead towards a future enhancement of possibly showing these docs via the --explain flag to make them easily accessible and discoverable.

[ehuss]

I have the following concerns about my approach which I would like some feedback on:

• The documentation takes up a lot of space in the source code. Several files are reaching towards the 3,000 line limit. Those files can be split up, but I am concerned that the docs obscure the lint definition. I am strongly tempted to take a different approach of moving the docs somewhere else, such as the rustc_error_codes package, similar to how error codes are done. This could potentially make it easier to edit, and support --explain. The downside is that the documentation is now "far away" from the lint definition, which requires a little more work to add, and people may be less likely to update them.

• This duplicates the rustdoc lints from the rustdoc book. I'm uncertain what to do here. I think it is useful to include all lints in one place. However, it doesn't seem right to remove them from the rustdoc book. I'm thinking about removing the rustdoc lint docs that I added. Or maybe just replacing them with a link to the rustdoc book?

• I considered adding a -Z flag that would print the lint data in a JSON format so that the tool doesn't need to do scraping. I'm not sure if it's worth it, but wanted to float it out there.

[wesleywiser]

• The documentation takes up a lot of space in the source code.

I think the benefit of having the documentation alongside the lint outweighs any concerns I have about the file length. We should just break those files up as they approach the limit.

I am concerned that the docs obscure the lint definition

I personally think the docs are pretty useful to have right next to the definition so I don't mind this.

moving the docs somewhere else, such as the rustc_error_codes package, similar to how error codes are done. This could potentially make it easier to edit, and support --explain

I'm not opposed to this but I do agree with your concerns.

This duplicates the rustdoc lints from the rustdoc book.

I think linking to the rustdoc book would be fine.

I considered adding a -Z flag that would print the lint data in a JSON format so that the tool doesn't need to do scraping.

The scraping is a bit hacky so I think this is a good idea but I don't think it needs to block this PR. I would be ok with landing this as-is and filing an issue for that enhancement if you don't want to do that in this PR.

[ehuss]

Thanks for the feedback! Sounds good to me. I changed the rustdoc lints to just be links to the rustdoc book. As for the extraction method, I think I'd like to stick with this for now and maybe transition to a -Z flag later. It's a relatively small amount of code, and it shouldn't be too difficult to change if needed. One drawback of a -Z flag is that x.py doc --stage=0 won't work.

[wesleywiser]

@bors r+

[bors]

📌 Commit 886b9970eb4e06698b91c848111a628ed3c44dc5 has been approved by wesleywiser

[bors]

⌛ Testing commit 886b9970eb4e06698b91c848111a628ed3c44dc5 with merge 7e5161b068ee2258497ed540c0de0e51e7925b94...

[bors]

💔 Test failed - checks-actions

[ehuss]

@wesleywiser I forgot to make the asm lint ignored. I made a change to make it easier to ignore special lints like that, and fixed another example that didn't fire on 32-bit platforms.

[wesleywiser]

Looks great!

@bors r+

[bors]

📌 Commit 49a61f5 has been approved by wesleywiser

[bors]

⌛ Testing commit 49a61f5 with merge b5f55b7...

[bors]

☀️ Test successful - checks-actions, checks-azure
Approved by: wesleywiser
Pushing b5f55b7 to master...

[petrochenkov]

@ehuss
It's very hard to add a new lint and pass this check without reading source code of the tool.
Doing this by trial, for example, requires building the full compiler multiple times (because rustc_lint_defs is everyone's dependency).

[petrochenkov]

Also the check runs on @bors try builds (e.g. https://github.com/rust-lang-ci/rust/runs/1403382921), which it shouldn't do.
(You can bors try with failing tests or docs, for example, which is very useful.)

[ehuss]

@petrochenkov Sorry this is causing problems. I tried to make the documentation on declare_lint very clear on what needs to be done. It's intended that one should just copy/paste the example (or another lint declaration). Is there something missing from that, or some way to make that clearer?

Removing it from try builds sounds very reasonable, and I'd be happy to fix that. I'm going to be gone for a while starting tomorrow, so I won't be able to get to it for a while, but I'll try when I get back in a week or two.

I'm not sure I understand the issue about rustc_lint_defs. AFAIK, most lints are defined in rustc_lint. That is fairly close to the root of the dependency tree, so the turnaround time to edit and rebuild should be about 1 or 2 minutes (I see it builds 5 crates), or about 30 seconds with -i --keep-stage-std=1. Also, it should only build the compiler once with ./x.py doc --stage=1 src/doc/rustc. Can you clarify what you mean there?

[petrochenkov]

Ok, some user experience report! (From #79078).

• I didn't actually see the documentation on declare_lint.
• I wasn't aware of the check, so I just added a lint without doc comment (crater doesn't need documentation).
• Then local testing failed at unspecified test suite (it's not clear that x.py doc src/doc/rustc is the failing command from rustbuild output) with a message about missing comment. I tried to reproduce the failure with x.py test src/tools/lint-docs (this is how tools like tidy run), but it didn't work.
• So I added a stub doc comment (/// Docs.) and pushed to CI where it eventually failed with a "missing ### Example" error.
• Then I went straight to the tools source code and saw that the comment's content is a subject to several checks, so then I copypasted a doc comment from the neighboring lint definition... and looks like failed again with "did not find lint legacy_derive_helpers in output of example", so I'll continue fixing.
• In the meantime I looked at the source code of impl Step for RustcBook which I found by searching for one of the tool's options (--rustc-target) in the codebase (you can't find it by searching for lint-docs) and found that the target that I need to run is x.py doc src/doc/rustc and not x.py test src/tools/lint-docs.
• Then I run x.py doc src/doc/rustc --stage 0 in hope to check my code faster, but it produced some irrelevant error (did not find lint temporary_cstring_as_ptr in output of example), so I started questioning whether x.py doc src/doc/rustc is the right command. Running x.py doc src/doc/rustc without an explicit stage also runs stage 0 and produces the same irrelevant error.
• Finally you've confirmed that x.py doc src/doc/rustc is indeed the right command in the previous comment, so I run it with explicit --stage 1 and finally reproduced the failure locally.
• x.py doc src/doc/rustc --stage 1 still requires a lot of time to run because I need to add a lint to https://github.com/rust-lang/rust/blob/603ab5bd6e0ffefafa7411cd8bd23a6ca82bcff0/compiler/rustc_lint_defs/src/builtin.rs

[petrochenkov]

I guess the conclusions are:

• @bors try is the most important thing to fix here.
• On failure rustbuild needs to print the command that allows to reproduce that failure.
• On failure lint-docs needs to point to that comment on declare_lint describing the rules and the way to test them.

[ehuss]

I see, that does sound frustrating! I'll take a look at a few different things:

• Somehow make it not required for "try" builds (I assume people normally aren't too picky about docs being correct on try builds).
• Clarify the error message on how to re-run the command.
• Add a link to the docs in the error message, and generally try to provide higher-level help.
• Add some extra help if you try to run with --stage=0 (or maybe just print some warnings but otherwise allow it?)
• Add some docs about using --keep-stage if you are just editing the comments, which should bring the round-trip time down to about 10 or 20s.

[Aaron1011]

I'm not sure I understand the issue about rustc_lint_defs. AFAIK, most lints are defined in rustc_lint.

I moved lint definitions to the new rustc_lint_defs crate as part of implementing future-incompat-report support.