Rustdoc doesn't play well with use ... as _ syntax

[Veetaha]

I tried this code:

mod ext {
    pub trait Foo {
         fn foo();
    }
    
    pub trait Bar {
        fn bar();
    }
}

pub mod prelude {
    pub use crate::ext::Foo as _;
    pub use crate::ext::Bar as _;
}

I expected to see this happen: there should be a way to view all the items reexported anonymously in docs. Maybe even just expose them by original names in docs.

Instead, this happened:

The traits are visible in rustodoc as a single prelude::_

And when you click on any of _ links you see only this:

Looks like rustdoc writes the docs for the arbitrary item reexported with use ... as _;

Meta

rustdoc --version --verbose:

rustdoc --version --verbose
rustdoc 1.60.0 (7737e0b5c 2022-04-04)
binary: rustdoc
commit-hash: 7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c
commit-date: 2022-04-04
host: x86_64-unknown-linux-gnu
release: 1.60.0
LLVM version: 14.0.0

rustc --version --verbose

rustc 1.60.0 (7737e0b5c 2022-04-04)
binary: rustc
commit-hash: 7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c
commit-date: 2022-04-04
host: x86_64-unknown-linux-gnu
release: 1.60.0
LLVM version: 14.0.0

[GuillaumeGomez]

The problem is that we "inline" them. The solution could be to keep the original name (and it would be a link to the original location), but if the item is originally in a private module, then there is no link generated and we would end up with use Trait as Whatever where you cannot get information on the item at all (since we wouldn't generate a page for Whatever or for Trait).

I'm not really sure what would be the best course of action in here...

cc @rust-lang/rustdoc

[Nemo157]

Is there any usecase for an anonymous reexport of a privately scoped trait? Seems like an edge case we don't need to really support.

[GuillaumeGomez]

I think it's a good point. For the more general problem of the reexports, we can always come back to it later on. I'll send a PR to explicitly ignore anonymous reexports.

[Veetaha]

@Nemo157 @GuillaumeGomez

Let's not rush with not-supporting this. We use this pattern in our code extensively.

We use it to export extension traits. For example, we have a trait that provides extension traits to the standard library

mod option {
     pub trait OptionExt<T> { /* our methods to put on all Option types */ }
     
     impl<T> OptionExt<T> for Option<T> { /* */ }
}

pub mod prelude {
    pub use crate::option::OptionExt as _;
}

We do it this way to guarantee that the users of our extension traits crate will not be able to both:

• Provide an impl OptionExt<T> for SomeOtherType, i.e. make the trait sealed
• Don't expose the name of trait to have smaller API surface. We don't want the users to care about the trait name. They just have to use crate_name::prelude::* and that's it. This way we may change the name of the trait without a breaking change.

[Nemo157]

For making the trait sealed the standard pattern is to use trait OptionExt: Sealed { ... } with a marker sealed trait (since it's just a marker it not being documented isn't an issue).

The latter requirement seems like an anti-pattern to me. I blanket-ban all glob imports in my projects so that would make your library unusable in them.

[Veetaha]

For making the trait sealed the standard pattern is to use trait OptionExt: Sealed { ... } with a marker sealed trait (since it's just a marker it not being documented isn't an issue).

That's true, but it is not the main benefit of this pattern. The trait is sealed for free when we do this.

The latter requirement seems like an anti-pattern to me. I blanket-ban all glob imports in my projects so that would make your library unusable in them.

Clippy specifically allows configuring wildcard_imports lint with this (link):

warn-on-all-wildcard-imports: bool: Whether to allow certain wildcard imports (prelude, super in tests). (defaults to false)

So things like use crate_foo::prelude::* are allowed, and I don't see a problem with that if crate defines a consistent prelude targeted for wildcard import specifically. It's subjective whether you want to follow that pattern but it is a known convention used throughout the ecosystem. Let alone std contains a number of prelude modules:

• std::io::prelude
• std::os::unix::prelude
• std::os::wasi::prelude
• std::os::windows::prelude

Requiring that the extension trait is available only via prelude::* import makes developers use consistent import style.
Blelive me, there will always be a person who doesn't read or know about our extension traits convention and would import the extension trait symbol directly instead of a prelude::* import unless we restrict it this way

[Nemo157]

Clippy specifically allows configuring wildcard_imports lint with this

And wildcard_imports is allow-by-default anyway. I have just chosen to enable it, and I don't allow prelude imports, because I believe that results in easier to navigate code and all crates should be usable still.

Let alone std contains a number of prelude modules

These are only re-exports of items that have other public paths. I know there are other buggy crates that force using paths like use proptest::prelude::ProptestConfig;, but most are well-behaved and give their items canonical locations outside the preludes.

[Nemo157]

Anyway, one approach to a fix I can think of is for rustdoc to pretend that prelude::OptionExt exists and the use as _ is a reexport of it. I have no idea how to handle something like pub use crate::{foo::OptionExt as _, bar::OptionExt as _} though, which seems like a plausible setup.

[Veetaha]

@Nemo157 you make very good points, but prelude module isn't considered to be an outright discouraged pattern. It isn't known to be a single way to export your extension traits. We in our company might be slightly overly strict about that though, but I believe there may be other people doing the same 🤔.

rustdoc is not a critical component of the toolchain, so it doesn't have to support literally every legal use of the language. Nevertheless, I really think we should support this because the pattern I described isn't something indicative of a completely brainless crate design or unworkable code. So I suggest we resolve the question of how to implement this.

Ideal fix

Ideally, a reexported symbol should have the additional context that indicates that it is unreachable. That sounds like a big-effort feature for rustdoc. That context would then be used to render the reexport in a special way e.g. with a label or warning "unreachable symbol". Unfortunately, I have never looked at rustdoc source code before, so I can only theorize on the complexity of this.

Low-effort Fix

If we want to have a low-effort fix for this problem, we could just generate a name for the reexported symbol there prefixed with some easy-to-understand token that makes it obvious the symbol is unreachable.

Generating The Symbol Name Based On Trait Name

We could generate the name for the reexport like this for example __unreachable_symbol__{TraitName}. The problem with this is the case you mentioned with reexporting multiple symbols of the same name with this pattern and general potential for collisions with other legal symbols in the namespace. If it is possible to detect that several symbols collide under the same name this way, then we could add a number into the symbol name here, for example.

Generating The Symbol Name Based On Reexport Identity

The reexport can be identified by its position in the module tree. For example, we could take the file/line/column to generate the symbol name like this {file_name}_{line}_{column}. However, this is problematic due to macro expansions potentially messing this up. This is where my theoretical ground is weak. Maybe we could identify the reexport in a different way to support it occurring in macro expansions? Maybe we could use a randomly-generated ID here (hello non-determinism😞), I am not really sure. Maybe we could just not support it in macro expansions, but still why not?

[lolbinarycat]

triage: seems to be an exact duplicate of #92379