Re-exports of hidden items are inconsistent

[lolbinarycat]

example

If an item in a hidden module is reexported, it will be automatically inlined, but if the item itself is hidden (and in a non-hidden module), it will show as a regular reexport, with a link to a normally hidden page.

This also makes the item not show in the "all items" list, despite being publicly reachable.

[xizheyin]

Has anyone started this issue already? Maybe I could try it. @lolbinarycat

[lolbinarycat]

@xizheyin do you have any experience working on rustdoc? this is not a trivial issue.

[xizheyin]

I have previously used librustdoc to create a small tool,but I only utilized a part of theclean/module. I took a look at the rest of librustdoc's code and got a general understanding of its structure.I'm not sure if I underestimated the difficulty.

[lolbinarycat]

I imagine it's an issue with the pass that removes doc(hidden) items. It might require a bit of refactoring to solve this issue in a performant way.

[xizheyin]

Ok, I'll take a deeper look. If the bug isn't particularly urgent.

[xizheyin]

@rustbot claim

[xizheyin]

I have tried to inline hidden_self_reexport as the discussion. Is that right? @lolbinarycat

Update:
It conflicts with the existing logic. What would be a better solution to this issue? Hide them all?

[lolbinarycat]

@xizheyin i don't have the final say on what the "right" way to fix it is, but if it was up to me, all 4 functions would be inlined.

figuring out how to do this efficiently without introducing new bugs is the hard part.

[xizheyin]

@xizheyin i don't have the final say on what the "right" way to fix it is, but if it was up to me, all 4 functions would be inlined.

yeah,that is what i have done. But it may conflict with the existing testcase.

[lolbinarycat]

@xizheyin if you look inside the testcase that's failing, you'll see it link to issue #109449

if you follow the links from that issue, you'll find pr #112304

that pr contains a section explaining what the intended behavior is. from my reading it seems to indicate that hidden_self_reexport should actually be hidden, while the others are visible. it may be worth waiting to see what t-rustdoc thinks.

[lolbinarycat]

@GuillaumeGomez is my understanding of the intended behavior here correct? is this a regression?

[GuillaumeGomez]

The rules are described here.

In particular:

If an item has #[doc(hidden)], it won't be inlined (nor visible in the generated documentation):

[xizheyin]

So hidden_self_reexport shouldn't be visible?

[lolbinarycat]

@GuillaumeGomez please look at the linked example. hidden_self_reexport actually contains a link to hidden_self_reexport_inline. This defiantly seems like a bug, but I'm not sure what the intended behavior actually is.

[GuillaumeGomez]

Hum I think it's a case that we didn't check. The docs say it's not supposed to be visible, even with doc(inline). So if it's visible, then it's definitely a bug.

So yes, hidden_self_reexport shouldn't be visible.

[xizheyin]

OK, let me try to fix it.

[lolbinarycat]

@GuillaumeGomez Wait, hidden items shouldn't be visible, even when explicitly inlined? This isn't entirly clear from reading the docs, but I guess that's what's meant by "(because the re-export itself and its attributes are ignored)".

If that's the case, then shouldn't hidden_self_reexport_inline also be hidden?

[GuillaumeGomez]

Hum, the reexport itself is not doc hidden. So I'd tend to say yes but unclear.

[lolbinarycat]

Sounds like the docs also need some clarification.

[GuillaumeGomez]

Both are welcome.

[xizheyin]

After I finish the code, and test it the outputs is

failures:
    [rustdoc] tests/rustdoc/attributes-inlining-108281.rs
    [rustdoc] tests/rustdoc/file-creation-111249.rs
    [rustdoc] tests/rustdoc/inline-private-with-intermediate-doc-hidden.rs
    [rustdoc] tests/rustdoc/reexport-attr-merge.rs
    [rustdoc] tests/rustdoc/reexport-doc-hidden-inside-private.rs
    [rustdoc] tests/rustdoc/reexport-doc-hidden.rs
    [rustdoc] tests/rustdoc/reexport-trait-from-hidden-111064-2.rs
    [rustdoc] tests/rustdoc/reexport-hidden-macro.rs
    [rustdoc] tests/rustdoc/reexport-of-doc-hidden.rs
    [rustdoc] tests/rustdoc/redirect.rs

test result: FAILED. 723 passed; 10 failed; 4 ignored; 0 measured; 0 filtered out; finished in 25.91s

10 previous testcases failed. Here is an example:

// Regression test for <https://github.com/rust-lang/rust/issues/108281>.
// It ensures that the attributes on the first reexport are not duplicated.

#![crate_name = "foo"]

//@ has 'foo/index.html'

#[doc(hidden)]
pub fn bar() {}
mod sub {
    pub fn public() {}
}

//@ matches - '//dd' '^Displayed$'
/// Displayed
#[doc(inline)]
pub use crate::bar as Bar;
//@ matches - '//dd' '^Hello\sDisplayed$'
#[doc(inline)]
/// Hello
pub use crate::Bar as Bar2;

//@ matches - '//dd' '^Public$'
/// Public
pub use crate::sub::public as Public;

By the logic we discussed, pub use crate::bar as Bar; shouldn't be displayed. So do I need to modify these testcases?
@GuillaumeGomez @lolbinarycat

[GuillaumeGomez]

Seems like the logic is that doc(inline) allows doc(hidden) items. What a mess. Well, if it's the current behaviour we're supposed to keep it. Gonna send a PR to update the rustdoc book.

So back to your example, what's weird is that if you have a reexport of a reexport of a hidden item, it's inlined whereas it shouldn't.

[xizheyin]

I have two modification logics

1. hide an item that is not marked as doc(inline) and whose import source is doc(hidden), i.e. hidden_self_reexport
2. hide an item that is marked as doc(inline) but the source is doc(hidden), i.e. hidden_self_reexport_inline

When I use both logic 1 and 2, 10 testcases failed; when I use only logic 1, only 5 testcases failed.
Which logic should I use? Should I just use logic 1? @GuillaumeGomez

[xizheyin]

When I use only logic 1, it failed the testcases as below:

#[macro_export]
#[doc(hidden)]
macro_rules! foo {
    () => {};
}

//@ has 'foo/index.html'
//@ has - '//*[@id="reexport.Macro"]/code' 'pub use crate::foo as Macro;'
pub use crate::foo as Macro;
//@ has - '//*[@id="reexport.Macro2"]/code' 'pub use crate::foo as Macro2;'
pub use crate::foo as Macro2;
//@ has - '//*[@id="reexport.Boo"]/code' 'pub use crate::Bar as Boo;'
pub use crate::Bar as Boo;
//@ has - '//*[@id="reexport.Boo2"]/code' 'pub use crate::Bar as Boo2;'
pub use crate::Bar as Boo2;

[GuillaumeGomez]

It should be 1 normally. If you use doc(inline) on doc(hidden), you should still have pub use X as Y.