Remove `rustc::existing_doc_keyword` lint #134202

nnethercote · 2024-12-12T07:29:07Z

The check doesn't require a lint.

nnethercote · 2024-12-12T07:29:18Z

Details in individual commits.

rustbot · 2024-12-12T07:29:19Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

rustbot · 2024-12-12T08:59:08Z

Some changes occurred in GUI tests.

cc @GuillaumeGomez

GuillaumeGomez · 2024-12-12T10:29:28Z

tests/rustdoc-gui/src/test_docs/lib.rs

@@ -156,7 +156,7 @@ pub enum AnEnum {
    WithVariants { and: usize, sub: usize, variants: usize },
 }

-#[doc(keyword = "CookieMonster")]
+#[doc(keyword = "auto")]


Why this change?

The test now fails if the value isn't a valid keyword.

Well, that brings this question: why this change?

Previously the "is it a valid keyword?" test was done by the lint. It has now moved to check_doc_keyword and it runs in several test cases where the lint did not.

No I mean, doc(keyword) was voluntarily left "open" in what kind of keyword we could define. You changed this behaviour. That's the part I don't understand. I don't mind changing it away from being a lint.

The lint checked this:

fn is_doc_keyword(s: Symbol) -> bool { s <= kw::Union }

That hasn't changed (well, sym::SelfTy is now also accepted, for the exception). What has changed is how often this check is being applied.

I see, my bad.

GuillaumeGomez · 2024-12-12T10:29:38Z

tests/rustdoc-json/keyword.rs

@@ -15,6 +15,6 @@ pub mod foo {}

 //@ !has "$.index[*][?(@.name=='hello')]"
 //@ !has "$.index[*][?(@.name=='bar')]"
-#[doc(keyword = "hello")]
+#[doc(keyword = "break")]


Why this change?

tests/ui/internal-lints/existing_doc_keyword.rs

nnethercote · 2024-12-12T22:34:53Z

@GuillaumeGomez: the tests/rustdoc-gui/search-result-color.goml is driving me crazy. I can't understand how it works. The keyword used for it is CookieMonster. The docs for the keyword are given in tests/rustdoc-gui/src/test_docs/lib.rs and the test search is specified by tests/rustdoc-gui/search-result-keyword.goml.

It seems like I should just be able to change CookieMonster in those two files to any other identifier. In practice, some changes work and some don't. Some examples (all from a repo without this PR applied):

Run the test (x test tests/rustdoc-gui/search-result-color.goml): pass
Change CookieMonster to ZookieMonster in
tests/rustdoc-gui/src/test_docs/lib.rs and
tests/rustdoc-gui/search-result-keyword.goml, run test: fail (3 errors)
Change ZookieMonster back to CookieMonster, run test: fail (4 errors)
Delete build/x86_64-unknown-linux-gnu/test/, run test: pass
Change CookieMonster to CookieThief, run test: pass
Change CookieThief to Mookie, run test: fail (3 errors)
Delete build/x86_64-unknown-linux-gnu/test/, run test: fail (3 errors)
Change Mookie to Cook, run test: fail (4 errors)
Delete build/x86_64-unknown-linux-gnu/test/, run test: pass

Keywords that are similar to CookieMonster and start with C tend to work. Those that aren't similar fail. Sometimes deleting the test/ dir is required to get things working again after a failed attempt.

I have viewed the generated docs in a browser. They always work fine no matter what keyword I use, and the search always works as expected. So I'm baffled. I just want to change CookieMonster to any existing keyword (such as auto) to satisfy the keyword check. Any ideas what is happening with this test?

GuillaumeGomez · 2024-12-13T10:59:13Z

tests/rustdoc-gui/search-result-keyword.goml

 // To be SURE that the search will be run.
 press-key: 'Enter'
 // Waiting for the search results to appear...
 wait-for: "#search-tabs"
-assert-text: (".result-keyword .result-name", "keyword CookieMonster")
+assert-text: (".result-keyword .result-name", "keyword auto")


