Fix intra-doc links for associated items

[jyn514]

@Manishearth and I found that links of the following sort are broken:

$ cat str_from.rs
/// [`String::from`]
pub fn foo() {}
$ rustdoc str_from.rs
warning: `[String::from]` cannot be resolved, ignoring it.
 --> str_from.rs:4:6
  |
4 | /// [`String::from`]
  |      ^^^^^^^^^^^^^^ cannot be resolved, ignoring

It turns out this is because the current implementation only looks at inherent impls (impl Bar {}) and traits for the item being documented. Note that this is not the same as the item being linked to. So this code would work:

pub trait T1 {
    fn method();
}

pub struct S;
impl T1 for S {
    /// [S::method] on method
    fn method() {}
}

but putting the documentation on trait T1 would not.

I realized that writing it up that my fix is only partially correct: It removes the inherent impls code when it should instead remove the trait_item code. Fixed.

Additionally, I discovered while writing this there is some ambiguity: you could have multiple methods with the same name, but for different traits:

pub trait T1 {
    fn method();
}

pub trait T2 {
    fn method();
}

/// See [S::method]
pub struct S;

Rustdoc should give an ambiguity error here, but since there is currently no way to disambiguate the traits (#74563) it does not (#74489 (comment)).

There is a third ambiguity that pops up: What if the trait is generic and is implemented multiple times with different generic parameters? In this case, my fix does not do very well: it thinks there is only one trait instantiated and links to that trait:

/// [`String::from`] -- this resolves to https://doc.rust-lang.org/nightly/alloc/string/struct.String.html#method.from
pub fn foo() {}

However, every From implementation has a method called from! So the browser picks a random one. This is not the desired behavior, but it's not clear how to avoid it.

To be consistent with the rest of intra-doc links, this only resolves associated items from traits that are in scope. This required changes to rustc_resolve to work cross-crate; the relevant commits are prefixed with resolve: . As a bonus, considering only traits in scope is slightly faster. To avoid re-calculating the traits over and over, rustdoc uses a cache to store the traits in scope for a given module.

[rust-highfive]

r? @steveklabnik

(rust_highfive has picked a reviewer for you, use r? to override)

[Manishearth]

r? @Manishearth

[jyn514]

I tried to split this up into commits as much as possible but it's definitely still a work in progress.

[jyn514]

I forgot that we already have a pass for this, collect_trait_impls. So I should be able to do this by looking up the clean::Item for the DefId. Not sure how hard that will be.

[Manishearth]

@jyn514 I don't think that will work because we won't have clean::Items for upstream crates

[jyn514]

@jyn514 I don't think that will work because we won't have clean::Items for upstream crates

@Manishearth yes you're right, I ended up using a different approach that used queries directly.

[jyn514]

Currently it's broken because it considers the same trait multiple times. Not yet sure what's going on. See https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/weird.20issues.20with.20finding.20trait.20implementations/near/204385714 for a little more info (and sorry for the enormous backlog there)

[Manishearth]

IMO we should not warn about ambiguity here until #74563 is fixed (which also involves fixing the anchors to not overlap)

[jyn514]

@Manishearth this is still a little messy but otherwise it's ready for review.

[Manishearth]

I like the approach but I think this has the potential to be really slow.

[jyn514]

@bors r=manishearth,petrochenkov

[bors]

📌 Commit d77eff2 has been approved by manishearth,petrochenkov

[bors]

⌛ Testing commit d77eff2 with merge 8fdce9b...

[bors]

☀️ Test successful - checks-actions, checks-azure
Approved by: manishearth,petrochenkov
Pushing 8fdce9b to master...