Skip to content

Commit

Permalink
Rollup merge of #73101 - jyn514:rustdoc-absolute-module, r=Manishearth
Browse files Browse the repository at this point in the history
Resolve items for cross-crate imports relative to the original module

~~Blocked on #73103 and #73566

Closes #65983.

I tested on the following code (as mentioned in #65983 (comment)):

```
pub use rand::Rng;
```
and rustdoc generated the following link: https://rust-random.github.io/rand/rand_core/trait.RngCore.html
  • Loading branch information
Manishearth authored Jul 17, 2020
2 parents 5c9e5df + c46e038 commit ec93d56
Show file tree
Hide file tree
Showing 21 changed files with 233 additions and 48 deletions.
17 changes: 11 additions & 6 deletions src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,17 @@ impl<'a> Resolver<'a> {
(self.cstore().crate_name_untracked(def_id.krate), None)
} else {
let def_key = self.cstore().def_key(def_id);
(
// This unwrap is safe: crates must always have a name
def_key.disambiguated_data.data.get_opt_name().unwrap(),
// This unwrap is safe since we know this isn't the root
Some(self.get_module(DefId { index: def_key.parent.unwrap(), ..def_id })),
)
let name = def_key
.disambiguated_data
.data
.get_opt_name()
.expect("given a DefId that wasn't a module");
// This unwrap is safe since we know this isn't the root
let parent = Some(self.get_module(DefId {
index: def_key.parent.expect("failed to get parent for module"),
..def_id
}));
(name, parent)
};

// Allocate and return a new module with the information we found
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2978,7 +2978,7 @@ impl<'a> Resolver<'a> {
span: Span,
path_str: &str,
ns: Namespace,
module_id: LocalDefId,
module_id: DefId,
) -> Result<(ast::Path, Res), ()> {
let path = if path_str.starts_with("::") {
ast::Path {
Expand All @@ -2998,7 +2998,7 @@ impl<'a> Resolver<'a> {
.collect(),
}
};
let module = self.module_map.get(&module_id).copied().unwrap_or(self.graph_root);
let module = self.get_module(module_id);
let parent_scope = &ParentScope::module(module);
let res = self.resolve_ast_path(&path, ns, parent_scope).map_err(|_| ())?;
Ok((path, res))
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt
DUMMY_SP,
extern_name,
TypeNS,
LocalDefId { local_def_index: CRATE_DEF_INDEX },
LocalDefId { local_def_index: CRATE_DEF_INDEX }.to_def_id(),
)
.unwrap_or_else(|()| {
panic!("Unable to resolve external crate {}", extern_name)
Expand Down
83 changes: 46 additions & 37 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_hir::def::{
Namespace::{self, *},
PerNS, Res,
};
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::def_id::DefId;
use rustc_middle::ty;
use rustc_resolve::ParentScope;
use rustc_session::lint;
Expand Down Expand Up @@ -50,7 +50,8 @@ enum ErrorKind {

struct LinkCollector<'a, 'tcx> {
cx: &'a DocContext<'tcx>,
mod_ids: Vec<hir::HirId>,
// NOTE: this may not necessarily be a module in the current crate
mod_ids: Vec<DefId>,
}

impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
Expand All @@ -62,7 +63,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
&self,
path_str: &str,
current_item: &Option<String>,
module_id: LocalDefId,
module_id: DefId,
) -> Result<(Res, Option<String>), ErrorKind> {
let cx = self.cx;

Expand Down Expand Up @@ -124,7 +125,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
}

/// Resolves a string as a macro.
fn macro_resolve(&self, path_str: &str, parent_id: Option<hir::HirId>) -> Option<Res> {
fn macro_resolve(&self, path_str: &str, parent_id: Option<DefId>) -> Option<Res> {
let cx = self.cx;
let path = ast::Path::from_ident(Ident::from_str(path_str));
cx.enter_resolver(|resolver| {
Expand All @@ -142,8 +143,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
if let Some(res) = resolver.all_macros().get(&Symbol::intern(path_str)) {
return Some(res.map_id(|_| panic!("unexpected id")));
}
if let Some(module_id) = parent_id.or(self.mod_ids.last().cloned()) {
let module_id = cx.tcx.hir().local_def_id(module_id);
if let Some(module_id) = parent_id {
if let Ok((_, res)) =
resolver.resolve_str_path_error(DUMMY_SP, path_str, MacroNS, module_id)
{
Expand All @@ -167,15 +167,14 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
disambiguator: Option<&str>,
ns: Namespace,
current_item: &Option<String>,
parent_id: Option<hir::HirId>,
parent_id: Option<DefId>,
extra_fragment: &Option<String>,
item_opt: Option<&Item>,
) -> Result<(Res, Option<String>), ErrorKind> {
let cx = self.cx;

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

impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
fn fold_item(&mut self, mut item: Item) -> Option<Item> {
let item_hir_id = if item.is_mod() {
if let Some(def_id) = item.def_id.as_local() {
Some(self.cx.tcx.hir().as_local_hir_id(def_id))
} else {
debug!("attempting to fold on a non-local item: {:?}", item);
return self.fold_item_recur(item);
}
} else {
None
};
use rustc_middle::ty::DefIdTree;

// FIXME: get the resolver to work with non-local resolve scopes.
let parent_node = self.cx.as_local_hir_id(item.def_id).and_then(|hir_id| {
// FIXME: this fails hard for impls in non-module scope, but is necessary for the
// current `resolve()` implementation.
match self.cx.as_local_hir_id(self.cx.tcx.parent_module(hir_id).to_def_id()).unwrap() {
id if id != hir_id => Some(id),
_ => None,
let parent_node = if item.is_fake() {
// FIXME: is this correct?
None
} else {
let mut current = item.def_id;
// The immediate parent might not always be a module.
// Find the first parent which is.
loop {
if let Some(parent) = self.cx.tcx.parent(current) {
if self.cx.tcx.def_kind(parent) == DefKind::Mod {
break Some(parent);
}
current = parent;
} else {
break None;
}
}
});
};

if parent_node.is_some() {
debug!("got parent node for {:?} {:?}, id {:?}", item.type_(), item.name, item.def_id);
trace!("got parent node for {:?} {:?}, id {:?}", item.type_(), item.name, item.def_id);
}

let current_item = match item.inner {
ModuleItem(..) => {
if item.attrs.inner_docs {
if item_hir_id.unwrap() != hir::CRATE_HIR_ID { item.name.clone() } else { None }
if item.def_id.is_top_level_module() { item.name.clone() } else { None }
} else {
match parent_node.or(self.mod_ids.last().cloned()) {
Some(parent) if parent != hir::CRATE_HIR_ID => {
match parent_node.or(self.mod_ids.last().copied()) {
Some(parent) if !parent.is_top_level_module() => {
// FIXME: can we pull the parent module's name from elsewhere?
Some(self.cx.tcx.hir().name(parent).to_string())
Some(self.cx.tcx.item_name(parent).to_string())
}
_ => None,
}
Expand All @@ -488,18 +487,22 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
for_.def_id().map(|did| self.cx.tcx.item_name(did).to_string())
}
// we don't display docs on `extern crate` items anyway, so don't process them.
ExternCrateItem(..) => return self.fold_item_recur(item),
ExternCrateItem(..) => {
debug!("ignoring extern crate item {:?}", item.def_id);
return self.fold_item_recur(item);
}
ImportItem(Import::Simple(ref name, ..)) => Some(name.clone()),
MacroItem(..) => None,
_ => item.name.clone(),
};

if item.is_mod() && item.attrs.inner_docs {
self.mod_ids.push(item_hir_id.unwrap());
self.mod_ids.push(item.def_id);
}

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

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

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

for (ori_link, link_range) in markdown_links(&dox) {
trace!("considering link '{}'", ori_link);

// Bail early for real links.
if ori_link.contains('/') {
continue;
Expand Down Expand Up @@ -641,8 +646,11 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
// we've already pushed this node onto the resolution stack but
// for outer comments we explicitly try and resolve against the
// parent_node first.
let base_node =
if item.is_mod() && item.attrs.inner_docs { None } else { parent_node };
let base_node = if item.is_mod() && item.attrs.inner_docs {
self.mod_ids.last().copied()
} else {
parent_node
};

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

if item.is_mod() && !item.attrs.inner_docs {
self.mod_ids.push(item_hir_id.unwrap());
self.mod_ids.push(item.def_id);
}

if item.is_mod() {
Expand Down Expand Up @@ -864,6 +872,7 @@ fn build_diagnostic(
Some(hir_id) => hir_id,
None => {
// If non-local, no need to check anything.
info!("ignoring warning from parent crate: {}", err_msg);
return;
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/test/rustdoc-ui/intra-links-private.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// check-pass
// revisions: public private
// [private]compile-flags: --document-private-items
#![cfg_attr(private, deny(intra_doc_resolution_failure))]
#![cfg_attr(private, deny(intra_doc_link_resolution_failure))]

/// docs [DontDocMe]
//[public]~^ WARNING `[DontDocMe]` public documentation for `DocMe` links to a private item
Expand Down
10 changes: 10 additions & 0 deletions src/test/rustdoc/intra-doc-crate/additional_doc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// aux-build:additional_doc.rs
// build-aux-docs
#![deny(intra_doc_link_resolution_failure)]

extern crate my_rand;

// @has 'additional_doc/trait.Rng.html' '//a[@href="../additional_doc/trait.Rng.html"]' 'Rng'
// @has 'additional_doc/trait.Rng.html' '//a[@href="../my_rand/trait.RngCore.html"]' 'RngCore'
/// This is an [`Rng`].
pub use my_rand::Rng;
6 changes: 6 additions & 0 deletions src/test/rustdoc/intra-doc-crate/auxiliary/additional_doc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#![crate_name = "my_rand"]
#![deny(intra_doc_link_resolution_failure)]

pub trait RngCore {}
/// Rng extends [`RngCore`].
pub trait Rng: RngCore {}
7 changes: 7 additions & 0 deletions src/test/rustdoc/intra-doc-crate/auxiliary/intra-doc-basic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#![crate_name = "a"]
#![deny(intra_doc_link_resolution_failure)]

pub struct Foo;

/// Link to [Foo]
pub struct Bar;
10 changes: 10 additions & 0 deletions src/test/rustdoc/intra-doc-crate/auxiliary/macro_inner.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#![crate_name = "macro_inner"]
#![deny(intra_doc_link_resolution_failure)]

pub struct Foo;

/// See also [`Foo`]
#[macro_export]
macro_rules! my_macro {
() => {}
}
7 changes: 7 additions & 0 deletions src/test/rustdoc/intra-doc-crate/auxiliary/module.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#![crate_name = "module_inner"]
#![deny(intra_doc_link_resolution_failure)]
/// [SomeType] links to [bar]
pub struct SomeType;
pub trait SomeTrait {}
/// [bar] links to [SomeTrait] and also [SomeType]
pub mod bar {}
20 changes: 20 additions & 0 deletions src/test/rustdoc/intra-doc-crate/auxiliary/proc_macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// force-host
// no-prefer-dynamic
// compile-flags: --crate-type proc-macro
#![crate_type="proc-macro"]
#![crate_name="proc_macro_inner"]

extern crate proc_macro;

use proc_macro::TokenStream;

/// Links to [`OtherDerive`]
#[proc_macro_derive(DeriveA)]
pub fn a_derive(input: TokenStream) -> TokenStream {
input
}

#[proc_macro_derive(OtherDerive)]
pub fn other_derive(input: TokenStream) -> TokenStream {
input
}
12 changes: 12 additions & 0 deletions src/test/rustdoc/intra-doc-crate/auxiliary/submodule-inner.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#![crate_name = "a"]
#![deny(intra_doc_link_resolution_failure)]

pub mod bar {
pub struct Bar;
}

pub mod foo {
use crate::bar;
/// link to [bar::Bar]
pub struct Foo;
}
13 changes: 13 additions & 0 deletions src/test/rustdoc/intra-doc-crate/auxiliary/submodule-outer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#![crate_name = "bar"]
#![deny(intra_doc_link_resolution_failure)]

pub trait Foo {
/// [`Bar`] [`Baz`]
fn foo();
}

pub trait Bar {
}

pub trait Baz {
}
16 changes: 16 additions & 0 deletions src/test/rustdoc/intra-doc-crate/auxiliary/traits.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#![crate_name = "inner"]
/// this is a trait
pub trait SomeTrait {
/// this is a method for [a trait][SomeTrait]
fn foo();
}

pub mod bar {
use super::SomeTrait;

pub struct BarStruct;

impl SomeTrait for BarStruct {
fn foo() {}
}
}
9 changes: 9 additions & 0 deletions src/test/rustdoc/intra-doc-crate/basic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// aux-build:intra-doc-basic.rs
// build-aux-docs
#![deny(intra_doc_link_resolution_failure)]

// from https://github.com/rust-lang/rust/issues/65983
extern crate a;

// @has 'basic/struct.Bar.html' '//a[@href="../a/struct.Foo.html"]' 'Foo'
pub use a::Bar;
12 changes: 12 additions & 0 deletions src/test/rustdoc/intra-doc-crate/macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// ignore-tidy-linelength
// aux-build:macro_inner.rs
// aux-build:proc_macro.rs
// build-aux-docs
#![deny(intra_doc_link_resolution_failure)]
extern crate macro_inner;
extern crate proc_macro_inner;

// @has 'macro/macro.my_macro.html' '//a[@href="../macro_inner/struct.Foo.html"]' 'Foo'
pub use macro_inner::my_macro;
// @has 'macro/derive.DeriveA.html' '//a[@href="../proc_macro_inner/derive.OtherDerive.html"]' 'OtherDerive'
pub use proc_macro_inner::DeriveA;
8 changes: 8 additions & 0 deletions src/test/rustdoc/intra-doc-crate/module.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// outer.rs
// aux-build: module.rs
// build-aux-docs
#![deny(intra_doc_link_resolution_failure)]
extern crate module_inner;
// @has 'module/bar/index.html' '//a[@href="../../module_inner/trait.SomeTrait.html"]' 'SomeTrait'
// @has 'module/bar/index.html' '//a[@href="../../module_inner/struct.SomeType.html"]' 'SomeType'
pub use module_inner::bar;
Loading

0 comments on commit ec93d56

Please sign in to comment.