Rollup merge of rust-lang#73101 - jyn514:rustdoc-absolute-module, r=Manishearth

Manishearth · web-flow · commit dcd018044969 · 2020-07-16T15:56:02.000-07:00
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
diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
@@ -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
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
@@ -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 {
@@ -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))
diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs
@@ -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)
diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -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;
@@ -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> {
@@ -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;
 
@@ -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| {
@@ -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)
                 {
@@ -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)
             });
@@ -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,
                     }
@@ -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);
 
@@ -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;
@@ -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::") {
@@ -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() {
@@ -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;
         }
     };
diff --git a/src/test/rustdoc-ui/intra-links-private.rs b/src/test/rustdoc-ui/intra-links-private.rs
@@ -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
diff --git a/src/test/rustdoc/intra-doc-crate/additional_doc.rs b/src/test/rustdoc/intra-doc-crate/additional_doc.rs
@@ -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;
diff --git a/src/test/rustdoc/intra-doc-crate/auxiliary/additional_doc.rs b/src/test/rustdoc/intra-doc-crate/auxiliary/additional_doc.rs
@@ -0,0 +1,6 @@
+#![crate_name = "my_rand"]
+#![deny(intra_doc_link_resolution_failure)]
+
+pub trait RngCore {}
+/// Rng extends [`RngCore`].
+pub trait Rng: RngCore {}
diff --git a/src/test/rustdoc/intra-doc-crate/auxiliary/intra-doc-basic.rs b/src/test/rustdoc/intra-doc-crate/auxiliary/intra-doc-basic.rs
@@ -0,0 +1,7 @@
+#![crate_name = "a"]
+#![deny(intra_doc_link_resolution_failure)]
+
+pub struct Foo;
+
+/// Link to [Foo]
+pub struct Bar;
diff --git a/src/test/rustdoc/intra-doc-crate/auxiliary/macro_inner.rs b/src/test/rustdoc/intra-doc-crate/auxiliary/macro_inner.rs
@@ -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 {
+    () => {}
+}
diff --git a/src/test/rustdoc/intra-doc-crate/auxiliary/module.rs b/src/test/rustdoc/intra-doc-crate/auxiliary/module.rs
@@ -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 {}
diff --git a/src/test/rustdoc/intra-doc-crate/auxiliary/proc_macro.rs b/src/test/rustdoc/intra-doc-crate/auxiliary/proc_macro.rs
@@ -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
+}
diff --git a/src/test/rustdoc/intra-doc-crate/auxiliary/submodule-inner.rs b/src/test/rustdoc/intra-doc-crate/auxiliary/submodule-inner.rs
@@ -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;
+}
diff --git a/src/test/rustdoc/intra-doc-crate/auxiliary/submodule-outer.rs b/src/test/rustdoc/intra-doc-crate/auxiliary/submodule-outer.rs
@@ -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 {
+}
diff --git a/src/test/rustdoc/intra-doc-crate/auxiliary/traits.rs b/src/test/rustdoc/intra-doc-crate/auxiliary/traits.rs
@@ -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() {}
+    }
+}
diff --git a/src/test/rustdoc/intra-doc-crate/basic.rs b/src/test/rustdoc/intra-doc-crate/basic.rs
@@ -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;
diff --git a/src/test/rustdoc/intra-doc-crate/macro.rs b/src/test/rustdoc/intra-doc-crate/macro.rs
@@ -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;
diff --git a/src/test/rustdoc/intra-doc-crate/module.rs b/src/test/rustdoc/intra-doc-crate/module.rs
@@ -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;
diff --git a/src/test/rustdoc/intra-doc-crate/submodule-inner.rs b/src/test/rustdoc/intra-doc-crate/submodule-inner.rs
diff --git a/src/test/rustdoc/intra-doc-crate/submodule-outer.rs b/src/test/rustdoc/intra-doc-crate/submodule-outer.rs
diff --git a/src/test/rustdoc/intra-doc-crate/traits.rs b/src/test/rustdoc/intra-doc-crate/traits.rs
diff --git a/src/test/rustdoc/intra-link-prim-precedence.rs b/src/test/rustdoc/intra-link-prim-precedence.rs

Original file line number	Diff line number	Diff line change
`@@ -430,7 +430,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt`
`430`	`430`	`DUMMY_SP,`
`431`	`431`	`extern_name,`
`432`	`432`	`TypeNS,`
`433`		`- LocalDefId { local_def_index: CRATE_DEF_INDEX },`
	`433`	`+ LocalDefId { local_def_index: CRATE_DEF_INDEX }.to_def_id(),`
`434`	`434`	`)`
`435`	`435`	`.unwrap_or_else(\|()\| {`
`436`	`436`	`panic!("Unable to resolve external crate {}", extern_name)`