Skip to content

Commit 7b66afd

Browse files
authored
Rollup merge of rust-lang#73101 - jyn514:rustdoc-absolute-module, r=Manishearth
Resolve items for cross-crate imports relative to the original module ~~Blocked on rust-lang#73103 and rust-lang#73566 Closes rust-lang#65983. I tested on the following code (as mentioned in rust-lang#65983 (comment)): ``` pub use rand::Rng; ``` and rustdoc generated the following link: https://rust-random.github.io/rand/rand_core/trait.RngCore.html
2 parents 5c9e5df + c46e038 commit 7b66afd

21 files changed

+233
-48
lines changed

src/librustc_resolve/build_reduced_graph.rs

+11-6
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,17 @@ impl<'a> Resolver<'a> {
111111
(self.cstore().crate_name_untracked(def_id.krate), None)
112112
} else {
113113
let def_key = self.cstore().def_key(def_id);
114-
(
115-
// This unwrap is safe: crates must always have a name
116-
def_key.disambiguated_data.data.get_opt_name().unwrap(),
117-
// This unwrap is safe since we know this isn't the root
118-
Some(self.get_module(DefId { index: def_key.parent.unwrap(), ..def_id })),
119-
)
114+
let name = def_key
115+
.disambiguated_data
116+
.data
117+
.get_opt_name()
118+
.expect("given a DefId that wasn't a module");
119+
// This unwrap is safe since we know this isn't the root
120+
let parent = Some(self.get_module(DefId {
121+
index: def_key.parent.expect("failed to get parent for module"),
122+
..def_id
123+
}));
124+
(name, parent)
120125
};
121126

122127
// Allocate and return a new module with the information we found

