Skip to content

intra-doc links: crate means something different when an item is re-exported #77193

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
dylni opened this issue Sep 25, 2020 · 4 comments · Fixed by #77253
Closed

intra-doc links: crate means something different when an item is re-exported #77193

dylni opened this issue Sep 25, 2020 · 4 comments · Fixed by #77253
Assignees
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. P-high High priority T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@dylni
Copy link
Contributor

dylni commented Sep 25, 2020

Crate quit exports main from quit_macros. main is documented in quit and has no documentation in quit_macros.

When that documentation has a link to [with_code], which is only in crate quit, it is unresolved. However, [crate::with_code] is resolved.

Rustdoc gives no warnings with either.

You can see the result with quit version 1.1.1 here.

cc @jyn514
cc #76106 (comment)

@dylni dylni added the C-bug Category: This is a bug. label Sep 25, 2020
@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 Sep 25, 2020
@dylni dylni changed the title Relative intra-rustdoc links are not resolved when documenting re-exported types Relative intra-rustdoc links are not resolved when documenting re-exported items Sep 25, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 25, 2020

All of this was wrong, I forgot to use +nightly

This isn't related to scopes, rustdoc will also fail to resolve these links:

/// [crate::process::Command]
///
/// [Vec]
pub use std::string::String;

Something about the re-export itself is messing it up.

@jyn514
Copy link
Member

jyn514 commented Sep 25, 2020

The good news is this is unrelated to proc-macros. Here's what I found:

/// [std::process::Command] // works
///
/// [Vec] // works
/// [with_code] // silently breaks
/// [crate::with_code] // works
pub use std::string::String;

This is really strange. I would have expected it to resolve relative to std::string, not relative to the current crate.

@jyn514
Copy link
Member

jyn514 commented Sep 25, 2020

Ok, I found the issue: rustdoc and resolve disagree about which crate this is in 🤦

Sep 25 13:38:39.133 DEBUG rustdoc::passes::collect_intra_doc_links: resolving with_code as a macro in the module DefId(5:3806 ~ alloc[e7ed]::string[0])
Sep 25 13:38:39.133 DEBUG rustdoc::passes::collect_intra_doc_links: with_code resolved to Err(()) in namespace ValueNS
Sep 25 13:38:39.133 DEBUG rustdoc::passes::collect_intra_doc_links: resolving crate::with_code as a macro in the module DefId(5:3806 ~ alloc[e7ed]::string[0])
Sep 25 13:38:39.133 DEBUG rustdoc::passes::collect_intra_doc_links: crate::with_code resolved to Ok((Path { span: re-export.rs:1:1: 1:1 (#0), segments: [PathSegment { ident: crate#0, id: NodeId(28), args: None }, PathSegment { ident: with_code#0, id: NodeId(29), args: None }], tokens: None }, Def(Fn, DefId(0:4 ~ re_export[8787]::with_code[0])))) in namespace ValueNS
Sep 25 13:38:39.133 DEBUG rustdoc::passes::collect_intra_doc_links: intra-doc link to crate::with_code resolved to Def(Fn, DefId(0:4 ~ re_export[8787]::with_code[0]))

For with_code, rustdoc passes the original module of String, alloc::string, and resolve correctly says there's no item named alloc::string::with_code. But for crate::with_code, resolve thinks that crate refers to the current crate, not to alloc.

This explains a lot of other broken links I've been seeing actually: anything that has crate:: will be wrong. So you were actually correct to start that this was the same issue as #76106 ;)

The fix will be a little tricky ... there's no 'change crate' button for resolve. There are a few things rustdoc can do:

  1. Resolve all links relative to the current module (the status quo before Resolve items for cross-crate imports relative to the original module #73101). This is a no-go, because it means links will break on re-exports (Support intra-doc links on cross-crate reexports #65983).
  2. Resolve all links relative to the original module. This is what I thought was the current behavior. It means that additional documentation can't refer to things in the current lexical scope, which is non-intuitive (cc [rustdoc] intra-doc links for macros always resolve relative to the crate root #72243).
  3. Resolve links in lexical scope: the original docs get resolved in the original module; the new docs get resolved in the module with the re-export. This is the ideal fix but somewhat hard to do because rustdoc doesn't distinguish between the two internally.

A possible way to implement 2. is by stripping crate:: wherever we see it and resolving relative to the crate root instead of the current module. This is slightly hacky but should work fairly reliably.

@jyn514 jyn514 changed the title Relative intra-rustdoc links are not resolved when documenting re-exported items intra-doc links: crate means something different when an item is re-exported Sep 25, 2020
@jyn514 jyn514 self-assigned this Sep 25, 2020
@jyn514 jyn514 added the P-high High priority label Sep 25, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 25, 2020

Marking as P-high after discussing with @GuillaumeGomez ; it would be unfortunate to still have this bug on the first release after stabilization.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 28, 2020
Resolve `crate` in intra-doc links properly across crates

Closes rust-lang#77193; see rust-lang#77193 (comment) for an explanation of what's going on here.
~~This also fixes the BTreeMap docs that have been broken for a while; see the description on the second commit for why and how.~~ Nope, see the second commit for why the link had to be changed.

r? @Manishearth
cc @dylni

@dylni note that this doesn't solve your original problem - now _both_ `with_code` and `crate::with_code` will be broken links. However this will fix a lot of other broken links (in particular I think https://docs.rs/sqlx/0.4.0-beta.1/sqlx/query/struct.Query.html is because of this bug). I'll open another issue for resolving additional docs in the new scope.
@bors bors closed this as completed in 9e34b72 Sep 29, 2020
# for free to join this conversation on GitHub. Already have an account? # 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. P-high High priority 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