rustdoc: allow list syntax for #[doc(alias)] attributes #82846

GuillaumeGomez · 2021-03-06T21:04:42Z

It now allows to have:

#[doc(alias = "x")]
// and:
#[doc(alias("y", "z"))]

cc @jplatte
r? @jyn514

jplatte · 2021-03-06T21:22:50Z

Nice! I didn't really think about syntax when creating #81205, I agree that this syntax looks better :)

compiler/rustc_passes/src/check_attr.rs

src/doc/rustdoc/src/advanced-features.md

src/librustdoc/clean/types.rs

GuillaumeGomez · 2021-03-08T16:31:42Z

Updated!

compiler/rustc_passes/src/check_attr.rs

jyn514 · 2021-03-08T21:38:03Z

@rfcbot fcp merge

This allows writing #[doc(alias("y", "z")] instead of

#[doc(alias = "y")]
#[doc(alias = "z")]

This makes the code more convenient to write, and does not conflict with any other possible features.

rfcbot · 2021-03-08T21:38:05Z

Team member @jyn514 has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

camelid

The goal of this change seems good to me. However, I'm wondering how this new syntax would interact with what's proposed in #82000. That issue proposes this syntax:

#[doc(alias = "sprintf", lang = "C, C++, POSIX")]

but I wonder if a less ambiguous form might end up being:

#[doc(alias("sprintf", lang = "C, C++, POSIX"))]

Would something like that still be possible after this change? I think so, but I wanted to check.

compiler/rustc_passes/src/check_attr.rs

camelid · 2021-03-09T02:42:00Z

Also, I noticed that @GuillaumeGomez proposed in #81205 that

Accepting both syntaxes seems like a good compromise. Well, I'll open a PR and we can debate over the syntax there.

Meaning accept both alias = ["one", "two"] and alias("one", "two"). What does this PR implement? I think it would be ideal to settle on only one syntax.

bors · 2021-03-09T05:06:35Z

