Skip to content

Commit

Permalink
fix intra-links for trait impls
Browse files Browse the repository at this point in the history
  • Loading branch information
QuietMisdreavus committed Sep 20, 2018
1 parent 755c02d commit 1106577
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 21 deletions.
13 changes: 9 additions & 4 deletions src/librustc/hir/map/mod.rs
Expand Up @@ -709,17 +709,22 @@ impl<'hir> Map<'hir> {
}
}

/// Returns the NodeId of `id`'s nearest module parent, or `id` itself if no
/// Returns the DefId of `id`'s nearest module parent, or `id` itself if no
/// module parent is in this map.
pub fn get_module_parent(&self, id: NodeId) -> DefId {
let id = match self.walk_parent_nodes(id, |node| match *node {
self.local_def_id(self.get_module_parent_node(id))
}

/// Returns the NodeId of `id`'s nearest module parent, or `id` itself if no
/// module parent is in this map.
pub fn get_module_parent_node(&self, id: NodeId) -> NodeId {
match self.walk_parent_nodes(id, |node| match *node {
Node::Item(&Item { node: ItemKind::Mod(_), .. }) => true,
_ => false,
}, |_| false) {
Ok(id) => id,
Err(id) => id,
};
self.local_def_id(id)
}
}

/// Returns the nearest enclosing scope. A scope is an item or block.
Expand Down
12 changes: 11 additions & 1 deletion src/librustdoc/core.rs
Expand Up @@ -26,7 +26,7 @@ use rustc_metadata::creader::CrateLoader;
use rustc_metadata::cstore::CStore;
use rustc_target::spec::TargetTriple;

use syntax::ast::{self, Ident};
use syntax::ast::{self, Ident, NodeId};
use syntax::source_map;
use syntax::edition::Edition;
use syntax::feature_gate::UnstableFeatures;
Expand Down Expand Up @@ -163,6 +163,16 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocContext<'a, 'tcx, 'rcx, 'cstore> {
def_id.clone()
}

/// Like the function of the same name on the HIR map, but skips calling it on fake DefIds.
/// (This avoids a slice-index-out-of-bounds panic.)
pub fn as_local_node_id(&self, def_id: DefId) -> Option<NodeId> {
if self.all_fake_def_ids.borrow().contains(&def_id) {
None
} else {
self.tcx.hir.as_local_node_id(def_id)
}
}

pub fn get_real_ty<F>(&self,
def_id: DefId,
def_ctor: &F,
Expand Down
60 changes: 44 additions & 16 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Expand Up @@ -69,16 +69,21 @@ impl<'a, 'tcx, 'rcx, 'cstore> LinkCollector<'a, 'tcx, 'rcx, 'cstore> {
/// Resolve a given string as a path, along with whether or not it is
/// in the value namespace. Also returns an optional URL fragment in the case
/// of variants and methods
fn resolve(&self, path_str: &str, is_val: bool, current_item: &Option<String>)
fn resolve(&self,
path_str: &str,
is_val: bool,
current_item: &Option<String>,
parent_id: Option<NodeId>)
-> Result<(Def, Option<String>), ()>
{
let cx = self.cx;

// In case we're in a module, try to resolve the relative
// path
if let Some(id) = self.mod_ids.last() {
if let Some(id) = parent_id.or(self.mod_ids.last().cloned()) {
// FIXME: `with_scope` requires the NodeId of a module
let result = cx.resolver.borrow_mut()
.with_scope(*id,
.with_scope(id,
|resolver| {
resolver.resolve_str_path_error(DUMMY_SP,
&path_str, is_val)
Expand Down Expand Up @@ -129,8 +134,9 @@ impl<'a, 'tcx, 'rcx, 'cstore> LinkCollector<'a, 'tcx, 'rcx, 'cstore> {
}
}

// FIXME: `with_scope` requires the NodeId of a module
let ty = cx.resolver.borrow_mut()
.with_scope(*id,
.with_scope(id,
|resolver| {
resolver.resolve_str_path_error(DUMMY_SP, &path, false)
})?;
Expand Down Expand Up @@ -218,6 +224,20 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor
None
};

// FIXME: get the resolver to work with non-local resolve scopes
let parent_node = self.cx.as_local_node_id(item.def_id).and_then(|node_id| {
// FIXME: this fails hard for impls in non-module scope, but is necessary for the
// current resolve() implementation
match self.cx.tcx.hir.get_module_parent_node(node_id) {
id if id != node_id => Some(id),
_ => None,
}
});

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

let current_item = match item.inner {
ModuleItem(..) => {
if item.attrs.inner_docs {
Expand All @@ -227,10 +247,10 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor
None
}
} else {
match self.mod_ids.last() {
Some(parent) if *parent != NodeId::new(0) => {
match parent_node.or(self.mod_ids.last().cloned()) {
Some(parent) if parent != NodeId::new(0) => {
//FIXME: can we pull the parent module's name from elsewhere?
Some(self.cx.tcx.hir.name(*parent).to_string())
Some(self.cx.tcx.hir.name(parent).to_string())
}
_ => None,
}
Expand Down Expand Up @@ -294,7 +314,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor

match kind {
PathKind::Value => {
if let Ok(def) = self.resolve(path_str, true, &current_item) {
if let Ok(def) = self.resolve(path_str, true, &current_item, parent_node) {
def
} else {
resolution_failure(cx, &item.attrs, path_str, &dox, link_range);
Expand All @@ -305,7 +325,7 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor
}
}
PathKind::Type => {
if let Ok(def) = self.resolve(path_str, false, &current_item) {
if let Ok(def) = self.resolve(path_str, false, &current_item, parent_node) {
def
} else {
resolution_failure(cx, &item.attrs, path_str, &dox, link_range);
Expand All @@ -316,16 +336,18 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor
PathKind::Unknown => {
// try everything!
if let Some(macro_def) = macro_resolve(cx, path_str) {
if let Ok(type_def) = self.resolve(path_str, false, &current_item) {
if let Ok(type_def) =
self.resolve(path_str, false, &current_item, parent_node)
{
let (type_kind, article, type_disambig)
= type_ns_kind(type_def.0, path_str);
ambiguity_error(cx, &item.attrs, path_str,
article, type_kind, &type_disambig,
"a", "macro", &format!("macro@{}", path_str));
continue;
} else if let Ok(value_def) = self.resolve(path_str,
true,
&current_item) {
} else if let Ok(value_def) =
self.resolve(path_str, true, &current_item, parent_node)
{
let (value_kind, value_disambig)
= value_ns_kind(value_def.0, path_str)
.expect("struct and mod cases should have been \
Expand All @@ -335,12 +357,16 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor
"a", "macro", &format!("macro@{}", path_str));
}
(macro_def, None)
} else if let Ok(type_def) = self.resolve(path_str, false, &current_item) {
} else if let Ok(type_def) =
self.resolve(path_str, false, &current_item, parent_node)
{
// It is imperative we search for not-a-value first
// Otherwise we will find struct ctors for when we are looking
// for structs, and the link won't work.
// if there is something in both namespaces
if let Ok(value_def) = self.resolve(path_str, true, &current_item) {
if let Ok(value_def) =
self.resolve(path_str, true, &current_item, parent_node)
{
let kind = value_ns_kind(value_def.0, path_str);
if let Some((value_kind, value_disambig)) = kind {
let (type_kind, article, type_disambig)
Expand All @@ -352,7 +378,9 @@ impl<'a, 'tcx, 'rcx, 'cstore> DocFolder for LinkCollector<'a, 'tcx, 'rcx, 'cstor
}
}
type_def
} else if let Ok(value_def) = self.resolve(path_str, true, &current_item) {
} else if let Ok(value_def) =
self.resolve(path_str, true, &current_item, parent_node)
{
value_def
} else {
resolution_failure(cx, &item.attrs, path_str, &dox, link_range);
Expand Down
40 changes: 40 additions & 0 deletions src/test/rustdoc/intra-link-in-bodies.rs
@@ -0,0 +1,40 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// we need to make sure that intra-doc links on trait impls get resolved in the right scope

#![deny(intra_doc_link_resolution_failure)]

pub mod inner {
pub struct SomethingOutOfScope;
}

pub mod other {
use inner::SomethingOutOfScope;
use SomeTrait;

pub struct OtherStruct;

/// Let's link to [SomethingOutOfScope] while we're at it.
impl SomeTrait for OtherStruct {}
}

pub trait SomeTrait {}

pub struct SomeStruct;

fn __implementation_details() {
use inner::SomethingOutOfScope;

// FIXME: intra-links resolve in their nearest module scope, not their actual scope in cases
// like this
// Let's link to [SomethingOutOfScope] while we're at it.
impl SomeTrait for SomeStruct {}
}

0 comments on commit 1106577

Please sign in to comment.