Skip to content

Commit 53447d8

Browse files
committed
fix dead link for method in trait of blanket impl from third party crate
1 parent 238fd72 commit 53447d8

File tree

3 files changed

+47
-31
lines changed

3 files changed

+47
-31
lines changed

src/librustdoc/clean/types.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ impl Item {
459459
.filter_map(|ItemLink { link: s, link_text, did, ref fragment }| {
460460
match did {
461461
Some(did) => {
462-
if let Some((mut href, ..)) = href(did.clone(), cx) {
462+
if let Ok((mut href, ..)) = href(did.clone(), cx) {
463463
if let Some(ref fragment) = *fragment {
464464
href.push('#');
465465
href.push_str(fragment);

src/librustdoc/html/format.rs

+36-22
Original file line numberDiff line numberDiff line change
@@ -472,15 +472,26 @@ impl clean::GenericArgs {
472472
}
473473
}
474474

475-
crate fn href(did: DefId, cx: &Context<'_>) -> Option<(String, ItemType, Vec<String>)> {
475+
// Possible errors when computing href link source for a `DefId`
476+
crate enum HrefError {
477+
// `DefId` is in an unknown location. This seems to happen when building without dependencies
478+
// but a trait from a dependency is still visible
479+
UnknownLocation,
480+
// Unavailable because private
481+
Unavailable,
482+
// Not in external cache, href link should be in same page
483+
NotInExternalCache,
484+
}
485+
486+
crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<String>), HrefError> {
476487
let cache = &cx.cache();
477488
let relative_to = &cx.current;
478489
fn to_module_fqp(shortty: ItemType, fqp: &[String]) -> &[String] {
479490
if shortty == ItemType::Module { &fqp[..] } else { &fqp[..fqp.len() - 1] }
480491
}
481492

482493
if !did.is_local() && !cache.access_levels.is_public(did) && !cache.document_private {
483-
return None;
494+
return Err(HrefError::Unavailable);
484495
}
485496

486497
let (fqp, shortty, mut url_parts) = match cache.paths.get(&did) {
@@ -489,22 +500,25 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Option<(String, ItemType, Vec<Str
489500
href_relative_parts(module_fqp, relative_to)
490501
}),
491502
None => {
492-
let &(ref fqp, shortty) = cache.external_paths.get(&did)?;
493-
let module_fqp = to_module_fqp(shortty, fqp);
494-
(
495-
fqp,
496-
shortty,
497-
match cache.extern_locations[&did.krate] {
498-
ExternalLocation::Remote(ref s) => {
499-
let s = s.trim_end_matches('/');
500-
let mut s = vec![&s[..]];
501-
s.extend(module_fqp[..].iter().map(String::as_str));
502-
s
503-
}
504-
ExternalLocation::Local => href_relative_parts(module_fqp, relative_to),
505-
ExternalLocation::Unknown => return None,
506-
},
507-
)
503+
if let Some(&(ref fqp, shortty)) = cache.external_paths.get(&did) {
504+
let module_fqp = to_module_fqp(shortty, fqp);
505+
(
506+
fqp,
507+
shortty,
508+
match cache.extern_locations[&did.krate] {
509+
ExternalLocation::Remote(ref s) => {
510+
let s = s.trim_end_matches('/');
511+
let mut s = vec![&s[..]];
512+
s.extend(module_fqp[..].iter().map(String::as_str));
513+
s
514+
}
515+
ExternalLocation::Local => href_relative_parts(module_fqp, relative_to),
516+
ExternalLocation::Unknown => return Err(HrefError::UnknownLocation),
517+
},
518+
)
519+
} else {
520+
return Err(HrefError::NotInExternalCache);
521+
}
508522
}
509523
};
510524
let last = &fqp.last().unwrap()[..];
@@ -518,7 +532,7 @@ crate fn href(did: DefId, cx: &Context<'_>) -> Option<(String, ItemType, Vec<Str
518532
url_parts.push(&filename);
519533
}
520534
}
521-
Some((url_parts.join("/"), shortty, fqp.to_vec()))
535+
Ok((url_parts.join("/"), shortty, fqp.to_vec()))
522536
}
523537

524538
/// Both paths should only be modules.
@@ -567,7 +581,7 @@ fn resolved_path<'a, 'cx: 'a>(
567581
write!(w, "{}{:#}", &last.name, last.args.print(cx))?;
568582
} else {
569583
let path = if use_absolute {
570-
if let Some((_, _, fqp)) = href(did, cx) {
584+
if let Ok((_, _, fqp)) = href(did, cx) {
571585
format!(
572586
"{}::{}",
573587
fqp[..fqp.len() - 1].join("::"),
@@ -675,7 +689,7 @@ crate fn anchor<'a, 'cx: 'a>(
675689
) -> impl fmt::Display + 'a {
676690
let parts = href(did.into(), cx);
677691
display_fn(move |f| {
678-
if let Some((url, short_ty, fqp)) = parts {
692+
if let Ok((url, short_ty, fqp)) = parts {
679693
write!(
680694
f,
681695
r#"<a class="{}" href="{}" title="{} {}">{}</a>"#,
@@ -907,7 +921,7 @@ fn fmt_type<'cx>(
907921
// look at).
908922
box clean::ResolvedPath { did, .. } => {
909923
match href(did.into(), cx) {
910-
Some((ref url, _, ref path)) if !f.alternate() => {
924+
Ok((ref url, _, ref path)) if !f.alternate() => {
911925
write!(
912926
f,
913927
"<a class=\"type\" href=\"{url}#{shortty}.{name}\" \

src/librustdoc/html/render/mod.rs

+10-8
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ use crate::formats::{AssocItemRender, Impl, RenderMode};
6262
use crate::html::escape::Escape;
6363
use crate::html::format::{
6464
href, print_abi_with_space, print_constness_with_space, print_default_space,
65-
print_generic_bounds, print_where_clause, Buffer, PrintWithSpace,
65+
print_generic_bounds, print_where_clause, Buffer, HrefError, PrintWithSpace,
6666
};
6767
use crate::html::markdown::{Markdown, MarkdownHtml, MarkdownSummaryLine};
6868

@@ -856,8 +856,8 @@ fn render_assoc_item(
856856
) {
857857
let name = meth.name.as_ref().unwrap();
858858
let href = match link {
859-
AssocItemLink::Anchor(Some(ref id)) => format!("#{}", id),
860-
AssocItemLink::Anchor(None) => format!("#{}.{}", meth.type_(), name),
859+
AssocItemLink::Anchor(Some(ref id)) => Some(format!("#{}", id)),
860+
AssocItemLink::Anchor(None) => Some(format!("#{}.{}", meth.type_(), name)),
861861
AssocItemLink::GotoSource(did, provided_methods) => {
862862
// We're creating a link from an impl-item to the corresponding
863863
// trait-item and need to map the anchored type accordingly.
@@ -867,9 +867,11 @@ fn render_assoc_item(
867867
ItemType::TyMethod
868868
};
869869

870-
href(did.expect_def_id(), cx)
871-
.map(|p| format!("{}#{}.{}", p.0, ty, name))
872-
.unwrap_or_else(|| format!("#{}.{}", ty, name))
870+
match href(did.expect_def_id(), cx) {
871+
Ok(p) => Some(format!("{}#{}.{}", p.0, ty, name)),
872+
Err(HrefError::UnknownLocation) => None,
873+
Err(_) => Some(format!("#{}.{}", ty, name)),
874+
}
873875
}
874876
};
875877
let vis = meth.visibility.print_with_space(meth.def_id, cx).to_string();
@@ -904,7 +906,7 @@ fn render_assoc_item(
904906
w.reserve(header_len + "<a href=\"\" class=\"fnname\">{".len() + "</a>".len());
905907
write!(
906908
w,
907-
"{indent}{vis}{constness}{asyncness}{unsafety}{defaultness}{abi}fn <a href=\"{href}\" class=\"fnname\">{name}</a>\
909+
"{indent}{vis}{constness}{asyncness}{unsafety}{defaultness}{abi}fn <a {href} class=\"fnname\">{name}</a>\
908910
{generics}{decl}{notable_traits}{where_clause}",
909911
indent = indent_str,
910912
vis = vis,
@@ -913,7 +915,7 @@ fn render_assoc_item(
913915
unsafety = unsafety,
914916
defaultness = defaultness,
915917
abi = abi,
916-
href = href,
918+
href = href.map(|href| format!("href=\"{}\"", href)).unwrap_or_else(|| "".to_string()),
917919
name = name,
918920
generics = g.print(cx),
919921
decl = d.full_print(header_len, indent, header.asyncness, cx),

0 commit comments

Comments
 (0)