src/librustc_resolve/lib.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2978,7 +2978,7 @@ impl<'a> Resolver<'a> {
29782978
span: Span,
29792979
path_str: &str,
29802980
ns: Namespace,
2981-
module_id: LocalDefId,
2981+
module_id: DefId,
29822982
) -> Result<(ast::Path, Res), ()> {
29832983
let path = if path_str.starts_with("::") {
29842984
ast::Path {
@@ -2998,7 +2998,7 @@ impl<'a> Resolver<'a> {
29982998
.collect(),
29992999
}
30003000
};
3001-
let module = self.module_map.get(&module_id).copied().unwrap_or(self.graph_root);
3001+
let module = self.get_module(module_id);
30023002
let parent_scope = &ParentScope::module(module);
30033003
let res = self.resolve_ast_path(&path, ns, parent_scope).map_err(|_| ())?;
30043004
Ok((path, res))

src/librustdoc/core.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
430430
DUMMY_SP,
431431
extern_name,
432432
TypeNS,
433-
LocalDefId { local_def_index: CRATE_DEF_INDEX },
433+
LocalDefId { local_def_index: CRATE_DEF_INDEX }.to_def_id(),
434434
)
435435
.unwrap_or_else(|()| {
436436
panic!("Unable to resolve external crate {}", extern_name)

src/librustdoc/passes/collect_intra_doc_links.rs

+46-37
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use rustc_hir::def::{
88
Namespace::{self, *},
99
PerNS, Res,
1010
};
11-
use rustc_hir::def_id::{DefId, LocalDefId};
11+
use rustc_hir::def_id::DefId;
1212
use rustc_middle::ty;
1313
use rustc_resolve::ParentScope;
1414
use rustc_session::lint;
@@ -50,7 +50,8 @@ enum ErrorKind {
5050

5151
struct LinkCollector<'a, 'tcx> {
5252
cx: &'a DocContext<'tcx>,
53-
mod_ids: Vec<hir::HirId>,
53+
// NOTE: this may not necessarily be a module in the current crate
54+
mod_ids: Vec<DefId>,
5455
}
5556

5657
impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
@@ -62,7 +63,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
6263
&self,
6364
path_str: &str,
6465
current_item: &Option<String>,
65-
module_id: LocalDefId,
66+
module_id: DefId,
6667
) -> Result<(Res, Option<String>), ErrorKind> {
6768
let cx = self.cx;
6869

@@ -124,7 +125,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
124125
}
125126

126127
/// Resolves a string as a macro.
127-
fn macro_resolve(&self, path_str: &str, parent_id: Option<hir::HirId>) -> Option<Res> {
128+
fn macro_resolve(&self, path_str: &str, parent_id: Option<DefId>) -> Option<Res> {
128129
let cx = self.cx;
129130
let path = ast::Path::from_ident(Ident::from_str(path_str));
130131
cx.enter_resolver(|resolver| {
@@ -142,8 +143,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
142143
if let Some(res) = resolver.all_macros().get(&Symbol::intern(path_str)) {
143144
return Some(res.map_id(|_| panic!("unexpected id")));
144145
}
145-
if let Some(module_id) = parent_id.or(self.mod_ids.last().cloned()) {
146-
let module_id = cx.tcx.hir().local_def_id(module_id);
146+
if let Some(module_id) = parent_id {
147147
if let Ok((_, res)) =
148148
resolver.resolve_str_path_error(DUMMY_SP, path_str, MacroNS, module_id)
149149
{
@@ -167,15 +167,14 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
167167
disambiguator: Option<&str>,
168168
ns: Namespace,
169169
current_item: &Option<String>,
170-
parent_id: Option<hir::HirId>,
170+
parent_id: Option<DefId>,
171171
extra_fragment: &Option<String>,
172172
item_opt: Option<&Item>,
173173
) -> Result<(Res, Option<String>), ErrorKind> {
174174
let cx = self.cx;
175175

176176
// In case we're in a module, try to resolve the relative path.
177-
if let Some(module_id) = parent_id.or(self.mod_ids.last().cloned()) {
178-
let module_id = cx.tcx.hir().local_def_id(module_id);
177+
if let Some(module_id) = parent_id {
179178
let result = cx.enter_resolver(|resolver| {
180179
resolver.resolve_str_path_error(DUMMY_SP, &path_str, ns, module_id)
181180
});
@@ -445,40 +444,40 @@ fn is_derive_trait_collision<T>(ns: &PerNS<Option<(Res, T)>>) -> bool {
445444

446445
impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
447446
fn fold_item(&mut self, mut item: Item) -> Option<Item> {
448-
let item_hir_id = if item.is_mod() {
449-
if let Some(def_id) = item.def_id.as_local() {
450-
Some(self.cx.tcx.hir().as_local_hir_id(def_id))
451-
} else {
452-
debug!("attempting to fold on a non-local item: {:?}", item);
453-
return self.fold_item_recur(item);
454-
}
455-
} else {
456-
None
457-
};
447+
use rustc_middle::ty::DefIdTree;
458448

459-
// FIXME: get the resolver to work with non-local resolve scopes.
460-
let parent_node = self.cx.as_local_hir_id(item.def_id).and_then(|hir_id| {
461-
// FIXME: this fails hard for impls in non-module scope, but is necessary for the
462-
// current `resolve()` implementation.
463-
match self.cx.as_local_hir_id(self.cx.tcx.parent_module(hir_id).to_def_id()).unwrap() {
464-
id if id != hir_id => Some(id),
465-
_ => None,
449+
let parent_node = if item.is_fake() {
450+
// FIXME: is this correct?
451+
None
452+
} else {
453+
let mut current = item.def_id;
454+
// The immediate parent might not always be a module.
455+
// Find the first parent which is.
456+
loop {
457+
if let Some(parent) = self.cx.tcx.parent(current) {
458+
if self.cx.tcx.def_kind(parent) == DefKind::Mod {
459+
break Some(parent);
460+
}
461+
current = parent;
462+
} else {
463+
break None;
464+
}
466465
}
467-
});
466+
};
468467

469468
if parent_node.is_some() {
470-
debug!("got parent node for {:?} {:?}, id {:?}", item.type_(), item.name, item.def_id);
469+
trace!("got parent node for {:?} {:?}, id {:?}", item.type_(), item.name, item.def_id);
471470
}
472471

473472
let current_item = match item.inner {
474473
ModuleItem(..) => {
475474
if item.attrs.inner_docs {
476-
if item_hir_id.unwrap() != hir::CRATE_HIR_ID { item.name.clone() } else { None }
475+
if item.def_id.is_top_level_module() { item.name.clone() } else { None }
477476
} else {
478-
match parent_node.or(self.mod_ids.last().cloned()) {
479-
Some(parent) if parent != hir::CRATE_HIR_ID => {
477+
match parent_node.or(self.mod_ids.last().copied()) {
478+
Some(parent) if !parent.is_top_level_module() => {
480479
// FIXME: can we pull the parent module's name from elsewhere?
481-
Some(self.cx.tcx.hir().name(parent).to_string())
480+
Some(self.cx.tcx.item_name(parent).to_string())
482481
}
483482
_ => None,
484483
}
@@ -488,18 +487,22 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
488487
for_.def_id().map(|did| self.cx.tcx.item_name(did).to_string())
489488
}
490489
// we don't display docs on `extern crate` items anyway, so don't process them.
491-
ExternCrateItem(..) => return self.fold_item_recur(item),
490+
ExternCrateItem(..) => {
491+
debug!("ignoring extern crate item {:?}", item.def_id);
492+
return self.fold_item_recur(item);
493+
}
492494
ImportItem(Import::Simple(ref name, ..)) => Some(name.clone()),
493495
MacroItem(..) => None,
494496
_ => item.name.clone(),
495497
};
496498

497499
if item.is_mod() && item.attrs.inner_docs {
498-
self.mod_ids.push(item_hir_id.unwrap());
500+
self.mod_ids.push(item.def_id);
499501
}
500502

501503
let cx = self.cx;
502504
let dox = item.attrs.collapsed_doc_value().unwrap_or_else(String::new);
505+
trace!("got documentation '{}'", dox);
503506

504507
look_for_tests(&cx, &dox, &item, true);
505508

@@ -541,6 +544,8 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
541544
});
542545

543546
for (ori_link, link_range) in markdown_links(&dox) {
547+
trace!("considering link '{}'", ori_link);
548+
544549
// Bail early for real links.
545550
if ori_link.contains('/') {
546551
continue;
@@ -641,8 +646,11 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
641646
// we've already pushed this node onto the resolution stack but
642647
// for outer comments we explicitly try and resolve against the
643648
// parent_node first.
644-
let base_node =
645-
if item.is_mod() && item.attrs.inner_docs { None } else { parent_node };
649+
let base_node = if item.is_mod() && item.attrs.inner_docs {
650+
self.mod_ids.last().copied()
651+
} else {
652+
parent_node
653+
};
646654

647655
// replace `Self` with suitable item's parent name
648656
if path_str.starts_with("Self::") {
@@ -826,7 +834,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
826834
}
827835

828836
if item.is_mod() && !item.attrs.inner_docs {
829-
self.mod_ids.push(item_hir_id.unwrap());
837+
self.mod_ids.push(item.def_id);
830838
}
831839

832840
if item.is_mod() {
@@ -864,6 +872,7 @@ fn build_diagnostic(
864872
Some(hir_id) => hir_id,
865873
None => {
866874
// If non-local, no need to check anything.
875+
info!("ignoring warning from parent crate: {}", err_msg);
867876
return;
868877
}
869878
};

src/test/rustdoc-ui/intra-links-private.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// check-pass
22
// revisions: public private
33
// [private]compile-flags: --document-private-items
4-
#![cfg_attr(private, deny(intra_doc_resolution_failure))]
4+
#![cfg_attr(private, deny(intra_doc_link_resolution_failure))]
55

66
/// docs [DontDocMe]
77
//[public]~^ WARNING `[DontDocMe]` public documentation for `DocMe` links to a private item
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
// aux-build:additional_doc.rs
2+
// build-aux-docs
3+
#![deny(intra_doc_link_resolution_failure)]
4+
5+
extern crate my_rand;
6+
7+
// @has 'additional_doc/trait.Rng.html' '//a[@href="../additional_doc/trait.Rng.html"]' 'Rng'
8+
// @has 'additional_doc/trait.Rng.html' '//a[@href="../my_rand/trait.RngCore.html"]' 'RngCore'
9+
/// This is an [`Rng`].
10+
pub use my_rand::Rng;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#![crate_name = "my_rand"]
2+
#![deny(intra_doc_link_resolution_failure)]
3+
4+
pub trait RngCore {}
5+
/// Rng extends [`RngCore`].
6+
pub trait Rng: RngCore {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#![crate_name = "a"]
2+
#![deny(intra_doc_link_resolution_failure)]
3+
4+
pub struct Foo;
5+
6+
/// Link to [Foo]
7+
pub struct Bar;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#![crate_name = "macro_inner"]
2+
#![deny(intra_doc_link_resolution_failure)]
3+
4+
pub struct Foo;
5+
6+
/// See also [`Foo`]
7+
#[macro_export]
8+
macro_rules! my_macro {
9+
() => {}
10+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#![crate_name = "module_inner"]
2+
#![deny(intra_doc_link_resolution_failure)]
3+
/// [SomeType] links to [bar]
4+
pub struct SomeType;
5+
pub trait SomeTrait {}
6+
/// [bar] links to [SomeTrait] and also [SomeType]
7+
pub mod bar {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// force-host
2+
// no-prefer-dynamic
3+
// compile-flags: --crate-type proc-macro
4+
#![crate_type="proc-macro"]
5+
#![crate_name="proc_macro_inner"]
6+
7+
extern crate proc_macro;
8+
9+
use proc_macro::TokenStream;
10+
11+
/// Links to [`OtherDerive`]
12+
#[proc_macro_derive(DeriveA)]
13+
pub fn a_derive(input: TokenStream) -> TokenStream {
14+
input
15+
}
16+
17+
#[proc_macro_derive(OtherDerive)]
18+
pub fn other_derive(input: TokenStream) -> TokenStream {
19+
input
20+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#![crate_name = "a"]
2+
#![deny(intra_doc_link_resolution_failure)]
3+
4+
pub mod bar {
5+
pub struct Bar;
6+
}
7+
8+
pub mod foo {
9+
use crate::bar;
10+
/// link to [bar::Bar]
11+
pub struct Foo;
12+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#![crate_name = "bar"]
2+
#![deny(intra_doc_link_resolution_failure)]
3+
4+
pub trait Foo {
5+
/// [`Bar`] [`Baz`]
6+
fn foo();
7+
}
8+
9+
pub trait Bar {
10+
}
11+
12+
pub trait Baz {
13+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#![crate_name = "inner"]
2+
/// this is a trait
3+
pub trait SomeTrait {
4+
/// this is a method for [a trait][SomeTrait]
5+
fn foo();
6+
}
7+
8+
pub mod bar {
9+
use super::SomeTrait;
10+
11+
pub struct BarStruct;
12+
13+
impl SomeTrait for BarStruct {
14+
fn foo() {}
15+
}
16+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// aux-build:intra-doc-basic.rs
2+
// build-aux-docs
3+
#![deny(intra_doc_link_resolution_failure)]
4+
5+
// from https://github.com/rust-lang/rust/issues/65983
6+
extern crate a;
7+
8+
// @has 'basic/struct.Bar.html' '//a[@href="../a/struct.Foo.html"]' 'Foo'
9+
pub use a::Bar;
+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// ignore-tidy-linelength
2+
// aux-build:macro_inner.rs
3+
// aux-build:proc_macro.rs
4+
// build-aux-docs
5+
#![deny(intra_doc_link_resolution_failure)]
6+
extern crate macro_inner;
7+
extern crate proc_macro_inner;
8+
9+
// @has 'macro/macro.my_macro.html' '//a[@href="../macro_inner/struct.Foo.html"]' 'Foo'
10+
pub use macro_inner::my_macro;
11+
// @has 'macro/derive.DeriveA.html' '//a[@href="../proc_macro_inner/derive.OtherDerive.html"]' 'OtherDerive'
12+
pub use proc_macro_inner::DeriveA;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// outer.rs
2+
// aux-build: module.rs
3+
// build-aux-docs
4+
#![deny(intra_doc_link_resolution_failure)]
5+
extern crate module_inner;
6+
// @has 'module/bar/index.html' '//a[@href="../../module_inner/trait.SomeTrait.html"]' 'SomeTrait'
7+
// @has 'module/bar/index.html' '//a[@href="../../module_inner/struct.SomeType.html"]' 'SomeType'
8+
pub use module_inner::bar;

0 commit comments

Comments
 (0)