☔ The latest upstream changes (presumably #82911) made this pull request unmergeable. Please resolve the merge conflicts.

Manishearth · 2021-03-09T05:33:17Z

Would something like that still be possible after this change? I think so, but I wanted to check.

Yes

Meaning accept both alias = ["one", "two"] and alias("one", "two"). What does this PR implement? I think it would be ideal to settle on only one syntax.

I would prefer to accept only one. I'm okay with alias = "foo" and alias("foo", "bar") coexisting but not alias = ["foo", "bar"], that last one is pretty uncommon in attributes.

GuillaumeGomez · 2021-03-09T08:57:03Z

Also, I noticed that @GuillaumeGomez proposed in #81205 that

Accepting both syntaxes seems like a good compromise. Well, I'll open a PR and we can debate over the syntax there.

Meaning accept both alias = ["one", "two"] and alias("one", "two"). What does this PR implement? I think it would be ideal to settle on only one syntax.

I was referring to alias = "x" and alias("x", "y"). I said that using [] was a bad idea (because no other attributes is using it, making it seem on its own).

GuillaumeGomez · 2021-03-09T09:16:59Z

@rfcbot fcp merge

This allows writing #[doc(alias("y", "z")] instead of

It's not instead of. Both syntaxes are working. :)

rfcbot · 2021-03-09T09:37:15Z

🔔 This is now entering its final comment period, as per the review above. 🔔

compiler/rustc_passes/src/check_attr.rs

GuillaumeGomez · 2021-03-12T20:19:13Z

Updated! Anything else to be done here?

jyn514 · 2021-03-13T05:49:22Z

@GuillaumeGomez this is still waiting on FCP: #82846 (comment)

pickfire · 2021-03-16T11:26:08Z

I find this not so intuitive since it is using = for one item and is likely to make a mistake here.

Can we have a suggestion when someone is trying #[doc(alias = ["a", "b"])]? I think I (or someone else) might try that.

At least they don't have to search the rust reference or online to find out how it works. Maybe we can have a clippy lint later on this to recommend using the multiple items style.

Also, why not just soft deprecate the old one since we just have this lately?

pickfire · 2021-03-16T11:28:28Z

src/test/ui/rustdoc/check-doc-alias-attr.rs

 #[doc(alias = "\"")] //~ ERROR
 #[doc(alias = "\n")] //~ ERROR
 #[doc(alias = "
 ")] //~^ ERROR
 #[doc(alias = "\t")] //~ ERROR
 #[doc(alias = " hello")] //~ ERROR
 #[doc(alias = "hello ")] //~ ERROR
+#[doc(alias = "")] //~ ERROR


Suggested change

#[doc(alias = "")] //~ ERROR

#[doc(alias = "")] //~ ERROR

#[doc(alias = ["foo", "bar"])] //~ SUGGESTION

We should show users the correct way to have multiple aliases.

GuillaumeGomez · 2021-03-16T11:56:33Z

I find this not so intuitive since it is using = for one item and is likely to make a mistake here.

Can we have a suggestion when someone is trying #[doc(alias = ["a", "b"])]? I think I (or someone else) might try that.

There is no attribute in the rust compiler using this syntax, so why would anyone use it? It'll return a compiler error because it's invalid syntax.

At least they don't have to search the rust reference or online to find out how it works. Maybe we can have a clippy lint later on this to recommend using the multiple items style.

Again, it's assuming that people would use a syntax which doesn't exist anywhere else in the compiler.

Also, why not just soft deprecate the old one since we just have this lately?

Both syntaxes are correct and working, so why deprecating one of them?

pickfire · 2021-03-16T15:17:12Z

Again, it's assuming that people would use a syntax which doesn't exist anywhere else in the compiler.

It exists but not in attributes, it exists as [1, 2] as in array and vectors.

Is this the first compiler attribute does supports both single value and multiple values?

Both syntaxes are correct and working, so why deprecating one of them?

Because it seemed redundant, the alias("foo") can be done instead of alias = "foo".

GuillaumeGomez · 2021-03-16T15:19:11Z

It exists but not in attributes, it exists as [1, 2] as in array and vectors.

It's completely out of scope here.

pickfire · 2021-03-16T15:20:15Z

   Compiling playground v0.0.1 (/playground)
error: expected unsuffixed literal or identifier, found `[`
 --> src/lib.rs:1:15
  |
1 | #[doc(alias = ["foo"])]
  |               ^

error: aborting due to previous error

error: could not compile `playground`

The current compilation error does not seemed to be useful to let them know #[doc(alias("foo", "bar"))].

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=dcc601d35deecf1558b3dd66660bf8d5

GuillaumeGomez · 2021-03-16T15:23:02Z

And once again: why would an attribute take an array whereas no other attributes is doing so?

pickfire · 2021-03-17T00:47:14Z

And once again: why would an attribute take an array whereas no other attributes is doing so?

I don't mean to need it to take an array. What I am suggesting is that we show a better error just in case someone did that. Just error handling, still not valid code.

GuillaumeGomez · 2021-03-17T09:09:57Z

I think the compiler error is clear enough: this is a syntax error, not an attribute one.

jyn514 · 2021-03-17T13:29:19Z

I think the compiler error is clear enough: this is a syntax error, not an attribute one.

Right - this would need to be changed in the parser, not here, rustdoc has no control over it. @pickfire feel free to open an issue for that but it doesn't belong in this pr.

rfcbot · 2021-03-19T09:45:19Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

jyn514 · 2021-03-19T09:55:35Z

@bors r+

bors · 2021-03-19T09:55:36Z

📌 Commit 1d26e6b has been approved by jyn514

…n514 rustdoc: allow list syntax for #[doc(alias)] attributes Fixes rust-lang#81205. It now allows to have: ```rust #[doc(alias = "x")] // and: #[doc(alias("y", "z"))] ``` cc `@jplatte` r? `@jyn514`

Rollup of 11 pull requests Successful merges: - rust-lang#82500 (Reuse `std::sys::unsupported::pipe` on `hermit`) - rust-lang#82759 (Remove unwrap_none/expect_none from compiler/.) - rust-lang#82846 (rustdoc: allow list syntax for #[doc(alias)] attributes) - rust-lang#82892 (Clarify docs for Read::read's return value) - rust-lang#83179 (Extend `proc_macro_back_compat` lint to `actix-web`) - rust-lang#83197 (Move some test-only code to test files) - rust-lang#83208 (Fix gitattibutes for old git versions) - rust-lang#83215 (Deprecate std::os::haiku::raw, which accidentally wasn't deprecated) - rust-lang#83230 (Remove unnecessary `forward_inner_docs` hack) - rust-lang#83236 (Upgrade memmap to memmap2) - rust-lang#83270 (Fix typo/inaccuracy in the documentation of Iterator::skip_while) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned jyn514 Mar 6, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 6, 2021

GuillaumeGomez changed the title ~~Doc alias list~~ rustdoc: allow list syntax for #[doc(alias)] attributes Mar 6, 2021

GuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Mar 6, 2021

jyn514 reviewed Mar 7, 2021

View reviewed changes

jyn514 added the needs-fcp This change is insta-stable, so needs a completed FCP to proceed. label Mar 7, 2021

GuillaumeGomez force-pushed the doc-alias-list branch from 68c43c7 to 4673bdd Compare March 8, 2021 16:31

jyn514 reviewed Mar 8, 2021

View reviewed changes

compiler/rustc_passes/src/check_attr.rs Outdated Show resolved Hide resolved

compiler/rustc_passes/src/check_attr.rs Outdated Show resolved Hide resolved

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Mar 8, 2021

camelid reviewed Mar 9, 2021

View reviewed changes

compiler/rustc_passes/src/check_attr.rs Outdated Show resolved Hide resolved

rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Mar 9, 2021

GuillaumeGomez force-pushed the doc-alias-list branch 2 times, most recently from 9b5d5cf to 6aa5007 Compare March 9, 2021 09:57

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the doc-alias-list branch from 6aa5007 to acd7a20 Compare March 9, 2021 10:13

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the doc-alias-list branch from acd7a20 to 133d035 Compare March 9, 2021 10:26

camelid reviewed Mar 10, 2021

View reviewed changes

compiler/rustc_passes/src/check_attr.rs Outdated Show resolved Hide resolved

pickfire reviewed Mar 16, 2021

View reviewed changes

pickfire mentioned this pull request Mar 17, 2021

Improve attribute list syntax suggestion #83232

Open

rfcbot added the finished-final-comment-period The final comment period is finished for this PR / Issue. label Mar 19, 2021

rfcbot added to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Mar 19, 2021

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 Mar 19, 2021

This was referenced Mar 19, 2021

Rollup of 11 pull requests #83300

Closed

Rollup of 11 pull requests #83301

Merged

bors merged commit 61372e1 into rust-lang:master Mar 19, 2021

rustbot added this to the 1.52.0 milestone Mar 19, 2021

GuillaumeGomez deleted the doc-alias-list branch March 19, 2021 18:59

apiraino removed the to-announce Announce this issue on triage meeting label Mar 25, 2021

koutheir mentioned this pull request Aug 22, 2021

Build on Debian fails with "doc alias attribute expects a string:" koutheir/selinux#1

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustdoc: allow list syntax for #[doc(alias)] attributes #82846

rustdoc: allow list syntax for #[doc(alias)] attributes #82846

GuillaumeGomez commented Mar 6, 2021 •

edited

Loading

jplatte commented Mar 6, 2021

GuillaumeGomez commented Mar 8, 2021

jyn514 commented Mar 8, 2021

rfcbot commented Mar 8, 2021 •

edited by camelid

Loading

camelid left a comment

camelid commented Mar 9, 2021

bors commented Mar 9, 2021

Manishearth commented Mar 9, 2021

GuillaumeGomez commented Mar 9, 2021 •

edited

Loading

GuillaumeGomez commented Mar 9, 2021

rfcbot commented Mar 9, 2021

This comment has been minimized.

This comment has been minimized.

GuillaumeGomez commented Mar 12, 2021

jyn514 commented Mar 13, 2021

pickfire commented Mar 16, 2021 •

edited

Loading

pickfire Mar 16, 2021

GuillaumeGomez commented Mar 16, 2021

pickfire commented Mar 16, 2021 •

edited

Loading

GuillaumeGomez commented Mar 16, 2021

pickfire commented Mar 16, 2021

GuillaumeGomez commented Mar 16, 2021 •

edited

Loading

pickfire commented Mar 17, 2021 •

edited

Loading

GuillaumeGomez commented Mar 17, 2021

jyn514 commented Mar 17, 2021

rfcbot commented Mar 19, 2021

jyn514 commented Mar 19, 2021

bors commented Mar 19, 2021

	#[doc(alias = "")] //~ ERROR
	#[doc(alias = "")] //~ ERROR
	#[doc(alias = ["foo", "bar"])] //~ SUGGESTION

rustdoc: allow list syntax for #[doc(alias)] attributes #82846

rustdoc: allow list syntax for #[doc(alias)] attributes #82846

Conversation

GuillaumeGomez commented Mar 6, 2021 • edited Loading

jplatte commented Mar 6, 2021

GuillaumeGomez commented Mar 8, 2021

jyn514 commented Mar 8, 2021

rfcbot commented Mar 8, 2021 • edited by camelid Loading

camelid left a comment

Choose a reason for hiding this comment

camelid commented Mar 9, 2021

bors commented Mar 9, 2021

Manishearth commented Mar 9, 2021

GuillaumeGomez commented Mar 9, 2021 • edited Loading

GuillaumeGomez commented Mar 9, 2021

rfcbot commented Mar 9, 2021

This comment has been minimized.

This comment has been minimized.

GuillaumeGomez commented Mar 12, 2021

jyn514 commented Mar 13, 2021

pickfire commented Mar 16, 2021 • edited Loading

pickfire Mar 16, 2021

Choose a reason for hiding this comment

GuillaumeGomez commented Mar 16, 2021

pickfire commented Mar 16, 2021 • edited Loading

GuillaumeGomez commented Mar 16, 2021

pickfire commented Mar 16, 2021

GuillaumeGomez commented Mar 16, 2021 • edited Loading

pickfire commented Mar 17, 2021 • edited Loading

GuillaumeGomez commented Mar 17, 2021

jyn514 commented Mar 17, 2021

rfcbot commented Mar 19, 2021

jyn514 commented Mar 19, 2021

bors commented Mar 19, 2021

GuillaumeGomez commented Mar 6, 2021 •

edited

Loading

rfcbot commented Mar 8, 2021 •

edited by camelid

Loading

GuillaumeGomez commented Mar 9, 2021 •

edited

Loading

pickfire commented Mar 16, 2021 •

edited

Loading

pickfire commented Mar 16, 2021 •

edited

Loading

GuillaumeGomez commented Mar 16, 2021 •

edited

Loading

pickfire commented Mar 17, 2021 •

edited

Loading