From 53447d81d05b75fb9923db08b6029381f691a9c2 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Fran=C3=A7ois=20Mockers?= Date: Sun, 27 Jun 2021 11:10:36 +0200 Subject: [PATCH] fix dead link for method in trait of blanket impl from third party crate --- src/librustdoc/clean/types.rs | 2 +- src/librustdoc/html/format.rs | 58 +++++++++++++++++++------------ src/librustdoc/html/render/mod.rs | 18 +++++----- 3 files changed, 47 insertions(+), 31 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index e716e09b8b3..859746b6a2d 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -459,7 +459,7 @@ pub fn from_def_id_and_attrs_and_parts( .filter_map(|ItemLink { link: s, link_text, did, ref fragment }| { match did { Some(did) => { - if let Some((mut href, ..)) = href(did.clone(), cx) { + if let Ok((mut href, ..)) = href(did.clone(), cx) { if let Some(ref fragment) = *fragment { href.push('#'); href.push_str(fragment); diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index c08fe47696b..bed4bae2492 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -472,7 +472,18 @@ fn print<'a, 'tcx: 'a>( } } -crate fn href(did: DefId, cx: &Context<'_>) -> Option<(String, ItemType, Vec)> { +// Possible errors when computing href link source for a `DefId` +crate enum HrefError { + // `DefId` is in an unknown location. This seems to happen when building without dependencies + // but a trait from a dependency is still visible + UnknownLocation, + // Unavailable because private + Unavailable, + // Not in external cache, href link should be in same page + NotInExternalCache, +} + +crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec), HrefError> { let cache = &cx.cache(); let relative_to = &cx.current; fn to_module_fqp(shortty: ItemType, fqp: &[String]) -> &[String] { @@ -480,7 +491,7 @@ fn to_module_fqp(shortty: ItemType, fqp: &[String]) -> &[String] { } if !did.is_local() && !cache.access_levels.is_public(did) && !cache.document_private { - return None; + return Err(HrefError::Unavailable); } let (fqp, shortty, mut url_parts) = match cache.paths.get(&did) { @@ -489,22 +500,25 @@ fn to_module_fqp(shortty: ItemType, fqp: &[String]) -> &[String] { href_relative_parts(module_fqp, relative_to) }), None => { - let &(ref fqp, shortty) = cache.external_paths.get(&did)?; - let module_fqp = to_module_fqp(shortty, fqp); - ( - fqp, - shortty, - match cache.extern_locations[&did.krate] { - ExternalLocation::Remote(ref s) => { - let s = s.trim_end_matches('/'); - let mut s = vec![&s[..]]; - s.extend(module_fqp[..].iter().map(String::as_str)); - s - } - ExternalLocation::Local => href_relative_parts(module_fqp, relative_to), - ExternalLocation::Unknown => return None, - }, - ) + if let Some(&(ref fqp, shortty)) = cache.external_paths.get(&did) { + let module_fqp = to_module_fqp(shortty, fqp); + ( + fqp, + shortty, + match cache.extern_locations[&did.krate] { + ExternalLocation::Remote(ref s) => { + let s = s.trim_end_matches('/'); + let mut s = vec![&s[..]]; + s.extend(module_fqp[..].iter().map(String::as_str)); + s + } + ExternalLocation::Local => href_relative_parts(module_fqp, relative_to), + ExternalLocation::Unknown => return Err(HrefError::UnknownLocation), + }, + ) + } else { + return Err(HrefError::NotInExternalCache); + } } }; let last = &fqp.last().unwrap()[..]; @@ -518,7 +532,7 @@ fn to_module_fqp(shortty: ItemType, fqp: &[String]) -> &[String] { url_parts.push(&filename); } } - Some((url_parts.join("/"), shortty, fqp.to_vec())) + Ok((url_parts.join("/"), shortty, fqp.to_vec())) } /// Both paths should only be modules. @@ -567,7 +581,7 @@ fn resolved_path<'a, 'cx: 'a>( write!(w, "{}{:#}", &last.name, last.args.print(cx))?; } else { let path = if use_absolute { - if let Some((_, _, fqp)) = href(did, cx) { + if let Ok((_, _, fqp)) = href(did, cx) { format!( "{}::{}", fqp[..fqp.len() - 1].join("::"), @@ -675,7 +689,7 @@ fn tybounds<'a, 'tcx: 'a>( ) -> impl fmt::Display + 'a { let parts = href(did.into(), cx); display_fn(move |f| { - if let Some((url, short_ty, fqp)) = parts { + if let Ok((url, short_ty, fqp)) = parts { write!( f, r#"{}"#, @@ -907,7 +921,7 @@ fn fmt_type<'cx>( // look at). box clean::ResolvedPath { did, .. } => { match href(did.into(), cx) { - Some((ref url, _, ref path)) if !f.alternate() => { + Ok((ref url, _, ref path)) if !f.alternate() => { write!( f, " format!("#{}", id), - AssocItemLink::Anchor(None) => format!("#{}.{}", meth.type_(), name), + AssocItemLink::Anchor(Some(ref id)) => Some(format!("#{}", id)), + AssocItemLink::Anchor(None) => Some(format!("#{}.{}", meth.type_(), name)), AssocItemLink::GotoSource(did, provided_methods) => { // We're creating a link from an impl-item to the corresponding // trait-item and need to map the anchored type accordingly. @@ -867,9 +867,11 @@ fn method( ItemType::TyMethod }; - href(did.expect_def_id(), cx) - .map(|p| format!("{}#{}.{}", p.0, ty, name)) - .unwrap_or_else(|| format!("#{}.{}", ty, name)) + match href(did.expect_def_id(), cx) { + Ok(p) => Some(format!("{}#{}.{}", p.0, ty, name)), + Err(HrefError::UnknownLocation) => None, + Err(_) => Some(format!("#{}.{}", ty, name)), + } } }; let vis = meth.visibility.print_with_space(meth.def_id, cx).to_string(); @@ -904,7 +906,7 @@ fn method( w.reserve(header_len + "{".len() + "".len()); write!( w, - "{indent}{vis}{constness}{asyncness}{unsafety}{defaultness}{abi}fn {name}\ + "{indent}{vis}{constness}{asyncness}{unsafety}{defaultness}{abi}fn {name}\ {generics}{decl}{notable_traits}{where_clause}", indent = indent_str, vis = vis, @@ -913,7 +915,7 @@ fn method( unsafety = unsafety, defaultness = defaultness, abi = abi, - href = href, + href = href.map(|href| format!("href=\"{}\"", href)).unwrap_or_else(|| "".to_string()), name = name, generics = g.print(cx), decl = d.full_print(header_len, indent, header.asyncness, cx), -- 2.44.0