It's checking here the first search result. Considering that auto might not be the first item, we'll need to tweak it a bit.

I checked for that, auto is the first item. ZookieMonster was too, as were a bunch of other things I tried.

GuillaumeGomez · 2024-12-13T11:01:15Z

tests/rustdoc-gui/search-result-keyword.goml

@@ -1,8 +1,8 @@
 // Checks that the "keyword" results have the expected text alongside them.
 go-to: "file://" + |DOC_PATH| + "/test_docs/index.html"
-write-into: (".search-input", "CookieMonster")
+write-into: (".search-input", "auto")


try:

Suggested change

write-into: (".search-input", "auto")

write-into: (".search-input", "keyword:auto")

Also you can open the generated docs with a web browser with build/[arch]/tests/rustdoc-gui/doc/lib2/index.html and test it manually.

I'll try the keyword change on Monday. I did already try testing manually in a browser, it always worked fine no matter what I tried. That's why it was so baffling why some things worked but most didn't -- I could find an explanation to explain the success/failure differences.

GuillaumeGomez · 2024-12-13T13:47:19Z

Found the issue in search-result-color.goml: there is no keyword displayed, meaning the test will fail since we can't check the color for a keyword. We need to use another keyword that might match other items.

GuillaumeGomez · 2024-12-13T13:58:34Z

I pushed a fix to your branch. To keep the fo query working, I used the for keyword instead.

nnethercote · 2024-12-15T21:48:50Z

Thank you! It was the "/test_docs/index.html?search=coo" that I overlooked. That explains why CookieMonster and CookieThief and similar terms worked, but other terms didn't.

nnethercote · 2024-12-15T21:50:24Z

I'm happy for this to merge if you are.

GuillaumeGomez · 2024-12-16T00:00:36Z

I am too so let's go!

@bors r=GuillaumeGomez,nnethercote

bors · 2024-12-16T00:00:39Z

📌 Commit bdbaeae has been approved by GuillaumeGomez,nnethercote

It is now in the queue for this repository.

…d, r=GuillaumeGomez,nnethercote Remove `rustc::existing_doc_keyword` lint The check doesn't require a lint. r? `@GuillaumeGomez`

…llaumeGomez Rollup of 6 pull requests Successful merges: - rust-lang#134111 (Fix `--nocapture` for run-make tests) - rust-lang#134202 (Remove `rustc::existing_doc_keyword` lint) - rust-lang#134314 (Make sure to use normalized ty for unevaluated const in default struct value) - rust-lang#134329 (Add m68k_target_feature) - rust-lang#134342 (crashes: more tests) - rust-lang#134354 (Handle fndef rendering together with signature rendering) Failed merges: - rust-lang#133265 (Add a range argument to vec.extract_if) r? `@ghost` `@rustbot` modify labels: rollup

…d, r=GuillaumeGomez,nnethercote Remove `rustc::existing_doc_keyword` lint The check doesn't require a lint. r? ``@GuillaumeGomez``

…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#134202 (Remove `rustc::existing_doc_keyword` lint) - rust-lang#134314 (Make sure to use normalized ty for unevaluated const in default struct value) - rust-lang#134342 (crashes: more tests) - rust-lang#134354 (Handle fndef rendering together with signature rendering) r? `@ghost` `@rustbot` modify labels: rollup

…d, r=GuillaumeGomez,nnethercote Remove `rustc::existing_doc_keyword` lint The check doesn't require a lint. r? ```@GuillaumeGomez```

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#134202 (Remove `rustc::existing_doc_keyword` lint) - rust-lang#134354 (Handle fndef rendering together with signature rendering) - rust-lang#134368 (Use links to edition guide for edition migrations) - rust-lang#134371 (Check for array lengths that aren't actually `usize`) - rust-lang#134378 (An octuple of polonius fact generation cleanups) Failed merges: - rust-lang#134365 (Rename `rustc_mir_build::build` to `builder`) r? `@ghost` `@rustbot` modify labels: rollup

aDotInTheVoid

@bors r- until rustdoc-json text fixed.

