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: Fix resolution of crate-relative paths in doc links #95337

Merged
merged 1 commit into from
Apr 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3285,12 +3285,21 @@ impl<'a> Resolver<'a> {
&mut self,
path_str: &str,
ns: Namespace,
module_id: DefId,
mut module_id: DefId,
) -> Option<Res> {
let mut segments =
Vec::from_iter(path_str.split("::").map(Ident::from_str).map(Segment::from_ident));
if path_str.starts_with("::") {
segments[0].ident.name = kw::PathRoot;
if let Some(segment) = segments.first_mut() {
if segment.ident.name == kw::Crate {
// FIXME: `resolve_path` always resolves `crate` to the current crate root, but
// rustdoc wants it to resolve to the `module_id`'s crate root. This trick of
// replacing `crate` with `self` and changing the current module should achieve
// the same effect.
segment.ident.name = kw::SelfLower;
module_id = module_id.krate.as_def_id();
} else if segment.ident.name == kw::Empty {
segment.ident.name = kw::PathRoot;
}
}

let module = self.expect_module(module_id);
Expand Down
37 changes: 5 additions & 32 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_hir::def::{
Namespace::{self, *},
PerNS,
};
use rustc_hir::def_id::{CrateNum, DefId, CRATE_DEF_ID};
use rustc_hir::def_id::{DefId, CRATE_DEF_ID};
use rustc_hir::Mutability;
use rustc_middle::ty::{DefIdTree, Ty, TyCtxt};
use rustc_middle::{bug, span_bug, ty};
Expand Down Expand Up @@ -1043,16 +1043,11 @@ impl<'a, 'tcx> DocVisitor for LinkCollector<'a, 'tcx> {
// so we know which module it came from.
for (parent_module, doc) in item.attrs.collapsed_doc_value_by_module_level() {
debug!("combined_docs={}", doc);

let (krate, parent_node) = if let Some(id) = parent_module {
(id.krate, Some(id))
} else {
(item.def_id.krate(), parent_node)
};
// NOTE: if there are links that start in one crate and end in another, this will not resolve them.
// This is a degenerate case and it's not supported by rustdoc.
let parent_node = parent_module.or(parent_node);
for md_link in markdown_links(&doc) {
let link = self.resolve_link(&item, &doc, parent_node, krate, md_link);
let link = self.resolve_link(&item, &doc, parent_node, md_link);
if let Some(link) = link {
self.cx.cache.intra_doc_links.entry(item.def_id).or_default().push(link);
}
Expand Down Expand Up @@ -1187,7 +1182,6 @@ impl LinkCollector<'_, '_> {
item: &Item,
dox: &str,
parent_node: Option<DefId>,
krate: CrateNum,
ori_link: MarkdownLink,
) -> Option<ItemLink> {
trace!("considering link '{}'", ori_link.link);
Expand All @@ -1199,7 +1193,7 @@ impl LinkCollector<'_, '_> {
link_range: ori_link.range.clone(),
};

let PreprocessingInfo { path_str, disambiguator, extra_fragment, link_text } =
let PreprocessingInfo { ref path_str, disambiguator, extra_fragment, link_text } =
match preprocess_link(&ori_link)? {
Ok(x) => x,
Err(err) => {
Expand All @@ -1221,7 +1215,6 @@ impl LinkCollector<'_, '_> {
return None;
}
};
let mut path_str = &*path_str;

let inner_docs = item.inner_docs(self.cx.tcx);

Expand All @@ -1239,7 +1232,7 @@ impl LinkCollector<'_, '_> {
let base_node =
if item.is_mod() && inner_docs { self.mod_ids.last().copied() } else { parent_node };

let Some(mut module_id) = base_node else {
let Some(module_id) = base_node else {
// This is a bug.
debug!("attempting to resolve item without parent module: {}", path_str);
resolution_failure(
Expand All @@ -1252,26 +1245,6 @@ impl LinkCollector<'_, '_> {
return None;
};

let resolved_self;
let is_lone_crate = path_str == "crate";
if path_str.starts_with("crate::") || is_lone_crate {
use rustc_span::def_id::CRATE_DEF_INDEX;

// HACK(jynelson): rustc_resolve thinks that `crate` is the crate currently being documented.
// But rustdoc wants it to mean the crate this item was originally present in.
// To work around this, remove it and resolve relative to the crate root instead.
// HACK(jynelson)(2): If we just strip `crate::` then suddenly primitives become ambiguous
// (consider `crate::char`). Instead, change it to `self::`. This works because 'self' is now the crate root.
// FIXME(#78696): This doesn't always work.
if is_lone_crate {
path_str = "self";
} else {
resolved_self = format!("self::{}", &path_str["crate::".len()..]);
path_str = &resolved_self;
}
module_id = DefId { krate, index: CRATE_DEF_INDEX };
}

let (mut res, fragment) = self.resolve_with_disambiguator_cached(
ResolutionInfo {
item_id: item.def_id,
Expand Down
8 changes: 0 additions & 8 deletions src/librustdoc/passes/collect_intra_doc_links/early.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,6 @@ crate fn early_resolve_intra_doc_links(
all_trait_impls: Default::default(),
};

// Because of the `crate::` prefix, any doc comment can reference
// the crate root's set of in-scope traits. This line makes sure
// it's available.
loader.add_traits_in_scope(CRATE_DEF_ID.to_def_id());
camelid marked this conversation as resolved.
Show resolved Hide resolved

// Overridden `visit_item` below doesn't apply to the crate root,
// so we have to visit its attributes and reexports separately.
loader.load_links_in_attrs(&krate.attrs);
Expand Down Expand Up @@ -105,9 +100,6 @@ impl IntraLinkCrateLoader<'_, '_> {
/// having impls in them.
fn add_foreign_traits_in_scope(&mut self) {
for cnum in Vec::from_iter(self.resolver.cstore().crates_untracked()) {
// FIXME: Due to #78696 rustdoc can query traits in scope for any crate root.
self.add_traits_in_scope(cnum.as_def_id());
camelid marked this conversation as resolved.
Show resolved Hide resolved

let all_traits = Vec::from_iter(self.resolver.cstore().traits_in_crate_untracked(cnum));
let all_trait_impls =
Vec::from_iter(self.resolver.cstore().trait_impls_in_crate_untracked(cnum));
Expand Down
5 changes: 5 additions & 0 deletions src/test/rustdoc-ui/intra-doc/crate-nonexistent.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#![deny(rustdoc::broken_intra_doc_links)]

/// [crate::DoesNotExist]
//~^ ERROR unresolved link to `crate::DoesNotExist`
pub struct Item;
14 changes: 14 additions & 0 deletions src/test/rustdoc-ui/intra-doc/crate-nonexistent.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error: unresolved link to `crate::DoesNotExist`
--> $DIR/crate-nonexistent.rs:3:6
|
LL | /// [crate::DoesNotExist]
| ^^^^^^^^^^^^^^^^^^^ no item named `DoesNotExist` in module `crate_nonexistent`
|
note: the lint level is defined here
--> $DIR/crate-nonexistent.rs:1:9
|
LL | #![deny(rustdoc::broken_intra_doc_links)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

17 changes: 17 additions & 0 deletions src/test/rustdoc/intra-doc/crate-relative-assoc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
pub mod io {
pub trait Read {
fn read(&mut self);
}
}

pub mod bufreader {
// @has crate_relative_assoc/bufreader/index.html '//a/@href' 'struct.TcpStream.html#method.read'
//! [`crate::TcpStream::read`]
use crate::io::Read;
}

pub struct TcpStream;

impl crate::io::Read for TcpStream {
fn read(&mut self) {}
}