From f5ee822098580f4e99bc0c710427727fe9df802c Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 26 Mar 2022 02:50:00 +0300 Subject: [PATCH] rustdoc: Fix resolution of `crate`-relative paths in doc links --- compiler/rustc_resolve/src/lib.rs | 15 ++++++-- .../passes/collect_intra_doc_links.rs | 37 +++---------------- .../passes/collect_intra_doc_links/early.rs | 8 ---- .../rustdoc-ui/intra-doc/crate-nonexistent.rs | 5 +++ .../intra-doc/crate-nonexistent.stderr | 14 +++++++ .../rustdoc/intra-doc/crate-relative-assoc.rs | 17 +++++++++ 6 files changed, 53 insertions(+), 43 deletions(-) create mode 100644 src/test/rustdoc-ui/intra-doc/crate-nonexistent.rs create mode 100644 src/test/rustdoc-ui/intra-doc/crate-nonexistent.stderr create mode 100644 src/test/rustdoc/intra-doc/crate-relative-assoc.rs diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 48675fa88270a..082da70be3969 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -3285,12 +3285,21 @@ impl<'a> Resolver<'a> { &mut self, path_str: &str, ns: Namespace, - module_id: DefId, + mut module_id: DefId, ) -> Option { 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); diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 2129814c16899..e19920cc2ceb6 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -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}; @@ -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); } @@ -1187,7 +1182,6 @@ impl LinkCollector<'_, '_> { item: &Item, dox: &str, parent_node: Option, - krate: CrateNum, ori_link: MarkdownLink, ) -> Option { trace!("considering link '{}'", ori_link.link); @@ -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) => { @@ -1221,7 +1215,6 @@ impl LinkCollector<'_, '_> { return None; } }; - let mut path_str = &*path_str; let inner_docs = item.inner_docs(self.cx.tcx); @@ -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( @@ -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, diff --git a/src/librustdoc/passes/collect_intra_doc_links/early.rs b/src/librustdoc/passes/collect_intra_doc_links/early.rs index 75e952c5122b8..39900270ccb6d 100644 --- a/src/librustdoc/passes/collect_intra_doc_links/early.rs +++ b/src/librustdoc/passes/collect_intra_doc_links/early.rs @@ -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); @@ -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)); diff --git a/src/test/rustdoc-ui/intra-doc/crate-nonexistent.rs b/src/test/rustdoc-ui/intra-doc/crate-nonexistent.rs new file mode 100644 index 0000000000000..ceecfa6816c1e --- /dev/null +++ b/src/test/rustdoc-ui/intra-doc/crate-nonexistent.rs @@ -0,0 +1,5 @@ +#![deny(rustdoc::broken_intra_doc_links)] + +/// [crate::DoesNotExist] +//~^ ERROR unresolved link to `crate::DoesNotExist` +pub struct Item; diff --git a/src/test/rustdoc-ui/intra-doc/crate-nonexistent.stderr b/src/test/rustdoc-ui/intra-doc/crate-nonexistent.stderr new file mode 100644 index 0000000000000..a69b1c52ac557 --- /dev/null +++ b/src/test/rustdoc-ui/intra-doc/crate-nonexistent.stderr @@ -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 + diff --git a/src/test/rustdoc/intra-doc/crate-relative-assoc.rs b/src/test/rustdoc/intra-doc/crate-relative-assoc.rs new file mode 100644 index 0000000000000..d4a0ecc35aeaf --- /dev/null +++ b/src/test/rustdoc/intra-doc/crate-relative-assoc.rs @@ -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) {} +}