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

doc attribute removes whitespace #70732

Closed
Luro02 opened this issue Apr 3, 2020 · 7 comments · Fixed by #78400
Closed

doc attribute removes whitespace #70732

Luro02 opened this issue Apr 3, 2020 · 7 comments · Fixed by #78400
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@Luro02
Copy link
Contributor

Luro02 commented Apr 3, 2020

I tried this code:

/// You are supposed to call it like this
///
/// # Example
///
/// ```
/// let example = Example::new()
///     .first("hello")
#[cfg_attr(not(feature = "one"), doc = "     .second(\"hello\")\n")]
///     .third("hello")
///     .build();
/// ```
pub struct Example {}

fn main() {}

with cargo doc --open

I expected to see this:

Screenshot_20200403_123723

Instead, this happened:

Screenshot_20200403_123648

Meta

rustc --version --verbose:

rustc 1.43.0-nightly (564758c4c 2020-03-08)
binary: rustc
commit-hash: 564758c4c329e89722454dd2fbb35f1ac0b8b47c
commit-date: 2020-03-08
host: x86_64-unknown-linux-gnu
release: 1.43.0-nightly
LLVM version: 9.0

running cargo expand over the code gives me

#![feature(prelude_import)]
#[prelude_import]                                                                                                                                                                             
use std::prelude::v1::*;                                                                                                                                                                      
#[macro_use]                                                                                                                                                                                  
extern crate std;                                                                                                                                                                             
/// You are supposed to call it like this
///                                                                                                                                                                                           
/// # Example                                                                                                                                                                                 
///                                                                                                                                                                                           
/// ```                                                                                                                                                                                       
/// let example = Example::new()                                                                                                                                                              
///     .first("hello")
///     .second("hello")
///     .third("hello")
///     .build();
/// ```
pub struct Example {}
@Luro02 Luro02 added the C-bug Category: This is a bug. label Apr 3, 2020
@jonas-schievink jonas-schievink added A-attributes Area: Attributes (`#[…]`, `#![…]`) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Apr 3, 2020
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Apr 6, 2020

As explained on discord, the attributes aren't applied to doc comments (EDIT: they are applied, the problem seems to be that it results in two doc comments for rustdoc). However, your doc comment is split in two so the second half isn't considered as being inside a code block, explaining why you have this display. Even if cargo expand shows it as one doc comment, rustdoc sees two of them.

Maybe we should add a warning or something that attributes aren't applied to doc comments... What do you think @rust-lang/rustdoc ?

@Mark-Simulacrum
Copy link
Member

How can an attribute get applied to a doc comment? Both are, under the hood, attributes, no? I would expect that to be equivalent to expecting #[cfg(foo)] #[derive(Debug)] struct Bar to only derive Debug if --cfg foo is passed, which is not how that works.

I'm guessing what is true is that doc = attributes directly written don't actually merge with ///-style comments. Potentially relevant: #65750, #60935, #60936

@Nemo157
Copy link
Member

Nemo157 commented Apr 6, 2020

Simplified reproducer

//! ```rust
//! foo();
#![doc = "   bar();"]
//! ```

Checking with 1.31.0 it still ignores it (so likely not #65750 related), but checking with 1.21.0 (and 1.0.0) it worked as expected, so somewhere within that range (but I don't have any other versions installed at the moment to check).

@Luro02
Copy link
Contributor Author

Luro02 commented Apr 6, 2020

but checking with 1.21.0 (and 1.0.0) it worked as expected, so somewhere within that range

Compilation broke somewhere between

  • 1.23.0 (works)
  • 1.24.0 (does not work)

I tested it with

//! ```rust
//! let x = r#"
#![doc = "   bar();"]
//! "#;
//! assert_eq!(x, "\n  bar();\n");
//! ```

@Luro02
Copy link
Contributor Author

Luro02 commented Apr 7, 2020

How can an attribute get applied to a doc comment?

They are not supposed to be applied to a doc comment

Both are, under the hood, attributes, no? I would expect that to be equivalent to expecting #[cfg(foo)] #[derive(Debug)] struct Bar to only derive Debug if --cfg foo is passed, which is not how that works.

I do not think that. Your example would make sense for

#[cfg(foo)]
/// doc comment
struct Example;

which I also consider to be wrong, but I am talking about cfg_attr which should insert the attribute conditionally

#[cfg_attr(foo, derive(Debug))]
struct Bar;

but that is not the point.


It might help to illustrate my thought process;

We have some kind of documented item:

/// ```rust
/// let example = Example::new()
///     .first()
///     .build();
/// ```
struct Example;

and each of those /// is turned into a doc attribute

That is, they are equivalent to writing #[doc="..."] around the body of the comment, i.e., /// Foo turns into #[doc="Foo"] and /** Bar */ turns into #[doc="Bar"].

https://doc.rust-lang.org/reference/comments.html#doc-comments

#[doc = "```rust"]
#[doc = "let example = Example::new()"]
#[doc = "    .first()"]
#[doc = "    .build();"]
#[doc = "```"]
struct Example;

In a later step those attributes would then be concatenated and parsed as markdown or whatever.


I want to use this behavior to document items conditionally, depending on which features the crate user has enabled.

One way to do this would be to do

#[cfg_attr(
    not(feature = "one"),
    doc = r#"```rust
let example = Example::new()
    .first()
    .build();
```"#
)]
#[cfg_attr(
    feature = "one",
    doc = r#"```rust
let example = Example::new()
    .first()
    .second()
    .build();
```"#
)]
struct Example;

but that would blow up the source code with many duplicate lines, where only some minor things were changed.

So I thought that it might make more sense to conditionally include a doc comment like this:

/// ```rust
/// let example = Example::new()
///     .first()
#[cfg_attr(feature = "one", doc = "     .second()")]
///     .build();
/// ```
struct Example;

which would turn into

#[doc = "```rust"]
#[doc = "let example = Example::new()"]
#[doc = "    .first()"]
#[doc = "    .second()"]
#[doc = "    .build();"]
#[doc = "```"]
struct Example;

or

#[doc = "```rust"]
#[doc = "let example = Example::new()"]
#[doc = "    .first()"]
#[doc = "    .build();"]
#[doc = "```"]
struct Example;

depending on whether the feature is enabled. So the attribute should not be inserted and not applied to the following doc comment.

The weird thing is that the manually expanded version is working as expected.

#[doc = "```rust"]
#[doc = "let example = Example::new()"]
#[doc = "    .first()"]
#[cfg_attr(feature = "one", doc = "    .second()")]
#[doc = "    .build();"]
#[doc = "```"]
pub struct WorkingExample;

but this is very unhandy to work with :/

@Nemo157
Copy link
Member

Nemo157 commented Apr 7, 2020

I was randomly looking at passes for another reason and noticed what is likely the source of this issue. There are two relevant passes:

  1. collapse-docs runs first and merges together neighboring doc attributes, but it distinguishes whether the doc was a SugaredDoc (///) or RawDoc (#[doc]) and only merges the same kind together.

  2. unindent-comments runs second and removes leading columns of whitespace from the docs. This runs on a single "doc fragment" at a time, because of merging that normally means the entire documentation string, but since sugared and raw docs aren't merged they remain as separate fragments and use their own widths of leading whitespace to strip.

@Nemo157
Copy link
Member

Nemo157 commented Apr 7, 2020

That also matches up with #44781 first released in 1.24 which introduced this distinction between doc kinds in order to fix #42760, which IMO was user error in not matching the indent depth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants