Fix rustdoc test directives that were accidentally ignored 🧐 #137099

yotamofek · 2025-02-15T21:16:55Z

Replace "// @" with "//@ ", and fix the tests so they actually pass, after directives are checked.
~~Only the first commit is mandatory, other two are small drive-bys.~~

rustbot · 2025-02-15T21:17:03Z

r? @fmease

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

fmease

Thanks for catching this! 🙏 We might want to modify {html,json}docck to reject // @ for the time being which is the retired syntax of these directives.

I'm not sure sure about the second commit though.

The fmt'ing commit is okay but generally we don't want to format tests as that may break them and since it's a bit churn-y.

fmease · 2025-02-15T21:40:07Z

tests/rustdoc/anchor-id-duplicate-method-name-25001.rs

I'm not sure how this makes it "more explicit in what it checks". #25001 is about items with duplicate names (here: quux), so I'm not sure why return types are relevant here?

I found this test was broken when I was messing around with the way the trait implementations for a certain type are ordered.
The //@ has directives for the associated type also make their order doesn't change, not just that there are three unique IDs, since the different types are part of the test. I thought it would be nice if the trait methods acted the same way, not because of the specific issue the test was written for, but just generally so if the order ever changes, it's not accidental.
I don't mind reverting this commit though :)

I'll check and reply later today

Please don't waste too much time on this, it's completely inconsequential. :)

fmease · 2025-02-15T21:41:26Z

tests/rustdoc/anchor-id-duplicate-method-name-25001.rs

-    //@ has - '//*[@id="method.quux"]//h4[@class="code-header"]' 'fn quux(self)'
-    fn quux(self) {}
+    //@ has - '//*[@id="method.quux"]//h4[@class="code-header"]' 'fn quux(self) -> T'
+    fn quux(self) -> T {}


Also, while rustdoc supports it by virtue of not really resolving and typeck'ing function bodies, it'd rather see loop {} or unimplemented!() in there rather than a "dormant" type error.

Oops, didn't even notice this won't compile.

replace "// @" with "//@ ", and fix the tests so they actually pass, after directives are checked

yotamofek · 2025-02-16T19:35:45Z

Hey @fmease , I rebased with just that first, less contentious commit. Hoping to get it in so it can also unblock #137103 😊
The other two commits weren't that important anyways.

fmease · 2025-02-23T11:15:25Z

@bors r+ rollup

bors · 2025-02-23T11:15:28Z

📌 Commit 2c1f489 has been approved by fmease

It is now in the queue for this repository.

Rollup of 8 pull requests Successful merges: - rust-lang#136439 (Misc. `rustc_codegen_ssa` cleanups 🧹) - rust-lang#136543 (intrinsics: unify rint, roundeven, nearbyint in a single round_ties_even intrinsic) - rust-lang#136637 (Add binary_format to rustc target specs) - rust-lang#137099 (Fix rustdoc test directives that were accidentally ignored 🧐) - rust-lang#137297 (Update `compiler-builtins` to 0.1.147) - rust-lang#137451 (FIx `sym` -> `syn` typo in tail-expr-drop-order type opt-out) - rust-lang#137452 (bootstrap: add module docs for core:metadata) - rust-lang#137483 (rename sub_ptr to offset_from_unsigned) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#137099 - yotamofek:pr/rustdoc/fix-ignored-test-directives, r=fmease Fix rustdoc test directives that were accidentally ignored 🧐 Replace "// `@"` with "//@ ", and fix the tests so they actually pass, after directives are checked. ~~Only the first commit is mandatory, other two are small drive-bys.~~

…directives, r=notriddle Fix (more) test directives that were accidentally ignored Continuation of rust-lang#137099 , caught by rust-lang#137103 (and needed to unblock that one). These test directives were accidentally using the old (`// `@`)` syntax

…directives, r=notriddle Fix (more) test directives that were accidentally ignored Continuation of rust-lang#137099 , caught by rust-lang#137103 (and needed to unblock that one). These test directives were accidentally using the old (`// ``@`)`` syntax

…directives, r=notriddle Fix (more) test directives that were accidentally ignored Continuation of rust-lang#137099 , caught by rust-lang#137103 (and needed to unblock that one). These test directives were accidentally using the old (`// ```@`)``` syntax

Rollup merge of rust-lang#137540 - yotamofek:pr/more-deprecated-test-directives, r=notriddle Fix (more) test directives that were accidentally ignored Continuation of rust-lang#137099 , caught by rust-lang#137103 (and needed to unblock that one). These test directives were accidentally using the old (`// ```@`)``` syntax

