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: Methods on type aliases are not documented if the target type is not publicly reachable #119015

Closed
sgued opened this issue Dec 16, 2023 · 1 comment · Fixed by #119997
Assignees
Labels
C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@sgued
Copy link
Contributor

sgued commented Dec 16, 2023

I tried this code:

mod foo {
    pub struct S;
}

use foo::S;

pub type A = S;

impl Default for A {
    fn default() -> Self {
        S
    }
}

impl A {
    pub fn a(&self) {}
}

I expected the documentation for the type alias A to show the method fn a(&self) and the implementation of Default.

Instead, the type alias does not show the method fn a(&self). It does however show the implementation of Default.

image

Workarounds

By publicly exposing foo::S through pub use foo::S, the documentation of fn a(&self) on the alias is documented as I expected, but then foo:S is publicly reachable, which in my case is unwanted due to being an internal implementation detail.

This also works when making the use statement #[doc(hidden)], which gets the docs to look like I expected them to.

mod foo {
    pub struct S;
}

#[doc(hidden)]
pub use foo::S;

pub type A = S;

impl Default for A {
    fn default() -> Self {
        S
    }
}

impl A {
    pub fn a(&self) {}
}

image

Meta

rustc --version --verbose:

cargo 1.74.1 (ecb9851af 2023-10-18)
release: 1.74.1
commit-hash: ecb9851afd3095e988daaa35a48bc7f3cb748e04
commit-date: 2023-10-18
host: x86_64-unknown-linux-gnu
libgit2: 1.7.1 (sys:0.18.0 vendored)
libcurl: 8.4.0-DEV (sys:0.4.68+curl-8.4.0 vendored ssl:OpenSSL/1.1.1u)
ssl: OpenSSL 1.1.1u  30 May 2023
os: Arch Linux [64-bit]

Thanks @GuillaumeGomez for helping me figure out why what I was expecting wasn't happening though it looked like it should.

@sgued sgued added the C-bug Category: This is a bug. label Dec 16, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 16, 2023
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Dec 16, 2023

I think it's ok to document an impl on a reexported if the reexported item is not doc hidden. I'll send a fix in the next weeks.

@GuillaumeGomez GuillaumeGomez self-assigned this Dec 16, 2023
@GuillaumeGomez GuillaumeGomez added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Dec 16, 2023
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jan 15, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jan 20, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jan 20, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jan 20, 2024
…n-ty-alias, r=notriddle

Fix impl stripped in rustdoc HTML whereas it should not be in case the impl is implemented on a type alias

Fixes rust-lang#119015.

I talked about it a bit with `@petrochenkov.` They might change what `EffectiveVisibilities` return for impl items like this one and make them not only reachable but also re-exported, which would fix this case. It could also potentially break other things, so it'll be done whenever they can and then we can check together.

Surprisingly, this fix is making rustdoc even closer to rustc in term of errors (the CI currently fails because currently accepted broken codes aren't working anymore with this change). Not sure exactly why though. This is linked to rust-lang#110631 from what I could find.

So either I'm missing something here, or we consider it's ok and we consider the failing tests as "should fail" and I'll update `rustdoc-ui` ones.

r? `@notriddle`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jan 20, 2024
…n-ty-alias, r=notriddle

Fix impl stripped in rustdoc HTML whereas it should not be in case the impl is implemented on a type alias

Fixes rust-lang#119015.

I talked about it a bit with ``@petrochenkov.`` They might change what `EffectiveVisibilities` return for impl items like this one and make them not only reachable but also re-exported, which would fix this case. It could also potentially break other things, so it'll be done whenever they can and then we can check together.

Surprisingly, this fix is making rustdoc even closer to rustc in term of errors (the CI currently fails because currently accepted broken codes aren't working anymore with this change). Not sure exactly why though. This is linked to rust-lang#110631 from what I could find.

So either I'm missing something here, or we consider it's ok and we consider the failing tests as "should fail" and I'll update `rustdoc-ui` ones.

r? ``@notriddle``
@bors bors closed this as completed in 1236fa2 Jan 20, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 20, 2024
Rollup merge of rust-lang#119997 - GuillaumeGomez:fix-stripped-impl-on-ty-alias, r=notriddle

Fix impl stripped in rustdoc HTML whereas it should not be in case the impl is implemented on a type alias

Fixes rust-lang#119015.

I talked about it a bit with ```@petrochenkov.``` They might change what `EffectiveVisibilities` return for impl items like this one and make them not only reachable but also re-exported, which would fix this case. It could also potentially break other things, so it'll be done whenever they can and then we can check together.

Surprisingly, this fix is making rustdoc even closer to rustc in term of errors (the CI currently fails because currently accepted broken codes aren't working anymore with this change). Not sure exactly why though. This is linked to rust-lang#110631 from what I could find.

So either I'm missing something here, or we consider it's ok and we consider the failing tests as "should fail" and I'll update `rustdoc-ui` ones.

r? ```@notriddle```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
3 participants