rustdoc: disambiguate between types of the same name

5758dd5
Opened by QuietMisdreavus at 2024-08-20 13:32:41

As an extension to #38414 and #39589, any hyperlinked reference to some type should be checked against everything within that crate - within any dependent crate too, if possible. This will help clear the confusion between the perennial problem types in std, std::result::Result and std::io::Result (and to a lesser extent, std::io::Write and std::fmt::Write).

Some before/after screenshots made with chrome's inspector with what I have in mind:

Write for String

File::create -> Result

  1. Related to #34999.

    Oliver Middleton at 2017-05-17 20:36:59

  2. I just wonder: should we add the path only when the item is present more than once or should we do it everytime?

    Guillaume Gomez at 2017-05-18 15:24:22

  3. As in, print the full path regardless of whether it shares its short-name with anything? I suppose that could work. It'd certainly be easier to implement, though it risks being overly verbose.

    QuietMisdreavus at 2017-05-18 15:42:56

  4. That's my main concern.

    Guillaume Gomez at 2017-05-19 08:22:04

  5. Looks like this is a more general version of #16096.

    QuietMisdreavus at 2017-07-29 18:49:05

  6. Always printing the full path would make the docs vastly more useful for me. Even if a name isn't reused, it's still important to see where it's actually coming from.

    But for reused names this is really important, since re-using the same name in many related crates is common practice. (E.g. today I was trying to figure something out with bincode, and ended up on https://docs.rs/bincode/1.0.0-alpha7/bincode/type.Serializer.html. The doc reads:

    type Serializer<W> = Serializer<W, LittleEndian>;

    This is not useful at all. Clicking on the last Serializer takes one to bincode::endian_choice::Serializer, which is not correct; it's actually referring to serde::Serializer. Even if the link was correct, though, this line is just meaningless without knowing what the names resolve to.

    Jeff Petkau at 2018-02-09 19:42:04

  7. @jyn514 mentioned it might be worth posting my OP at #65330 (a duplicate issue) to the main discussion here, but it looks like a lot of my points have already been covered here so I'll just leave it at a link.

    should we add the path only when the item is present more than once or should we do it everytime?

    My personal preference would be to prepend the parent modules until there can no longer be any ambiguity to which type is referred to. The paths to some types can get incredibly long. If the full path didn't help to disambiguate the type, I imagine it might just appear a bit noisy.

    From #65330,

    Keeping in mind I am unfamiliar with rustdoc internals, I wonder if this can be achieved by keeping a map from item names to whether or not they occur more than once during the documentation process. For all items that occur more than once in the crate or its dependency graph, their parent modules are prefixed to the item name until the ambiguity is resolved?

    mitchmindtree at 2020-08-27 10:26:59

  8. keeping a map from item names to whether or not they occur more than once during the documentation process

    I think it should be per-page, not for the whole crate. Otherwise you'd get lots of false positives from dependencies when you really are interested in disambiguating the current page.

    jyn at 2020-08-27 11:49:40

  9. I think it should be per-page

    Can you elaborate on what you mean by per-page?

    Personally, I'm interested in disambiguating between all of the types with the same name within the current crate and all its dependencies - not just potential name conflicts that appear on the current page.

    When reading docs, I'd like to be able to know which of the possible set of types is being referred to (of all public types that share the same name within this crate and its dependencies) without having to click on the type and wait for a page to load or hover over it and wait for a hint to appear to work it out.

    mitchmindtree at 2020-08-27 12:18:44

  10. So, obviously this is very confusing and should be fixed (from https://github.com/rust-lang/rust/issues/74924, https://docs.serde.rs/serde_json/struct.Error.html#trait-implementations): image

    But what about impl std::error::Error for MyError, when there's no other Error type in scope? https://docs.rs/hexponent/0.3.1/hexponent/struct.ParseError.html image

    If we always disambiguated when there was a duplicate name anywhere in the dependency tree, rustdoc would always use std::result::Result and std::error::Error, since they conflict with std::io::Result and std::io::Error. That seems like clutter to me.

    jyn at 2020-08-27 13:20:34

  11. But what about impl std::error::Error for MyError, when there's no other Error type in scope?

    If the serde crate is a dependency of hexponent, then I would still opt to disambiguate the trait name. I would do so because it is still just as unclear which trait is being implemented for ParseError.

    E.g.

    impl Error for ParseError

    Knowing that serde is a dependency, it is unclear which Error trait is being implemented without using the mouse to hover over the Error name or click on it.

    If we always disambiguated when there was a duplicate name anywhere in the dependency tree, rustdoc would always use std::result::Result and std::error::Error, since they conflict with std::io::Result and std::io::Error.

    I would love this! I think for beginners especially it's easy to get confused between result::Result, io::Result and fmt::Result.

    All that said, I'm not sure how this would affect the std library more generally, as both core and std has their own public "instances" of a majority of the types and traits. Maybe public re-exports under the same name should be omitted from disambiguation?

    mitchmindtree at 2020-08-27 14:49:13

  12. Here's an example of total failure with ambiguous names (link is not to docs.rs because the identifier is not present in latest release):

    https://docs.iced.rs/iced_renderer/type.Compositor.html (version 0.13.0-dev)

    pub type Compositor = Compositor<Compositor, Compositor>;

    You have to hover the links to make any sense of that.

    Tv at 2024-07-10 21:23:19

  13. There is a work in progress on that. I should check what's the current status... cc @krtab

    Guillaume Gomez at 2024-07-11 09:03:55

  14. Hi everyone, Sorry, I'm back from holidays. Unfortunately I can't really find time to dedicate to #122872 as of now. If somebody is more available and wants to move things forward, feel free to. Otherwise, I may get back to it in the future but can't commit on any timeline. (cc @Dylan-DPC )

    Arthur Carcano at 2024-08-20 13:32:41