From: QuietMisdreavus Date: Wed, 19 Sep 2018 14:28:49 +0000 (-0500) Subject: fix intra-links for trait impls X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=110657711605c439b0f175a8ba674c89f9d86e81;p=rust.git fix intra-links for trait impls --- diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 31fba3ad974..6e35755d9a4 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -709,17 +709,22 @@ pub fn get_parent(&self, id: NodeId) -> NodeId { } } - /// 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. diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index db6e0183804..e8f1733e532 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -26,7 +26,7 @@ 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; @@ -163,6 +163,16 @@ pub fn next_def_id(&self, crate_num: CrateNum) -> DefId { 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 { + 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(&self, def_id: DefId, def_ctor: &F, diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 2d5512c538f..7b2eb2259d6 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -69,16 +69,21 @@ fn new(cx: &'a DocContext<'a, 'tcx, 'rcx, 'cstore>) -> Self { /// 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) + fn resolve(&self, + path_str: &str, + is_val: bool, + current_item: &Option, + parent_id: Option) -> Result<(Def, Option), ()> { 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) @@ -129,8 +134,9 @@ fn resolve(&self, path_str: &str, is_val: bool, current_item: &Option) } } + // 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) })?; @@ -218,6 +224,20 @@ fn fold_item(&mut self, mut item: Item) -> Option { 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 { @@ -227,10 +247,10 @@ fn fold_item(&mut self, mut item: Item) -> Option { 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, } @@ -294,7 +314,7 @@ fn fold_item(&mut self, mut item: Item) -> Option { match kind { PathKind::Value => { - if let Ok(def) = self.resolve(path_str, true, ¤t_item) { + if let Ok(def) = self.resolve(path_str, true, ¤t_item, parent_node) { def } else { resolution_failure(cx, &item.attrs, path_str, &dox, link_range); @@ -305,7 +325,7 @@ fn fold_item(&mut self, mut item: Item) -> Option { } } PathKind::Type => { - if let Ok(def) = self.resolve(path_str, false, ¤t_item) { + if let Ok(def) = self.resolve(path_str, false, ¤t_item, parent_node) { def } else { resolution_failure(cx, &item.attrs, path_str, &dox, link_range); @@ -316,16 +336,18 @@ fn fold_item(&mut self, mut item: Item) -> Option { PathKind::Unknown => { // try everything! if let Some(macro_def) = macro_resolve(cx, path_str) { - if let Ok(type_def) = self.resolve(path_str, false, ¤t_item) { + if let Ok(type_def) = + self.resolve(path_str, false, ¤t_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, - ¤t_item) { + } else if let Ok(value_def) = + self.resolve(path_str, true, ¤t_item, parent_node) + { let (value_kind, value_disambig) = value_ns_kind(value_def.0, path_str) .expect("struct and mod cases should have been \ @@ -335,12 +357,16 @@ fn fold_item(&mut self, mut item: Item) -> Option { "a", "macro", &format!("macro@{}", path_str)); } (macro_def, None) - } else if let Ok(type_def) = self.resolve(path_str, false, ¤t_item) { + } else if let Ok(type_def) = + self.resolve(path_str, false, ¤t_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, ¤t_item) { + if let Ok(value_def) = + self.resolve(path_str, true, ¤t_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) @@ -352,7 +378,9 @@ fn fold_item(&mut self, mut item: Item) -> Option { } } type_def - } else if let Ok(value_def) = self.resolve(path_str, true, ¤t_item) { + } else if let Ok(value_def) = + self.resolve(path_str, true, ¤t_item, parent_node) + { value_def } else { resolution_failure(cx, &item.attrs, path_str, &dox, link_range); diff --git a/src/test/rustdoc/intra-link-in-bodies.rs b/src/test/rustdoc/intra-link-in-bodies.rs new file mode 100644 index 00000000000..8c01941234e --- /dev/null +++ b/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 or the MIT license +// , 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 {} +}