tests/rustdoc-json/keyword.rs

aDotInTheVoid · 2024-12-16T23:28:39Z

@bors r-

All the other unconditional keywords are in the alphabetical order, but `while` is for some reason not.

`CheckAttrVisitor::check_doc_keyword` checks `#[doc(keyword = "..")]` attributes to ensure they are on an empty module, and that the value is a non-empty identifier. The `rustc::existing_doc_keyword` lint checks these attributes to ensure that the value is the name of a keyword. It's silly to have two different checking mechanisms for these attributes. This commit does the following. - Changes `check_doc_keyword` to check that the value is the name of a keyword (avoiding the need for the identifier check, which removes a dependency on `rustc_lexer`). - Removes the lint. - Updates tests accordingly. There is one hack: the `SelfTy` FIXME case used to used to be handled by disabling the lint, but now is handled with a special case in `is_doc_keyword`. That hack will go away if/when the FIXME is fixed. Co-Authored-By: Guillaume Gomez <guillaume1.gomez@gmail.com>

nnethercote · 2024-12-17T02:56:57Z

@bors r=GuillaumeGomez

bors · 2024-12-17T02:56:59Z

📌 Commit 121e87b has been approved by GuillaumeGomez

It is now in the queue for this repository.

…d, r=GuillaumeGomez Remove `rustc::existing_doc_keyword` lint The check doesn't require a lint. r? `@GuillaumeGomez`

Rollup of 9 pull requests Successful merges: - rust-lang#133265 (Add a range argument to vec.extract_if) - rust-lang#134202 (Remove `rustc::existing_doc_keyword` lint) - rust-lang#134354 (Handle fndef rendering together with signature rendering) - rust-lang#134365 (Rename `rustc_mir_build::build` to `builder`) - rust-lang#134368 (Use links to edition guide for edition migrations) - rust-lang#134388 (Update books) - rust-lang#134397 (rustc_borrowck: Suggest changing `&raw const` to `&raw mut` if applicable) - rust-lang#134398 (AIX: add alignment info for test) - rust-lang#134400 (Fix some comments related to upvars handling) Failed merges: - rust-lang#134399 (Do not do if ! else, use unnegated cond and swap the branches instead) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 10 pull requests Successful merges: - rust-lang#134202 (Remove `rustc::existing_doc_keyword` lint) - rust-lang#134354 (Handle fndef rendering together with signature rendering) - rust-lang#134365 (Rename `rustc_mir_build::build` to `builder`) - rust-lang#134368 (Use links to edition guide for edition migrations) - rust-lang#134397 (rustc_borrowck: Suggest changing `&raw const` to `&raw mut` if applicable) - rust-lang#134398 (AIX: add alignment info for test) - rust-lang#134400 (Fix some comments related to upvars handling) - rust-lang#134406 (Fix `-Z input-stats` ordering) - rust-lang#134409 (bootstrap: fix a comment) - rust-lang#134412 (small borrowck cleanup) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#134202 - nnethercote:rm-existing_doc_keyword, r=GuillaumeGomez Remove `rustc::existing_doc_keyword` lint The check doesn't require a lint. r? ``@GuillaumeGomez``

rustbot assigned GuillaumeGomez Dec 12, 2024

This comment has been minimized.

Sign in to view

nnethercote force-pushed the rm-existing_doc_keyword branch from 7a0a3f2 to 353aff4 Compare December 12, 2024 08:59

This comment has been minimized.

Sign in to view

GuillaumeGomez reviewed Dec 12, 2024

View reviewed changes

tests/ui/internal-lints/existing_doc_keyword.rs Show resolved Hide resolved

GuillaumeGomez reviewed Dec 13, 2024

View reviewed changes

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 Dec 16, 2024

GuillaumeGomez mentioned this pull request Dec 16, 2024

Rollup of 6 pull requests #134359

Closed

matthiaskrgr mentioned this pull request Dec 16, 2024

Rollup of 4 pull requests #134360

Closed

