fix intra-links for trait impls

QuietMisdreavus · QuietMisdreavus · commit 110657711605 · 2018-09-20T05:54:31.000-05:00
diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs
@@ -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.
diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs
@@ -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;
@@ -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,
diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -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)
@@ -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)
             })?;
@@ -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 {
@@ -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,
                     }
@@ -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);
@@ -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);
@@ -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 \
@@ -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)
@@ -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);
diff --git a/src/test/rustdoc/intra-link-in-bodies.rs 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 <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 {}
+}
