Auto merge of #34752 - ollie27:rustdoc_search, r=GuillaumeGomez

bors · web-flow · commit 4a12a70a5c51 · 2016-07-13T07:27:43.000-07:00
rustdoc: Fix methods in seach results Currently methods from extern crates are sometimes added to the search index when they shouldn't be or added with the original path rather than the reexported path. This fixes that by making sure `cache().paths` only contains local paths like the description for it states. It also fixes a few minor issues with link rendering and redirect generation which would point to local crate docs even if the docs for that crate hadn't been generated. Also a bug with methods implemented on traits which caused wrong paths and so dead links in the search results has been fixed. For example: [before](https://doc.rust-lang.org/nightly/std/?search=is_disjoint) [after](https://ollie27.github.io/rust_doc_test/std/?search=is_disjoint) [before](https://doc.rust-lang.org/nightly/std/?search=map_or) [after](https://ollie27.github.io/rust_doc_test/std/?search=map_or) [before](https://doc.rust-lang.org/nightly/std/?search=unsafecell%3A%3Anew) [after](https://ollie27.github.io/rust_doc_test/std/?search=unsafecell%3A%3Anew) [before](https://doc.rust-lang.org/nightly/std/?search=rng%3A%3Agen_) [after](https://ollie27.github.io/rust_doc_test/std/?search=rng%3A%3Agen_) [before](https://doc.rust-lang.org/nightly/std/?search=downcast_ref) [after](https://ollie27.github.io/rust_doc_test/std/?search=downcast_ref) Fixes #20246
diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs
@@ -19,7 +19,7 @@ use std::fmt;
 use std::iter::repeat;
 
 use rustc::middle::cstore::LOCAL_CRATE;
-use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
+use rustc::hir::def_id::DefId;
 use syntax::abi::Abi;
 use rustc::hir;
 
@@ -301,18 +301,19 @@ pub fn href(did: DefId) -> Option<(String, ItemType, Vec<String>)> {
     }
 
     let loc = CURRENT_LOCATION_KEY.with(|l| l.borrow().clone());
-    let &(ref fqp, shortty) = match cache.paths.get(&did) {
-        Some(p) => p,
-        None => return None,
-    };
-
-    let mut url = if did.is_local() || cache.inlined.contains(&did) {
-        repeat("../").take(loc.len()).collect::<String>()
-    } else {
-        match cache.extern_locations[&did.krate] {
-            (_, render::Remote(ref s)) => s.to_string(),
-            (_, render::Local) => repeat("../").take(loc.len()).collect(),
-            (_, render::Unknown) => return None,
+    let (fqp, shortty, mut url) = match cache.paths.get(&did) {
+        Some(&(ref fqp, shortty)) => {
+            (fqp, shortty, repeat("../").take(loc.len()).collect())
+        }
+        None => match cache.external_paths.get(&did) {
+            Some(&(ref fqp, shortty)) => {
+                (fqp, shortty, match cache.extern_locations[&did.krate] {
+                    (_, render::Remote(ref s)) => s.to_string(),
+                    (_, render::Local) => repeat("../").take(loc.len()).collect(),
+                    (_, render::Unknown) => return None,
+                })
+            }
+            None => return None,
         }
     };
     for component in &fqp[..fqp.len() - 1] {
@@ -387,22 +388,18 @@ fn primitive_link(f: &mut fmt::Formatter,
             needs_termination = true;
         }
         Some(&cnum) => {
-            let path = &m.paths[&DefId {
-                krate: cnum,
-                index: CRATE_DEF_INDEX,
-            }];
             let loc = match m.extern_locations[&cnum] {
-                (_, render::Remote(ref s)) => Some(s.to_string()),
-                (_, render::Local) => {
+                (ref cname, render::Remote(ref s)) => Some((cname, s.to_string())),
+                (ref cname, render::Local) => {
                     let len = CURRENT_LOCATION_KEY.with(|s| s.borrow().len());
-                    Some(repeat("../").take(len).collect::<String>())
+                    Some((cname, repeat("../").take(len).collect::<String>()))
                 }
                 (_, render::Unknown) => None,
             };
-            if let Some(root) = loc {
+            if let Some((cname, root)) = loc {
                 write!(f, "<a class='primitive' href='{}{}/primitive.{}.html'>",
                        root,
-                       path.0.first().unwrap(),
+                       cname,
                        prim.to_url_str())?;
                 needs_termination = true;
             }
diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs
@@ -230,7 +230,7 @@ pub struct Cache {
 
     /// Similar to `paths`, but only holds external paths. This is only used for
     /// generating explicit hyperlinks to other crates.
-    pub external_paths: HashMap<DefId, Vec<String>>,
+    pub external_paths: HashMap<DefId, (Vec<String>, ItemType)>,
 
     /// This map contains information about all known traits of this crate.
     /// Implementations of a crate should inherit the documentation of the
@@ -249,9 +249,6 @@ pub struct Cache {
     /// Cache of where documentation for primitives can be found.
     pub primitive_locations: HashMap<clean::PrimitiveType, ast::CrateNum>,
 
-    /// Set of definitions which have been inlined from external crates.
-    pub inlined: HashSet<DefId>,
-
     // Note that external items for which `doc(hidden)` applies to are shown as
     // non-reachable while local items aren't. This is because we're reusing
     // the access levels from crateanalysis.
@@ -505,20 +502,20 @@ pub fn run(mut krate: clean::Crate,
 
     // Crawl the crate to build various caches used for the output
     let RenderInfo {
-        inlined,
+        inlined: _,
         external_paths,
         external_typarams,
         deref_trait_did,
     } = renderinfo;
 
-    let paths = external_paths.into_iter()
-                              .map(|(k, (v, t))| (k, (v, ItemType::from_type_kind(t))))
-                              .collect::<HashMap<_, _>>();
+    let external_paths = external_paths.into_iter()
+        .map(|(k, (v, t))| (k, (v, ItemType::from_type_kind(t))))
+        .collect();
 
     let mut cache = Cache {
         impls: HashMap::new(),
-        external_paths: paths.iter().map(|(&k, v)| (k, v.0.clone())).collect(),
-        paths: paths,
+        external_paths: external_paths,
+        paths: HashMap::new(),
         implementors: HashMap::new(),
         stack: Vec::new(),
         parent_stack: Vec::new(),
@@ -534,15 +531,14 @@ pub fn run(mut krate: clean::Crate,
         traits: mem::replace(&mut krate.external_traits, HashMap::new()),
         deref_trait_did: deref_trait_did,
         typarams: external_typarams,
-        inlined: inlined,
     };
 
     // Cache where all our extern crates are located
     for &(n, ref e) in &krate.externs {
         cache.extern_locations.insert(n, (e.name.clone(),
                                           extern_location(e, &cx.dst)));
         let did = DefId { krate: n, index: CRATE_DEF_INDEX };
-        cache.paths.insert(did, (vec![e.name.to_string()], ItemType::Module));
+        cache.external_paths.insert(did, (vec![e.name.to_string()], ItemType::Module));
     }
 
     // Cache where all known primitives have their documentation located.
@@ -753,7 +749,10 @@ fn write_shared(cx: &Context,
         //        theory it should be...
         let &(ref remote_path, remote_item_type) = match cache.paths.get(&did) {
             Some(p) => p,
-            None => continue,
+            None => match cache.external_paths.get(&did) {
+                Some(p) => p,
+                None => continue,
+            }
         };
 
         let mut mydst = dst.clone();
@@ -1055,12 +1054,11 @@ impl DocFolder for Cache {
                         let last = self.parent_stack.last().unwrap();
                         let did = *last;
                         let path = match self.paths.get(&did) {
-                            Some(&(_, ItemType::Trait)) =>
-                                Some(&self.stack[..self.stack.len() - 1]),
                             // The current stack not necessarily has correlation
                             // for where the type was defined. On the other
                             // hand, `paths` always has the right
                             // information if present.
+                            Some(&(ref fqp, ItemType::Trait)) |
                             Some(&(ref fqp, ItemType::Struct)) |
                             Some(&(ref fqp, ItemType::Enum)) =>
                                 Some(&fqp[..fqp.len() - 1]),
@@ -1092,12 +1090,10 @@ impl DocFolder for Cache {
                         });
                     }
                 }
-                (Some(parent), None) if is_method || (!self.stripped_mod)=> {
-                    if parent.is_local() {
-                        // We have a parent, but we don't know where they're
-                        // defined yet. Wait for later to index this item.
-                        self.orphan_methods.push((parent, item.clone()))
-                    }
+                (Some(parent), None) if is_method => {
+                    // We have a parent, but we don't know where they're
+                    // defined yet. Wait for later to index this item.
+                    self.orphan_methods.push((parent, item.clone()));
                 }
                 _ => {}
             }
@@ -1127,7 +1123,6 @@ impl DocFolder for Cache {
                 // not a public item.
                 if
                     !self.paths.contains_key(&item.def_id) ||
-                    !item.def_id.is_local() ||
                     self.access_levels.is_public(item.def_id)
                 {
                     self.paths.insert(item.def_id,
@@ -1521,7 +1516,7 @@ impl<'a> Item<'a> {
         } else {
             let cache = cache();
             let external_path = match cache.external_paths.get(&self.item.def_id) {
-                Some(path) => path,
+                Some(&(ref path, _)) => path,
                 None => return None,
             };
             let mut path = match cache.extern_locations.get(&self.item.def_id.krate) {
@@ -2106,7 +2101,7 @@ fn item_trait(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item,
            path = if it.def_id.is_local() {
                cx.current.join("/")
            } else {
-               let path = &cache.external_paths[&it.def_id];
+               let (ref path, _) = cache.external_paths[&it.def_id];
                path[..path.len() - 1].join("/")
            },
            ty = shortty(it).to_static_str(),
diff --git a/src/test/rustdoc/auxiliary/extern-links.rs b/src/test/rustdoc/auxiliary/extern-links.rs
@@ -0,0 +1,11 @@
+// Copyright 2016 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.
+
+pub struct Foo;
diff --git a/src/test/rustdoc/extern-links.rs b/src/test/rustdoc/extern-links.rs
@@ -0,0 +1,31 @@
+// Copyright 2016 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.
+
+// aux-build:extern-links.rs
+// ignore-cross-compile
+
+#![crate_name = "foo"]
+
+extern crate extern_links;
+
+// @!has foo/index.html '//a' 'extern_links'
+#[doc(no_inline)]
+pub use extern_links as extern_links2;
+
+// @!has foo/index.html '//a' 'Foo'
+#[doc(no_inline)]
+pub use extern_links::Foo;
+
+#[doc(hidden)]
+pub mod hidden {
+    // @!has foo/hidden/extern_links/index.html
+    // @!has foo/hidden/extern_links/struct.Foo.html
+    pub use extern_links;
+}