matthiaskrgr mentioned this pull request Dec 16, 2024

Rollup of 5 pull requests #134380

Closed

aDotInTheVoid reviewed Dec 16, 2024

View reviewed changes

tests/rustdoc-json/keyword.rs Outdated Show resolved Hide resolved

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 16, 2024

nnethercote and others added 2 commits December 17, 2024 13:40

Move doc(keyword = "while").

f10169c

All the other unconditional keywords are in the alphabetical order, but `while` is for some reason not.

nnethercote force-pushed the rm-existing_doc_keyword branch from bdbaeae to 121e87b Compare December 17, 2024 02:56

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 17, 2024

jhpratt added a commit to jhpratt/rust that referenced this pull request Dec 17, 2024

Rollup merge of rust-lang#134202 - nnethercote:rm-existing_doc_keywor…

d5b8fb4

…d, r=GuillaumeGomez Remove `rustc::existing_doc_keyword` lint The check doesn't require a lint. r? `@GuillaumeGomez`

jhpratt mentioned this pull request Dec 17, 2024

Rollup of 9 pull requests #134411

Closed

jhpratt mentioned this pull request Dec 17, 2024

Rollup of 10 pull requests #134414

Merged

bors merged commit 52b4557 into rust-lang:master Dec 17, 2024
6 checks passed

rustbot added this to the 1.85.0 milestone Dec 17, 2024

nnethercote deleted the rm-existing_doc_keyword branch December 17, 2024 21:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `rustc::existing_doc_keyword` lint #134202

Remove `rustc::existing_doc_keyword` lint #134202

nnethercote commented Dec 12, 2024

nnethercote commented Dec 12, 2024

rustbot commented Dec 12, 2024

This comment has been minimized.

rustbot commented Dec 12, 2024

This comment has been minimized.

GuillaumeGomez Dec 12, 2024

nnethercote Dec 12, 2024

GuillaumeGomez Dec 12, 2024

nnethercote Dec 12, 2024

GuillaumeGomez Dec 12, 2024

nnethercote Dec 12, 2024

GuillaumeGomez Dec 12, 2024

GuillaumeGomez Dec 12, 2024

nnethercote Dec 12, 2024

nnethercote commented Dec 12, 2024 •

edited

Loading

GuillaumeGomez Dec 13, 2024

nnethercote Dec 13, 2024

GuillaumeGomez Dec 13, 2024

nnethercote Dec 13, 2024

GuillaumeGomez commented Dec 13, 2024

GuillaumeGomez commented Dec 13, 2024

nnethercote commented Dec 15, 2024 •

edited

Loading

nnethercote commented Dec 15, 2024

GuillaumeGomez commented Dec 16, 2024

bors commented Dec 16, 2024

aDotInTheVoid left a comment

aDotInTheVoid commented Dec 16, 2024

nnethercote commented Dec 17, 2024

bors commented Dec 17, 2024

	write-into: (".search-input", "auto")
	write-into: (".search-input", "keyword:auto")

Remove rustc::existing_doc_keyword lint #134202

Remove rustc::existing_doc_keyword lint #134202

Conversation

nnethercote commented Dec 12, 2024

nnethercote commented Dec 12, 2024

rustbot commented Dec 12, 2024

This comment has been minimized.

rustbot commented Dec 12, 2024

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nnethercote commented Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Dec 13, 2024

GuillaumeGomez commented Dec 13, 2024

nnethercote commented Dec 15, 2024 • edited Loading

nnethercote commented Dec 15, 2024

GuillaumeGomez commented Dec 16, 2024

bors commented Dec 16, 2024

aDotInTheVoid left a comment

Choose a reason for hiding this comment

aDotInTheVoid commented Dec 16, 2024

nnethercote commented Dec 17, 2024

bors commented Dec 17, 2024

Remove `rustc::existing_doc_keyword` lint #134202

Remove `rustc::existing_doc_keyword` lint #134202

nnethercote commented Dec 12, 2024 •

edited

Loading

nnethercote commented Dec 15, 2024 •

edited

Loading