Skip to content

Commit b905464

Browse files
ollie27ollie27
committed
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.
1 parent 5c69a4f commit b905464

File tree

4 files changed

+80
-46
lines changed

4 files changed

+80
-46
lines changed

‎src/librustdoc/html/format.rs

+19-22
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use std::fmt;
1919
use std::iter::repeat;
2020

2121
use rustc::middle::cstore::LOCAL_CRATE;
22-
use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
22+
use rustc::hir::def_id::DefId;
2323
use syntax::abi::Abi;
2424
use rustc::hir;
2525

@@ -301,18 +301,19 @@ pub fn href(did: DefId) -> Option<(String, ItemType, Vec<String>)> {
301301
}
302302

303303
let loc = CURRENT_LOCATION_KEY.with(|l| l.borrow().clone());
304-
let &(ref fqp, shortty) = match cache.paths.get(&did) {
305-
Some(p) => p,
306-
None => return None,
307-
};
308-
309-
let mut url = if did.is_local() || cache.inlined.contains(&did) {
310-
repeat("../").take(loc.len()).collect::<String>()
311-
} else {
312-
match cache.extern_locations[&did.krate] {
313-
(_, render::Remote(ref s)) => s.to_string(),
314-
(_, render::Local) => repeat("../").take(loc.len()).collect(),
315-
(_, render::Unknown) => return None,
304+
let (fqp, shortty, mut url) = match cache.paths.get(&did) {
305+
Some(&(ref fqp, shortty)) => {
306+
(fqp, shortty, repeat("../").take(loc.len()).collect())
307+
}
308+
None => match cache.external_paths.get(&did) {
309+
Some(&(ref fqp, shortty)) => {
310+
(fqp, shortty, match cache.extern_locations[&did.krate] {
311+
(_, render::Remote(ref s)) => s.to_string(),
312+
(_, render::Local) => repeat("../").take(loc.len()).collect(),
313+
(_, render::Unknown) => return None,
314+
})
315+
}
316+
None => return None,
316317
}
317318
};
318319
for component in &fqp[..fqp.len() - 1] {
@@ -387,22 +388,18 @@ fn primitive_link(f: &mut fmt::Formatter,
387388
needs_termination = true;
388389
}
389390
Some(&cnum) => {
390-
let path = &m.paths[&DefId {
391-
krate: cnum,
392-
index: CRATE_DEF_INDEX,
393-
}];
394391
let loc = match m.extern_locations[&cnum] {
395-
(_, render::Remote(ref s)) => Some(s.to_string()),
396-
(_, render::Local) => {
392+
(ref cname, render::Remote(ref s)) => Some((cname, s.to_string())),
393+
(ref cname, render::Local) => {
397394
let len = CURRENT_LOCATION_KEY.with(|s| s.borrow().len());
398-
Some(repeat("../").take(len).collect::<String>())
395+
Some((cname, repeat("../").take(len).collect::<String>()))
399396
}
400397
(_, render::Unknown) => None,
401398
};
402-
if let Some(root) = loc {
399+
if let Some((cname, root)) = loc {
403400
write!(f, "<a class='primitive' href='{}{}/primitive.{}.html'>",
404401
root,
405-
path.0.first().unwrap(),
402+
cname,
406403
prim.to_url_str())?;
407404
needs_termination = true;
408405
}

‎src/librustdoc/html/render.rs

+19-24
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ pub struct Cache {
230230

231231
/// Similar to `paths`, but only holds external paths. This is only used for
232232
/// generating explicit hyperlinks to other crates.
233-
pub external_paths: HashMap<DefId, Vec<String>>,
233+
pub external_paths: HashMap<DefId, (Vec<String>, ItemType)>,
234234

235235
/// This map contains information about all known traits of this crate.
236236
/// Implementations of a crate should inherit the documentation of the
@@ -249,9 +249,6 @@ pub struct Cache {
249249
/// Cache of where documentation for primitives can be found.
250250
pub primitive_locations: HashMap<clean::PrimitiveType, ast::CrateNum>,
251251

252-
/// Set of definitions which have been inlined from external crates.
253-
pub inlined: HashSet<DefId>,
254-
255252
// Note that external items for which `doc(hidden)` applies to are shown as
256253
// non-reachable while local items aren't. This is because we're reusing
257254
// the access levels from crateanalysis.
@@ -505,20 +502,20 @@ pub fn run(mut krate: clean::Crate,
505502

506503
// Crawl the crate to build various caches used for the output
507504
let RenderInfo {
508-
inlined,
505+
inlined: _,
509506
external_paths,
510507
external_typarams,
511508
deref_trait_did,
512509
} = renderinfo;
513510

514-
let paths = external_paths.into_iter()
515-
.map(|(k, (v, t))| (k, (v, ItemType::from_type_kind(t))))
516-
.collect::<HashMap<_, _>>();
511+
let external_paths = external_paths.into_iter()
512+
.map(|(k, (v, t))| (k, (v, ItemType::from_type_kind(t))))
513+
.collect();
517514

518515
let mut cache = Cache {
519516
impls: HashMap::new(),
520-
external_paths: paths.iter().map(|(&k, v)| (k, v.0.clone())).collect(),
521-
paths: paths,
517+
external_paths: external_paths,
518+
paths: HashMap::new(),
522519
implementors: HashMap::new(),
523520
stack: Vec::new(),
524521
parent_stack: Vec::new(),
@@ -534,15 +531,14 @@ pub fn run(mut krate: clean::Crate,
534531
traits: mem::replace(&mut krate.external_traits, HashMap::new()),
535532
deref_trait_did: deref_trait_did,
536533
typarams: external_typarams,
537-
inlined: inlined,
538534
};
539535

540536
// Cache where all our extern crates are located
541537
for &(n, ref e) in &krate.externs {
542538
cache.extern_locations.insert(n, (e.name.clone(),
543539
extern_location(e, &cx.dst)));
544540
let did = DefId { krate: n, index: CRATE_DEF_INDEX };
545-
cache.paths.insert(did, (vec![e.name.to_string()], ItemType::Module));
541+
cache.external_paths.insert(did, (vec![e.name.to_string()], ItemType::Module));
546542
}
547543

548544
// Cache where all known primitives have their documentation located.
@@ -753,7 +749,10 @@ fn write_shared(cx: &Context,
753749
// theory it should be...
754750
let &(ref remote_path, remote_item_type) = match cache.paths.get(&did) {
755751
Some(p) => p,
756-
None => continue,
752+
None => match cache.external_paths.get(&did) {
753+
Some(p) => p,
754+
None => continue,
755+
}
757756
};
758757

759758
let mut mydst = dst.clone();
@@ -1055,12 +1054,11 @@ impl DocFolder for Cache {
10551054
let last = self.parent_stack.last().unwrap();
10561055
let did = *last;
10571056
let path = match self.paths.get(&did) {
1058-
Some(&(_, ItemType::Trait)) =>
1059-
Some(&self.stack[..self.stack.len() - 1]),
10601057
// The current stack not necessarily has correlation
10611058
// for where the type was defined. On the other
10621059
// hand, `paths` always has the right
10631060
// information if present.
1061+
Some(&(ref fqp, ItemType::Trait)) |
10641062
Some(&(ref fqp, ItemType::Struct)) |
10651063
Some(&(ref fqp, ItemType::Enum)) =>
10661064
Some(&fqp[..fqp.len() - 1]),
@@ -1092,12 +1090,10 @@ impl DocFolder for Cache {
10921090
});
10931091
}
10941092
}
1095-
(Some(parent), None) if is_method || (!self.stripped_mod)=> {
1096-
if parent.is_local() {
1097-
// We have a parent, but we don't know where they're
1098-
// defined yet. Wait for later to index this item.
1099-
self.orphan_methods.push((parent, item.clone()))
1100-
}
1093+
(Some(parent), None) if is_method => {
1094+
// We have a parent, but we don't know where they're
1095+
// defined yet. Wait for later to index this item.
1096+
self.orphan_methods.push((parent, item.clone()));
11011097
}
11021098
_ => {}
11031099
}
@@ -1127,7 +1123,6 @@ impl DocFolder for Cache {
11271123
// not a public item.
11281124
if
11291125
!self.paths.contains_key(&item.def_id) ||
1130-
!item.def_id.is_local() ||
11311126
self.access_levels.is_public(item.def_id)
11321127
{
11331128
self.paths.insert(item.def_id,
@@ -1521,7 +1516,7 @@ impl<'a> Item<'a> {
15211516
} else {
15221517
let cache = cache();
15231518
let external_path = match cache.external_paths.get(&self.item.def_id) {
1524-
Some(path) => path,
1519+
Some(&(ref path, _)) => path,
15251520
None => return None,
15261521
};
15271522
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,
21062101
path = if it.def_id.is_local() {
21072102
cx.current.join("/")
21082103
} else {
2109-
let path = &cache.external_paths[&it.def_id];
2104+
let (ref path, _) = cache.external_paths[&it.def_id];
21102105
path[..path.len() - 1].join("/")
21112106
},
21122107
ty = shortty(it).to_static_str(),

‎src/test/rustdoc/auxiliary/extern-links.rs

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
pub struct Foo;

‎src/test/rustdoc/extern-links.rs

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// aux-build:extern-links.rs
12+
// ignore-cross-compile
13+
14+
#![crate_name = "foo"]
15+
16+
extern crate extern_links;
17+
18+
// @!has foo/index.html '//a' 'extern_links'
19+
#[doc(no_inline)]
20+
pub use extern_links as extern_links2;
21+
22+
// @!has foo/index.html '//a' 'Foo'
23+
#[doc(no_inline)]
24+
pub use extern_links::Foo;
25+
26+
#[doc(hidden)]
27+
pub mod hidden {
28+
// @!has foo/hidden/extern_links/index.html
29+
// @!has foo/hidden/extern_links/struct.Foo.html
30+
pub use extern_links;
31+
}

0 commit comments

Comments
 (0)