…ted-syntax, r=aDotInTheVoid {json|html}docck: catch and error on deprecated syntax rust-lang#137099 (review)

Rollup merge of rust-lang#137103 - yotamofek:pr/jsonhtmldocck-deprecated-syntax, r=aDotInTheVoid {json|html}docck: catch and error on deprecated syntax rust-lang#137099 (review)

rustbot assigned fmease Feb 15, 2025

fmease approved these changes Feb 15, 2025

View reviewed changes

fmease 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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 15, 2025

yotamofek mentioned this pull request Feb 15, 2025

{json|html}docck: catch and error on deprecated syntax #137103

Merged

fix rustdoc test directives that were accidentally ignored

2c1f489

replace "// @" with "//@ ", and fix the tests so they actually pass, after directives are checked

yotamofek force-pushed the pr/rustdoc/fix-ignored-test-directives branch from 059d284 to 2c1f489 Compare February 16, 2025 19:36

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 Feb 23, 2025

tgross35 mentioned this pull request Feb 23, 2025

Rollup of 8 pull requests #137497

Merged

bors merged commit 5f35f78 into rust-lang:master Feb 24, 2025
6 checks passed

rustbot added this to the 1.87.0 milestone Feb 24, 2025

yotamofek mentioned this pull request Feb 24, 2025

Fix (more) test directives that were accidentally ignored #137540

Merged

yotamofek deleted the pr/rustdoc/fix-ignored-test-directives branch February 24, 2025 21:24

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 1, 2025

Rollup merge of rust-lang#137103 - yotamofek:pr/jsonhtmldocck-depreca…

cdc47e7

…ted-syntax, r=aDotInTheVoid {json|html}docck: catch and error on deprecated syntax rust-lang#137099 (review)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 2, 2025

Rollup merge of rust-lang#137103 - yotamofek:pr/jsonhtmldocck-depreca…

de85a27

…ted-syntax, r=aDotInTheVoid {json|html}docck: catch and error on deprecated syntax rust-lang#137099 (review)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 2, 2025

Rollup merge of rust-lang#137103 - yotamofek:pr/jsonhtmldocck-depreca…

7b9ce1d

…ted-syntax, r=aDotInTheVoid {json|html}docck: catch and error on deprecated syntax rust-lang#137099 (review)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 3, 2025

Rollup merge of rust-lang#137103 - yotamofek:pr/jsonhtmldocck-depreca…

0447829

…ted-syntax, r=aDotInTheVoid {json|html}docck: catch and error on deprecated syntax rust-lang#137099 (review)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix rustdoc test directives that were accidentally ignored 🧐 #137099

Fix rustdoc test directives that were accidentally ignored 🧐 #137099

yotamofek commented Feb 15, 2025 •

edited by fmease

Loading

rustbot commented Feb 15, 2025

fmease left a comment •

edited

Loading

fmease Feb 15, 2025

yotamofek Feb 15, 2025 •

edited

Loading

fmease Feb 23, 2025

yotamofek Feb 23, 2025

fmease Feb 15, 2025

yotamofek Feb 15, 2025

yotamofek commented Feb 16, 2025

fmease commented Feb 23, 2025

bors commented Feb 23, 2025

Fix rustdoc test directives that were accidentally ignored 🧐 #137099

Fix rustdoc test directives that were accidentally ignored 🧐 #137099

Conversation

yotamofek commented Feb 15, 2025 • edited by fmease Loading

rustbot commented Feb 15, 2025

fmease left a comment • edited Loading

Choose a reason for hiding this comment

fmease Feb 15, 2025

Choose a reason for hiding this comment

yotamofek Feb 15, 2025 • edited Loading

Choose a reason for hiding this comment

fmease Feb 23, 2025

Choose a reason for hiding this comment

yotamofek Feb 23, 2025

Choose a reason for hiding this comment

fmease Feb 15, 2025

Choose a reason for hiding this comment

yotamofek Feb 15, 2025

Choose a reason for hiding this comment

yotamofek commented Feb 16, 2025

fmease commented Feb 23, 2025

bors commented Feb 23, 2025

yotamofek commented Feb 15, 2025 •

edited by fmease

Loading

fmease left a comment •

edited

Loading

yotamofek Feb 15, 2025 •

edited

Loading