rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.55k stars 12.74k forks source link

Discussion: Enforce formatting of doc comments #105198

Open AldaronLau opened 1 year ago

AldaronLau commented 1 year ago

Currently, the examples in the Rust documentation is inconsistent in style. For instance, there are many examples that use 3 space tabs (probably accidentally), while most use 4.

How do we enforce that examples are formatted correctly? We have a few options.

Rustfmt

Use rustfmt's format_code_in_doc_comments.

While this initially seems like a good solution, there are a couple problems:

How do we solve the second problem? I see two possible ways this can be done (by changing rust-fmt).

Add docs table to rustfmt.toml

Rustfmt could add support for alternate rustfmt settings for doc comments.

edition = "2021"
unstable_features = true
use_small_heuristics = "Max"
format_code_in_doc_comments = true

[docs]
use_small_heuristics = "Default"

Add rustfmt-docs.toml

Add an additional file that has the same format as rustfmt.toml, but applies to docs.


What if changes aren't/can't be made in Rustfmt?

There are 3 main solutions in this category.

Move docs out to their own folder with another rustfmt.toml file

This is something that could be done without additional tooling.

```rust
#![doc = include_str!("../docs/name_of_example.rs")]
```

Of course, this would be a very large change from where doc comments are written today, and it could be argued that having it separated out will make things harder to follow.

Extract docs with tooling and check against rustfmt.toml file

A tool could be made (as part of tidy) to verify docs are formatted correctly. Rust code could be parsed for doc comments, and written into temporary files in the same directory with a .rustfmt folder.

Extract docs with tooling and format with another rustfmt.toml file then overwrite

A tool could be made (as part of tidy) to autoformat doc comments. Implementation details would be similar to the previous option.


How should this be approached? I see pros and cons to all solutions, but I think it's something that definitely should be solved. Are there any approaches I missed? I tried to list all of the ones I could think of, but could have missed some.

GuillaumeGomez commented 1 year ago

I think I remember an old issue suggesting this too. I think it'd be nice to format doc comments as well but that's something that might better brought up with the @rust-lang/rustfmt team?

ytmimi commented 1 year ago

We have a new unstable option called doc_comment_code_block_width that one can use alongside format_code_in_doc_comments to set the width.

We've also got https://github.com/rust-lang/rustfmt/pull/5288 opened to address the trailing whitespace issue.

AldaronLau commented 1 year ago

The one thing I'd be skeptical about is since it is desired by many individuals that the docs formatting specialize in a lot more than just code block width, that there would need to be doc_comment_* for a lot of format options. The things I know for sure are going to need to be different for docs for a PR to be accepted by the library team (from previous conversation) are at least:

There are probably others I missed, too.

tgross35 commented 1 year ago

Echoing what I mentioned here, enabling rustfmt on doc comments should probably wait on https://github.com/rust-lang/rustfmt/pull/5601 and https://github.com/rust-lang/rustfmt/pull/5536 to land

Those are both marked ready for merge so hopefully won't be too long

ytmimi commented 1 year ago

@AldaronLau would it be possible to link to some examples of what you're looking for or other discussions that you've had?

Maybe this should be discussed somewhere else, but based on https://github.com/rust-lang/rustfmt/issues/5623 I think it would also be useful to define a new code fence attribute that rustfmt understands as skip formatting and rustdoc understands as still run the doctest.

This could be useful in cases where issues / limitations in rustfmt prevent the user from formatting the doc comment exactly how they want.

AldaronLau commented 1 year ago

I don't think we have a notion of column width that differs from the length of a line.

@ytmimi the new doc_comment_code_block_width option works for this.

Comment Formatting

There are many places in the Rust documentation where something like the following:

do_something();      // 1. First step
do_next();           // 2. Second step
do_something_else(); // 3. Third step

Would be reformatted as:

do_something(); // 1. First step
do_next(); // 2. Second step
do_something_else(); // 3. Third step

This is something that needs a flag to disable this kind of comment formatting specifically for doc comments before a new PR can be made.

Heuristics

There places in the Rust documentation where the following:

let abcd = [
    "a",
    "b",
    "c",
    "d",
];

Would be reformatted as:

let abcd = ["a", "b", "c", "d"];

This is undesirable, and needs a separate flag to change use_small_heuristics to Off specifically for doc comments.


Maybe this should be discussed somewhere else, but based on https://github.com/rust-lang/rustfmt/issues/5623 I think it would also be useful to define a new code fence attribute that rustfmt understands as skip formatting and rustdoc understands as still run the doctest.

This could be useful in cases where issues / limitations in rustfmt prevent the user from formatting the doc comment exactly how they want.

If I'm understanding right, that would just be a shorthand for:

/// ```rust
/// # #![rustfmt::skip]
/// ```
pub struct Unit;

Right?

AldaronLau commented 1 year ago

@ytmimi These requirements are based on the discussion in the PR comments for https://github.com/rust-lang/rust/pull/104058, when I attempted to get something in place for this (which was admittedly way too soon).

ytmimi commented 1 year ago

the new doc_comment_code_block_width option works for this.

Great!


There are many places in the Rust documentation where something like the following:

do_something();      // 1. First step
do_next();           // 2. Second step
do_something_else(); // 3. Third step

Would be reformatted as:

do_something(); // 1. First step
do_next(); // 2. Second step
do_something_else(); // 3. Third step

Got it. to my knowledge rustfmt doesn't currently have any option to preserver the whitespace before a trailing comment.


needs a separate flag to change use_small_heuristics to Off specifically for doc comments.

If it's just use_small_heuristics then I don't see why we couldn't implement another doc_comment_* config for that, but if we need super fine grained control over doc comment formatting that varies drastically from the other configs then I can see the utility in a [docs] table in rustfmt.toml.

ytmimi commented 1 year ago

If I'm understanding right, that would just be a shorthand for:

/// ```rust
/// # #![rustfmt::skip]
/// ```
pub struct Unit;

Right?

You've got the right idea! Not to get too into the weeds, but hidden rustdoc lines prevent us from parsing the code block as valid rust since random #s at the start of a line aren't valid syntax. In order to get around this we convert all hidden lines into custom comments so # #![rustfmt::skip] becomes //#### #![rustfmt::skip] before formatting. Then we just make sure to reverse our transformation before emitting the formatted code block.

I had something more like this in mind:

// using `rustfmt::skip` here for consistency with `#[rustfmt::skip]` and `#![rustfmt::skip]`

/// ```rustfmt::skip
/// do_something();      // 1. First step
/// do_next();           // 2. Second step
/// do_something_else(); // 3. Third step
/// ```
pub struct Unit;

The main advantage here is that we could avoid trying to format the code block altogether instead of parsing the code block just to discover there's a #![rustfmt::skip], which will prevent formatting anyway (and for now hit a bug mentioned in https://github.com/rust-lang/rustfmt/issues/5623).

This skipping behavior already exists when we encounter rustdoc's ignore and compile_fail attributes so the idea is to add a rustfmt specific attribute that won't prevent rustdoc from running doc tests.

Also, as demonstrated above it could be used as a workaround to not mess with comment alignment.

calebcartwright commented 1 year ago

I'm going to suggest somewhat of a pivot in how this issue is utilized going forward. I'm not keen on discussing rustfmt specifics outside of rust-lang/rustfmt because that's where we have our issue tracker, do development, etc., not to mention that a couple lines of rustfmt-centric discussion on this thread already have well trodden discussions within r-l/rustfmt that I do not want to bifurcate.

We can share succinct updates and/or decisions from the rustfmt perspective here, but I think that's best provided as a short data point that can be used in the consideration of the questions/decisions outlined in the OP

GuillaumeGomez commented 1 year ago

Or we can move this issue directly on rustfmt repository.

calebcartwright commented 1 year ago

Or we can move this issue directly on rustfmt repository.

Understand the thinking, but I'm pretty strongly opposed to doing that. The purpose of this issue, at least AIUI, is to figure out whether or not to apply automated formatting to comments in Rust documentation, and if so, how.

In my opinion that's neither a rustfmt consideration nor decision, rustfmt is just one of the options under consideration for the "how".

Moving this issue to r-l/rustfmt would result in us having an issue that's half duplicates of existing rustfmt issues, and half off-topic/non-actionable for rustfmt

zkrising commented 1 year ago

Somewhat related, a lot of the official documentation uses move|| {} for move closures rather than the rustfmt-preferred move || {}. Is this intentional, or would it be preferable that this is consistent with rustfmt?

GuillaumeGomez commented 1 year ago

Somewhat related, a lot of the official documentation uses move|| {} for move closures rather than the rustfmt-preferred move || {}. Is this intentional, or would it be preferable that this is consistent with rustfmt?

I think that's a typo.