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: disambiguate between types of the same name #42066

Open
QuietMisdreavus opened this issue May 17, 2017 · 14 comments
Open

rustdoc: disambiguate between types of the same name #42066

QuietMisdreavus opened this issue May 17, 2017 · 14 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@QuietMisdreavus
Copy link
Member

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

@frewsxcv frewsxcv added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. labels May 17, 2017
@ollie27
Copy link
Member

ollie27 commented May 17, 2017

Related to #34999.

@GuillaumeGomez
Copy link
Member

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

@QuietMisdreavus
Copy link
Member Author

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.

@GuillaumeGomez
Copy link
Member

That's my main concern.

@QuietMisdreavus QuietMisdreavus self-assigned this Jun 22, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 27, 2017
@QuietMisdreavus
Copy link
Member Author

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

@jpetkau
Copy link

jpetkau commented Feb 9, 2018

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 = 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.

@mitchmindtree
Copy link
Contributor

@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?

@jyn514
Copy link
Member

jyn514 commented Aug 27, 2020

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.

@mitchmindtree
Copy link
Contributor

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.

@jyn514
Copy link
Member

jyn514 commented Aug 27, 2020

So, obviously this is very confusing and should be fixed (from #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.

@mitchmindtree
Copy link
Contributor

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?

@tv42
Copy link
Contributor

tv42 commented Jul 10, 2024

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.

@GuillaumeGomez
Copy link
Member

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

@krtab
Copy link
Contributor

krtab commented Aug 20, 2024

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 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. 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