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

Intra-doc links are not resolved in inherited docs from upstream traits #73829

Closed
euclio opened this issue Jun 28, 2020 · 11 comments · Fixed by #75176
Closed

Intra-doc links are not resolved in inherited docs from upstream traits #73829

euclio opened this issue Jun 28, 2020 · 11 comments · Fixed by #75176
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@euclio
Copy link
Contributor

euclio commented Jun 28, 2020

I tried this code:

There are two crates, outer and inner. outer depends on inner.

outer/src/lib.rs:

use inner::SomeTrait;

pub struct SomeStruct;

impl SomeTrait for SomeStruct {
    fn foo() {}
}

outer/inner/src/lib.rs:

/// this is a trait
pub trait SomeTrait {
    /// this is a method for [SomeTrait]
    fn foo();
}

pub mod bar {
    use super::SomeTrait;

    pub struct BarStruct;

    impl SomeTrait for BarStruct {
        fn foo() {}
    }
}

I expected to see this happen: The trait implementation docs for SomeTrait::foo on outer::SomeStruct link to SomeTrait.

Instead, this happened: The link is not resolved, and there is no warning or indication that it failed. Here is a screenshot of the generated docs:

foo

Notably, the docs on inner::bar::BarStruct resolve correctly despite being in a different module, indicating that this is a cross-crate issue.

Meta

rustc --version --verbose:

rustc 1.44.1 (c7087fe00 2020-06-17)
binary: rustc
commit-hash: c7087fe00d2ba919df1d813c040a5d47e43b0fe7
commit-date: 2020-06-17
host: x86_64-unknown-linux-gnu
release: 1.44.1
LLVM version: 9.0
@euclio euclio added the C-bug Category: This is a bug. label Jun 28, 2020
@euclio
Copy link
Contributor Author

euclio commented Jun 28, 2020

cc @jyn514 who has been doing some work in this area recently.

@jonas-schievink jonas-schievink added A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jun 28, 2020
@jyn514
Copy link
Member

jyn514 commented Jun 28, 2020

I expect this to be fixed by #73101. I'll test locally.

@jyn514
Copy link
Member

jyn514 commented Jun 28, 2020

Interesting, it was not fixed - rustdoc resolves links for associated items relative to the type, not relative to the trait. The lack of warning is definitely a bug, but I'm not sure the behavior is incorrect.

ImplItem(Impl { ref for_, .. }) => {

@jyn514
Copy link
Member

jyn514 commented Jun 28, 2020

Oh hold on that shouldn't make a difference - the trait is in scope in both places. I have to look at this more, I'm not sure what's going on.

@euclio
Copy link
Contributor Author

euclio commented Jun 28, 2020

It's worth mentioning that these impl method docs are subtly broken in other ways (see my linked PR above) but as long as you use inline links you should be able to avoid those bugs.

@jyn514
Copy link
Member

jyn514 commented Jul 10, 2020

Reduced test case:

// inner.rs

/// [SomeType] links to [bar]
pub struct SomeType;
pub trait SomeTrait {}
/// [bar] links to [SomeTrait] and also [SomeType]
pub mod bar {}
// outer.rs
pub use inner::bar;

All the links on the documentation for outer::bar will be broken (even with the changes from #73101).

@jyn514
Copy link
Member

jyn514 commented Jul 10, 2020

This code looks wrong:

let item_hir_id = if item.is_mod() {
if let Some(def_id) = item.def_id.as_local() {
Some(self.cx.tcx.hir().as_local_hir_id(def_id))
} else {
debug!("attempting to fold on a non-local item: {:?}", item);
return self.fold_item_recur(item);

Not sure if it's related to the impl issue though.

@jyn514
Copy link
Member

jyn514 commented Jul 11, 2020

I have a fix for this in #73101. Thanks for the bug report!

@jyn514
Copy link
Member

jyn514 commented Jul 15, 2020

Update so all the discussion is in one place: My fix didn't do what I thought (it only worked for modules, not docs taken from the trait parent). However it will depend on the changes in #73101, so I'm leaving this be for now until that lands.

@jyn514
Copy link
Member

jyn514 commented Aug 5, 2020

... I think I found why this is broken. Oops.

// FIXME: if we can resolve intra-doc links from other crates, we can use the stock
// `fold_crate`, but until then we should avoid scanning `krate.external_traits` since those
// will never resolve properly
fn fold_crate(&mut self, mut c: Crate) -> Crate {
c.module = c.module.take().and_then(|module| self.fold_item(module));
c
}

@jyn514
Copy link
Member

jyn514 commented Aug 5, 2020

Removing that override gives a BorrowMut error. Investigating now.

@bors bors closed this as completed in db534b3 Aug 30, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 3, 2020
…illaumeGomez

rustdoc: do not use plain summary for trait impls

Fixes rust-lang#38386.
Fixes rust-lang#48332.
Fixes rust-lang#49430.
Fixes rust-lang#62741.
Fixes rust-lang#73474.

Unfortunately this is not quite ready to go because the newly-working links trigger a bunch of linkcheck failures. The failures are tough to fix because the links are resolved relative to the implementor, which could be anywhere in the module hierarchy.

(In the current docs, these links end up rendering as uninterpreted markdown syntax, so I don't think these failures are any worse than the status quo. It might be acceptable to just add them to the linkchecker whitelist.)

Ideally this could be fixed with intra-doc links ~~but it isn't working for me: I am currently investigating if it's possible to solve it this way.~~ Opened rust-lang#73829.

EDIT: This is now ready!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants