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: link to the source of the re-export if the original definition wasn't documented #81311

Open
camelid opened this issue Jan 23, 2021 · 19 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@camelid
Copy link
Member

camelid commented Jan 23, 2021

Type aliases don't seem to have [src] links currently; see for example rustc_data_structures::fx::FxHashMap:

image

@camelid camelid added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Jan 23, 2021
@jyn514
Copy link
Member

jyn514 commented Jan 24, 2021

I looked around in the source a bit - this should have been generated around

write_srclink(cx, item, buf, cache);
, so I expect the issue is that
fn src_href(&self, item: &clean::Item, cache: &Cache) -> Option<String> {
is returning None for some reason.

@jyn514 jyn514 added E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Jan 24, 2021
@LeSeulArtichaut LeSeulArtichaut self-assigned this Jan 24, 2021
@LeSeulArtichaut
Copy link
Contributor

LeSeulArtichaut commented Jan 24, 2021

Rustdoc successfully generates an [src] link for Bar in the following code:

pub struct Foo;
pub type Bar = Foo;

@LeSeulArtichaut
Copy link
Contributor

rustc_data_structures::fx::FxHashMap probably doesn't work because it's a reexport from rustc_hash which is not documented in the published nightly-rustc docs.

By contrast, FxIndexMap, which is defined directly in rustc_data_structures, has a [src] link.

@LeSeulArtichaut
Copy link
Contributor

LeSeulArtichaut commented Jan 24, 2021

The following does reproduce this issue:

a/lib.rs

pub struct Foo;
pub type Bar = Foo;

b/lib.rs

pub use a::{Foo, Bar};

Here, both Foo andBar will appear without an [src] link, so this is not an issue with type aliases but with the way external sources are handled.

What could be done instead is having an [src] pointing at the re-export instead of having no link, unsure if that sounds good, @camelid what do you think?

@jyn514
Copy link
Member

jyn514 commented Jan 24, 2021

Hmm, maybe rustdoc should fall back to showing the re-export if the original definition isn't available.

@camelid
Copy link
Member Author

camelid commented Jan 24, 2021

The following does reproduce this issue:

a/lib.rs

pub struct Foo;

pub type Bar = Foo;

b/lib.rs

pub use a::{Foo, Bar};

Here, both Foo andBar will appear without an [src] link, so this is not an issue with type aliases but with the way external sources are handled.

What could be done instead is having an [src] pointing at the re-export instead of having no link, unsure if that sounds good, @camelid what do you think?

Yeah, that seems good.

@camelid
Copy link
Member Author

camelid commented Feb 1, 2021

Here, both Foo andBar will appear without an [src] link, so this is not an issue with type aliases but with the way external sources are handled.

Hmm, I'm not sure if this is right. rustc_hir::def_id::CrateNum has a [src] link, but that's a re-export of rustc_span::def_id::CrateNum. The [src] link of the re-export points to the source of the dependency.

I'm not sure why it's working in this case but not your repro...

@LeSeulArtichaut
Copy link
Contributor

Hmm, I'm not sure if this is right. rustc_hir::def_id::CrateNum has a [src] link, but that's a re-export of rustc_span::def_id::CrateNum. The [src] link of the re-export points to the source of the dependency.

The difference is that here, rustc_span is documented. In my example, you need to document b with --no-deps to reproduce the issue.

@camelid
Copy link
Member Author

camelid commented Feb 1, 2021

Ah, I see what you mean 👍

@LeSeulArtichaut LeSeulArtichaut removed their assignment Mar 6, 2021
@jyn514 jyn514 changed the title rustdoc: Add [src] links for type aliases rustdoc: link to the source of the re-export if the original definition wasn't documented Apr 8, 2021
@jyn514
Copy link
Member

jyn514 commented Apr 8, 2021

If #41903 is implemented, it should also fix this issue.

@camelid
Copy link
Member Author

camelid commented Apr 9, 2021

It looks like that issue has been closed as intended behavior?

@jyn514
Copy link
Member

jyn514 commented Apr 9, 2021

Yup, I commented here before it had been closed.

@lolgeny
Copy link

lolgeny commented Apr 13, 2021

I'll try doing this
@rustbot claim

@vramana
Copy link
Contributor

vramana commented Mar 26, 2022

@jyn514
Copy link
Member

jyn514 commented Mar 26, 2022

It looks like at some point bootstrap started passing -Zrustdoc-map to cargo, so rustdoc now knows where rustc_hash is and can link to it. The original issue is not solved though, that re-exports of undocumented items will not have a source link generated. Here's an MCVE:

// inner crate
pub type DMatrix = Vec<u32>;

// outer crate
pub use inner::DMatrix;

cargo doc --no-deps generates
image

@vramana
Copy link
Contributor

vramana commented Mar 27, 2022

@jyn514 Can I work on this issue?

@jyn514
Copy link
Member

jyn514 commented Mar 27, 2022

Sure :) use @rustbot claim so other people know you're working on it.

@vramana
Copy link
Contributor

vramana commented Mar 27, 2022

@rustbot claim

@rustbot rustbot assigned vramana and unassigned lolgeny Mar 27, 2022
@vramana
Copy link
Contributor

vramana commented Apr 24, 2022

The problem appears to be with generate href for the undocumented item in href_from_span (context.rs)
We recieve krate variable as ExternalLocation::Unkown.

let (krate, src_root) = match *self.cache().extern_locations.get(&cnum)? {
ExternalLocation::Local => {
let e = ExternalCrate { crate_num: cnum };
(e.name(self.tcx()), e.src_root(self.tcx()))
}
ExternalLocation::Remote(ref s) => {
root = s.to_string();
let e = ExternalCrate { crate_num: cnum };
(e.name(self.tcx()), e.src_root(self.tcx()))
}
ExternalLocation::Unknown => return None,
};

This in turn is caused may be incorrect Cache population. populate (cache.rs)

let name = e.name(tcx);
let render_options = &cx.render_options;
let extern_url = render_options.extern_html_root_urls.get(name.as_str()).map(|u| &**u);
let extern_url_takes_precedence = render_options.extern_html_root_takes_precedence;
let dst = &render_options.output;
let location = e.location(extern_url, extern_url_takes_precedence, dst, tcx);
cx.cache.extern_locations.insert(e.crate_num, location);
cx.cache.external_paths.insert(e.def_id(), (vec![name], ItemType::Module));

When cache is populated, the crate location (ExternalLocation) is set as Unknown. For crates linked via workspace, they are classified as local by checking crate name exists in destination path. For other external crates, we deduce it from tcx.

let did = self.crate_num.as_def_id();
tcx.get_attrs(did)
.lists(sym::doc)
.filter(|a| a.has_name(sym::html_root_url))
.filter_map(|a| a.value_str())
.map(to_remote)
.next()
.or_else(|| extern_url.map(to_remote)) // NOTE: only matters if `extern_url_takes_precedence` is false
.unwrap_or(Unknown) // Well, at least we tried.

But tcx.get_atts(did) is an empty array when crate is hosted on directly on github. Apparently this is expected according to @jyn514

We probably need to modify href_from_span(context.rs) to handle the ExternalLocation::Unkown case. I don't know have any ideas on how to fix this. I'll unassign myself from this issue.

My test repo to reproduce the issue https://github.com/vramana/test-rustdoc-link

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. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. 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

6 participants