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

Validate lint docs separately. #79522

Merged
merged 4 commits into from
Dec 1, 2020
Merged

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Nov 28, 2020

This addresses some concerns raised in #76549 (comment) about errors with the lint docs being confusing and cumbersome. Errors from validating the lint documentation were being generated during x.py doc (and x.py dist), since extraction and validation are being done in a single step. This changes it so that extraction and validation are separated, so that x.py doc will not error if there is a validation problem, and tests are moved to x.py test src/tools/lint-docs.

This includes the following changes:

  • Separate validation to x.py test.
  • Added some more documentation on how to more easily modify and test the docs.
  • Added more help to the error messages to hopefully provide more information on how to fix things.

The first commit just moves the code around, so you may consider looking at the other commits for a smaller diff.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 28, 2020
@ehuss
Copy link
Contributor Author

ehuss commented Nov 28, 2020

cc @petrochenkov FYI

@jyn514 jyn514 added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-testsuite Area: The testsuite used to check the correctness of rustc labels Nov 29, 2020
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me with nits fixed (feel free to ignore strip_prefix though if it doesn't seem like net improvement after modifying).

Some((lineno, line)) => {
let line = line.trim();
if line.starts_with("/// ") {
doc_lines.push(line.trim()[4..].to_string());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW strip_prefix would probably be a little better. But I guess we need the trim and to_string either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I think the strip_prefix is better. I'm not sure why, but I keep forgetting about it.

"
This error was generated by the lint-docs tool.
This tool extracts documentation for lints from the source code and places
them in the rustc book. See the declare_lint! documentation for an example of
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we link to the documentation or list the file of declare_lint!? I guess it's a macro_rules! declare_lint grep away, but that's not the most intuitive thing I imagine.

This will hopefully help users figure out what was wrong and how
to fix it.
@ehuss
Copy link
Contributor Author

ehuss commented Nov 29, 2020

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Nov 29, 2020

📌 Commit a90fdfc has been approved by Mark-Simulacrum

@bors 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 Nov 29, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Nov 30, 2020
…imulacrum

Validate lint docs separately.

This addresses some concerns raised in rust-lang#76549 (comment) about errors with the lint docs being confusing and cumbersome. Errors from validating the lint documentation were being generated during `x.py doc` (and `x.py dist`), since extraction and validation are being done in a single step. This changes it so that extraction and validation are separated, so that `x.py doc` will not error if there is a validation problem, and tests are moved to `x.py test src/tools/lint-docs`.

This includes the following changes:

* Separate validation to `x.py test`.
* Added some more documentation on how to more easily modify and test the docs.
* Added more help to the error messages to hopefully provide more information on how to fix things.

The first commit just moves the code around, so you may consider looking at the other commits for a smaller diff.
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Dec 1, 2020
…imulacrum

Validate lint docs separately.

This addresses some concerns raised in rust-lang#76549 (comment) about errors with the lint docs being confusing and cumbersome. Errors from validating the lint documentation were being generated during `x.py doc` (and `x.py dist`), since extraction and validation are being done in a single step. This changes it so that extraction and validation are separated, so that `x.py doc` will not error if there is a validation problem, and tests are moved to `x.py test src/tools/lint-docs`.

This includes the following changes:

* Separate validation to `x.py test`.
* Added some more documentation on how to more easily modify and test the docs.
* Added more help to the error messages to hopefully provide more information on how to fix things.

The first commit just moves the code around, so you may consider looking at the other commits for a smaller diff.
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 1, 2020
Rollup of 11 pull requests

Successful merges:

 - rust-lang#79038 (Change ui test that are run-pass and that do not test the compiler to library tests)
 - rust-lang#79184 (Stop adding '*' at the end of slice and str typenames for MSVC case)
 - rust-lang#79227 (Remove const_fn_feature_flags test)
 - rust-lang#79444 (Move const ip in ui test to unit test)
 - rust-lang#79522 (Validate lint docs separately.)
 - rust-lang#79525 (Add -Z normalize-docs and enable it for compiler docs)
 - rust-lang#79527 (Move intra-doc link tests into a subdirectory)
 - rust-lang#79548 (Show since when a function is const in stdlib)
 - rust-lang#79568 (update Miri)
 - rust-lang#79573 (Update with status for various NetBSD ports.)
 - rust-lang#79583 (Update books)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 36ce8db into rust-lang:master Dec 1, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 1, 2020
@ehuss ehuss mentioned this pull request Dec 5, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 27, 2020
…Simulacrum

lint-docs: Warn on missing lint when documenting.

In rust-lang#79522, I missed converting one of the errors to a warning, in the situation where a lint is missing.  This was unearthed by the renaming of overlapping-patterns (rust-lang#78242).  This will still be validated during the test phase.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants