Revisiting spotlight / "notable traits" in rustdoc

[jsha]

Rustdoc has a "notable traits" tooltip when hovering the (i) icon next to some return values. It currently looks like this:

It was implemented in 2017, in response to a 2015 issue requesting more clarity about iterator returns. @GuillaumeGomez removed it in early 2020 for being too complex and not doing its job. @Manishearth re-added it in mid-2020 after a useful forum thread here.

The current implementation still doesn't do its job because the (i) is so unobtrusive. Most people don't notice it, as @GuillaumeGomez' Twitter poll suggested. Also, even if you notice the (i), the text in the box is inscrutable to a beginner (and still hard to eyeball for an experienced Rustacean).

The current version considers Iterator, Read, Write, and Future to be notable. Note that the original issue requested a way to show the iteration type of a returned iterator - it didn't request a generic way to show notable traits. And most of the discussion in @Manishearth's forum thread centered around iterators. I haven't seen someone make the case for Read, Write, and Future definitely needing this treatment.

I think we can do a lot better by specializing to just Iterator, and displaying some text inline rather than hiding it behind a tooltip:

Thoughts?

[steffahn]

I think it's very reasonable to also consider Future. This makes reading the docs for e. g. traits like AsyncReadExt a lot easier.

[scottmcm]

Spitballing: This is a bit of an abuse of notation, but maybe

-> IterMut<'_, K, V, Item = (&K, &mut V)>
-> Read<'a, Self, Output = Result<usize, Error>>

Alternatively, Pre-RFC: Absolute minimum for in-impl imports has me pondering inherent trait impls as a potential definition for "notable", though of course that wouldn't be helpful to anyone for a while.

[SkiFire13]

@jsha

This looks nice, but I would change the works. Instead of "an Iterator of &T", which doesn't work for all types, I would use something like "impl Iterator<Item = &'a T>", possibly using the same 'a and T as the impl block/fn generics that are used.

For example if we had:

impl<'a, T> Iterator for Iter<'a, T> {
    type Item = &'a T;
    // ...
}

impl<U> [U] {
    pub fn iter(&self) -> Iter<'_, U> { /* ... */ }
}

Then in the docs, on the right of the function signature, there would be written "impl Iterator<Item = &'_ U>".

Possible downsides:

• This may create confusion with lifetimes, in particular elided lifetimes
• If the type of Item (or other types present in the trait implementation) are pretty long this could easily get out of control.
• How do you show multiple notable traits? Think for example of types that implement both Read and BufRead, or Iterator and FusedIterator

It would also be nice to have a way to restrict/select notable traits. For example in the docs of Iterator::by_ref I don't care that the returned reference may implement Read, I only care about Iterator.

[CAD97]

The point of the OP suggestion is that (i) is trying to serve the general case, and failing at even just (effectively) serving the basic case of just Iterator. So perhaps we would be better off with a specific case for Iterator rather than trying to have a general system that serves all cases.

At a minimum, we definitely can tone down the notable traits from "it could implement it if the input type does" because that's not super useful. Instead, restrict it to the one that's guaranteed to hold.

I do think this is similar to inherent trait impls, but distinct. The whole point of the notable traits feature is that some types exist primarily to be used through the trait -- so the marker should live on the impl, not the trait!

Instead of #[doc(spotlight)] trait Iterator, give us just #[doc(spotlight)] impl Iterator for Iter.

The standard argument against this is that ~most implementors of the notable trait would want to spotlight it. I think this hasn't really held up in practice: we both over spotlight every potential notable trait in generic situations and under spotlight in that it's hidden in the (i).

By putting the annotation on the impl, you gain the communication of RPIT with the control of a named type. And I think treating it similarly to RPIT in the docs is the ideal case.

The reasoning for sticking the spotlight behind the (i) is that it claims to much space otherwise. Making it granularly opt in puts more effort on the library author for documenting well, but because it's an active decision to spotlight it, it also allows us to spend more space on it, because we've been told it's important.

I don't have any useful input design wise, but content wise, I think this is the best way of thinking about it.

[GuillaumeGomez]

I think simplifying it is a good idea. As long as we keep a link on the various types in the spotlight display, all good for me!

[jsha]

@CAD97

The point of the OP suggestion is that (i) is trying to serve the general case, and failing at even just (effectively) serving the basic case of just Iterator . So perhaps we would be better off with a specific case for Iterator rather than trying to have a general system that serves all cases.

Exactly. Great summary, thanks.

@CAD97

By putting the annotation on the impl, you gain the communication of RPIT with the control of a named type. And I think treating it similarly to RPIT in the docs is the ideal case.

I had to search for RPIT in the source, so for anyone reading who needs the acronym expanded: Return Position Impl Trait.

@steffahn

I think it's very reasonable to also consider Future . This makes reading the docs for e. g. traits like AsyncReadExt a lot easier.

Good point. The main reasons I want to specialize to Iterator are (a) we don't have to deal with spotlighting multiple traits, and (b) there's a single, well-known output type. Since it's quite unusual for a type to implement both Iterator and Future, we could say "if both of these, one of them wins."

Current:

Possible future:

[matklad]

Somewhat related, in rust-analyzer, we already have some logic to "erase" some concrete iterator types to an impl Trait syntax.

We use a rather ham-handed approach to deciding whether we should do the erasure:

matklad/rust-analyzer/blob/7283163bb9b4314ec3c7b25fad90ae794e7b7320/crates/ide/src/inlay_hints.rs#L229-L231

[Manishearth]

I disagree that just specializing to Iterator is a good idea. The full impls are really really useful because the precise types involved often become important, as well as the trait bounds.

I do think the proposed UI is nice and useful but I don't think it should replace the info tooltip, I think it should supplement it: in cases where there is a straightforward answer, we should show the straightforward answer and stick the (i) inside the parentheses so it's clear they're linked.

My guess is that there will be cases where we can still show an (i) but there is no straightforward answer as to what to show in the grey text portion. Or cases where there is but we need to incrementally approach it: When things like associated types of type parameters get involved for example, implementation-wise collapsing it into a readable Iterator of X is going to be tricky.

I also do think the Read, Write, and Future impls are important, and it's worth highlighting that these three traits are used by subsets of the community. Most rust code uses iterators, but only a subset of rust code really cares about this, which is why these three being notable seems less useful. It is not necessary that a feature serve everyone, as long as it's not overly degrading the experience for those who don't need it.

(This logic is also why I'm against replacing this UI: I find the amount of information exposed by the existing UI incredibly useful, I need it on a regular basis, and just because it's not useful to everyone doesn't mean it should be removed, but perhaps supplemented)

The current implementation still doesn't do its job because the (i) is so unobtrusive. Most people don't notice it, as @GuillaumeGomez' Twitter poll suggested

I don't see any evidence for this? The twitter poll is for the older UI.

[dhm]

While I like the idea, I'm not fond of it being non-Rust syntax intermingled with Rust one (I am referring to "an Iterator of ..."). Remember that CSS styling may not be available to all rustdoc users due to accessibility reasons, so one ought to avoid relying too much on that.

I personally think that the nicer syntax for this is to have rustdoc emit trivially true where clause bounds:

pub fn iter(&self) -> Iter<'_, T>
where
                      Iter<'_, T> : Iterator<Item = &'_ T>,
                      Iter<'_, T> : Read, // let's imagine multiple highlighted traits
    /* original where clauses, if any */

Or, if we really wanted to stretch things, a shorthand syntax like:

pub fn iter(&self) -> Iter<'_, T> : Iterator<Item = &'_ T>
                                  + Read

(although the latter doesn't generalize as well for bounds "iterable"-like,
such as &'_ ReturnType : IntoIterator<...>, but maybe that's not an issue )

---

In that regard, I don't like impl Iterator syntax if the original code did not have it, since the actual type, whether we like it or not, is meaningful, Rust-wise (e.g., replacing it with an impl Iterator is a breaking change), so if rustdoc made that replacement it would technically be lying.

---

Finally, only tangentially related, I find that signatures involving elided lifetimes can already be hard enough to "unelide"; and if trait bounds are to be added, then rustdoc ought to seriously consider adding a (potentially opt-in) mechanism to unelide lifetimes (which, incidentally, could help teaching Rust and making complex signatures more readable).

[Lonami]

Would a special section dedicated to listing the notable traits of the return value be too intrusive? In my opinion, it would be too much to put it in the same line as the fn definition.

Maybe it could be a <details> expandable tag with a <summary> of "Notable traits implemented by the return type". That should definitely be more easily noticed.

[Manishearth]

@dhm

While I like the idea, I'm not fond of it being non-Rust syntax intermingled with Rust one (I am referring to "an Iterator of ..."). Remember that CSS styling may not be available to all rustdoc users due to accessibility reasons, so one ought to avoid relying too much on that.

I actually think using Rust syntax here is fraught because it might be confused with actual Rust syntax. For example, those where bounds aren't actually real, and that seems pretty confusing. Similarly Iter: Iterator will give people the impression that such syntax works.

If you're worried about accessibility and CSS; HTML is semantic and these needs can be easily addressed by using the appropriate tags or ARIA categories.

[Aloso]

@dhm

Remember that CSS styling may not be available to all rustdoc users due to accessibility reasons

Not an expert, but I think adding the proper aria attributes to the element would help.

I think expressing the notable traits in a way that looks like Rust syntax would be confusing, because people might assume that they are part of the original code, and that the notable traits are trait bounds that the caller of the function must satisfy. Other people might get stumped why rustdoc only adds some traits and not others.

If the notable traits are instead written in prose ("an iterator of ..."), I think it's clear enough that this is not Rust syntax, and it can be made more obvious by using a smaller, proportional font. I don't think there's any reason why the documentation must be valid Rust syntax.

[skysch]

@Aloso

I don't think there's any reason why the documentation must be valid Rust syntax.

Because people will copy & paste it into their code. Especially if it is some complicated function signature you have to implement for a trait.

[jsha]

@Manishearth

@jsha

The current implementation still doesn't do its job because the (i) is so unobtrusive. Most people don't notice it, as @GuillaumeGomez' Twitter poll suggested

I don't see any evidence for this? The twitter poll is for the older UI.

I stand corrected, thanks for pointing this out. So I fall back for evidence on my personal experience: I did not notice the (i) until its embedded <div> showed up in a tidy run. And also on a general feeling that if the information is very important we should dedicate some space to showing it by default; and if not we should rely on clicking through (and ensuring that the clicked-to page loads very fast, and the link takes you directly to the most important aspect of the target).

@Manishearth

I also do think the Read , Write , and Future impls are important, and it's worth highlighting that these three traits are used by subsets of the community.

Good points. I see why Future needs the same treatment. Can you tell me more about the subsets of the community that need spotlight treatment for Read and Write? Are there cases where a return type implements multiple of these four traits, and they all need to be spotlighted?

@skysch

people will copy & paste it into their code. Especially if it is some complicated function signature you have to implement for a trait.

To solve this, we can make that part of the function signature not participate in selection: user-select - CSS: Cascading Style Sheets | MDN.

[Manishearth]

@jsha

And also on a general feeling that if the information is very important we should dedicate some space to showing it by default; and if not we should rely on clicking through (and ensuring that the clicked-to page loads very fast, and the link takes you directly to the most important aspect of the target).

Is it "very important"? It's very useful for people working with complex types, but ... that's not everyone! And that's okay!

Clicking through is not a solution at all, often you need to get this information in a flash for multiple methods and having to context switch a bunch makes it harder to retain it. Especially since clicking through gives you a lot more information and you need to pinpoint the stuff you need.

I do think that showing some cases by default makes sense, but I'm very much against the "either show it by default or not at all" dichotomy.

@jsha

Good points. I see why Future needs the same treatment. Can you tell me more about the subsets of the community that need spotlight treatment for Read and Write ? Are there cases where a return type implements multiple of these four traits, and they all need to be spotlighted?

Mostly when you're doing IO stuff.

@jsha

So I fall back for evidence on my personal experience: I did not notice the (i) until its embedded <div> showed up in a tidy run

I think the (i) being unobtrusive is fine; this is more of a power user feature, not everything needs to be immediately obvious to users. I don't mind making it more noticeable or by showing "(an iterator of T)" in some cases. But again, I don't think that's justification for replacing the feature unless the new feature can satisfy the same use cases.

---

So one concern I have is that (i) right now shows a lot of stuff and it's not always obvious (programmatically) which is the most important entry there. I think that all the entries are important to show, but one of them is the most obvious and that's what would feed into the show-by-default UI being proposed here.

Perhaps we can have an attribute for methods to opt in to this behavior, which can identify the "most important" trait. The downside is that non-stdlib crates do not get this for free, like they do for (i) right now.

[Manishearth]

Fundamentally, I think there might be two sets of use cases involved here: the simpler case where you're just calling a function and want to know what you should do with the thing you got back ("iterate it" / "await it" / "read it" / etc), and the more complicated case where all of your types are abstract and you need to be able to reason about the exact type you're getting back (which is where knowing the full impl bounds / where clauses comes up).

The latter is the one that comes up more often for me and for other folks who write code heavy with type parameters, but a lot of people in this thread seem to be focused on the former. The former is important too, but I basically want to be really clear that the feature not effectively solving the former use case does not mean that it isn't effectively solving a use case.

[jsha]

Can you link to some crates or types that you're talking about so I can better understand the use case? And when you say "reason about the exact type you're getting back," what are some examples of the types of reasoning you're doing?

[Manishearth]

I haven't written such code in a while so I'm afraid I can't think of crates immediately.

The types of reasoning are basically around "what is the final value of the Item associated type after being chained through a lot of iterators" for example.