Skip to content

Commit

Permalink
Auto merge of #95337 - petrochenkov:doclink3, r=camelid
Browse files Browse the repository at this point in the history
rustdoc: Fix resolution of `crate`-relative paths in doc links

Resolve `crate::foo` paths transparently to rustdoc, so their resolution no longer affects diagnostics and modules used for determining traits in scope.

The proper solution is to account for the current `module_id`/`parent_scope` in `fn resolve_crate_root`, but it's a slightly larger compiler changes. This PR moves the code closer to it, but keeps it rustdoc-specific.

Fixes #78696
Fixes #94924
  • Loading branch information
bors committed Apr 5, 2022
2 parents a22cf2a + f5ee822 commit 949b98c
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 43 deletions.
15 changes: 12 additions & 3 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3270,12 +3270,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());

// 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());

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) {}
}

0 comments on commit 949b98c

Please sign in to comment.