mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #86662 - mockersf:fix-86620-link-unknown-location, r=jyn514

Browse Source 
fix dead link for method in trait of blanket impl from third party crate

fix #86620

* changes `href` method to raise the actual error it had instead of an `Option`
* set the href link correctly in case of an error

I did not manage to make a small reproducer, I think it happens in a situation where
* crate A expose a trait with a blanket impl
* crate B use the trait from crate A
* crate C use types from crate B
* building docs for crate C without dependencies

r? `@jyn514`
This commit is contained in:
bors 2021-07-16 06:44:10 +00:00
commit a6470c7fa8
6 changed files with 71 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/librustdoc/clean/types.rs
View File

@ -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);

59
src/librustdoc/html/format.rs
View File

@ -472,7 +472,19 @@ fn print<'a, 'tcx: 'a>(
}
}
crate fn href(did: DefId, cx: &Context<'_>) -> Option<(String, ItemType, Vec<String>)> {
// Possible errors when computing href link source for a `DefId`
crate enum HrefError {
/// This item is known to rustdoc, but from a crate that does not have documentation generated.
///
/// This can only happen for non-local items.
DocumentationNotBuilt,
/// This can only happen for non-local items when `--document-private-items` is not passed.
Private,
// Not in external cache, href link should be in same page
NotInExternalCache,
}
crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<String>), HrefError> {
let cache = &cx.cache();
let relative_to = &cx.current;
fn to_module_fqp(shortty: ItemType, fqp: &[String]) -> &[String] {
@ -480,7 +492,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::Private);
}
let (fqp, shortty, mut url_parts) = match cache.paths.get(&did) {
@ -489,22 +501,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::DocumentationNotBuilt),
},
)
} else {
return Err(HrefError::NotInExternalCache);
}
}
};
let last = &fqp.last().unwrap()[..];
@ -518,7 +533,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 +582,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 +690,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#"<a class="{}" href="{}" title="{} {}">{}</a>"#,
@ -907,7 +922,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,
"<a class=\"type\" href=\"{url}#{shortty}.{name}\" \

19
src/librustdoc/html/render/mod.rs
View File

@ -62,7 +62,7 @@
use crate::html::escape::Escape;
use crate::html::format::{
href, print_abi_with_space, print_constness_with_space, print_default_space,
print_generic_bounds, print_where_clause, Buffer, PrintWithSpace,
print_generic_bounds, print_where_clause, Buffer, HrefError, PrintWithSpace,
};
use crate::html::markdown::{Markdown, MarkdownHtml, MarkdownSummaryLine};
@ -856,8 +856,8 @@ fn method(
) {
let name = meth.name.as_ref().unwrap();
let href = match link {
AssocItemLink::Anchor(Some(ref id)) => 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), ty) {
(Ok(p), ty) => Some(format!("{}#{}.{}", p.0, ty, name)),
(Err(HrefError::DocumentationNotBuilt), ItemType::TyMethod) => None,
(Err(_), ty) => 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 + "<a href=\"\" class=\"fnname\">{".len() + "</a>".len());
write!(
w,
"{indent}{vis}{constness}{asyncness}{unsafety}{defaultness}{abi}fn <a href=\"{href}\" class=\"fnname\">{name}</a>\
"{indent}{vis}{constness}{asyncness}{unsafety}{defaultness}{abi}fn <a {href} class=\"fnname\">{name}</a>\
{generics}{decl}{notable_traits}{where_clause}",
indent = indent_str,
vis = vis,
@ -913,7 +915,8 @@ fn method(
unsafety = unsafety,
defaultness = defaultness,
abi = abi,
href = href,
// links without a href are valid - https://www.w3schools.com/tags/att_a_href.asp
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),

11
src/test/rustdoc/auxiliary/issue-86620-1.rs Normal file
View File

@ -0,0 +1,11 @@
#![crate_name = "issue_86620_1"]
pub trait VZip {
fn vzip() -> usize;
}
impl<T> VZip for T {
fn vzip() -> usize {
0
}
}

2
src/test/rustdoc/extern-default-method.rs
View File

@ -4,4 +4,6 @@
extern crate rustdoc_extern_default_method as ext;
// @count extern_default_method/struct.Struct.html '//*[@id="method.provided"]' 1
// @has extern_default_method/struct.Struct.html '//div[@id="method.provided"]//a[@class="fnname"]/@href' #method.provided
// @has extern_default_method/struct.Struct.html '//div[@id="method.provided"]//a[@class="anchor"]/@href' #method.provided
pub use ext::Struct;

9
src/test/rustdoc/issue-86620.rs Normal file
View File

@ -0,0 +1,9 @@
// aux-build:issue-86620-1.rs
extern crate issue_86620_1;
use issue_86620_1::*;
// @!has issue_86620/struct.S.html '//div[@id="method.vzip"]//a[@class="fnname"]/@href' #tymethod.vzip
// @has issue_86620/struct.S.html '//div[@id="method.vzip"]//a[@class="anchor"]/@href' #method.vzip
pub struct S;