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

rustdoc: Inconsistant decision to include impl with docs but no visible items #111564

Open
aDotInTheVoid opened this issue May 14, 2023 · 1 comment
Labels
T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@aDotInTheVoid
Copy link
Member

Since #90905, rustdoc has impl blocks with docs but no items: But it's logic doesn't make (user facing) sense

pub struct Foo;

/// 1. Empty
impl Foo {}

/// 2. Private
impl Foo {
    fn private() {}
}

/// 3. Public
impl Foo {
    pub fn public() {}
}

/// 4. Public Hidden
impl Foo {
    #[doc(hidden)]
    fn public_hidden() {}
}

/// 5. Private Hidden
impl Foo {
    #[doc(hidden)]
    fn priviate_hidden() {}
}

Running this code across various flag's gets us:

Header --document-private-items --document-hidden-items --document-private-items --document-hidden-items
Empty ✔️ ✔️ ✔️ ✔️
Private ✔️ ✔️ ✔️
Public ✔️ ✔️ ✔️ ✔️
Public Hidden ✔️ ✔️ ✔️ ✔️
Private Hidden ✔️ ✔️ ✔️ ✔️

It doesn't make sense that adding #[doc(hidden)] to a private function makes it's block visible, or that running --document-hidden-items makes a block with only private functions visible. I'm not sure what the exact behavior should be here

@aDotInTheVoid aDotInTheVoid added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label May 14, 2023
@GuillaumeGomez
Copy link
Member

In the code, the comment says:

Impl blocks can be skipped if they are: empty; not a trait impl; and have no
documentation.

There is one special case: if the impl block contains only private items.

The logic makes sense to me: impl blocks with only private items should very likely not be visible to readers. The question here I think should be more "should we do the same with impls containing only hidden items?". What do you think?

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 17, 2023
…laumeGomez

rustdoc-json: Add tests for visibility of impls

[Apparrently rustdoc use to give these `crate` instead of `default`](obi1kenobi/trustfall-rustdoc-adapter@rustdoc-v24...rustdoc-v25#diff-58e57a0fc73d1353fa3a057f0fe81c6ecfd4548b429cef1aee36b1c84d8d15a4L366). CC `@obi1kenobi`

The output is arguably still buggy as to weather some of these impls should be stripped, but that's a seperate issue and shouldn't block adding these tests (rust-lang#111564)

r? `@GuillaumeGomez`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants