Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for intra-doc-links #47701

Merged
merged 7 commits into from
Jan 26, 2018
Merged

Fixes for intra-doc-links #47701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f7ba00c
Turn error into warning
Manishearth Jan 24, 2018
6b26b74
Return Err() if resolve() is called before modules are set up
Manishearth Jan 24, 2018
f0e7677
Return Def from resolve()
Manishearth Jan 24, 2018
5a89f40
Handle methods
Manishearth Jan 24, 2018
a4d3692
Make it work for traits
Manishearth Jan 24, 2018
c26f887
Handle variants
Manishearth Jan 24, 2018
08ca4fd
Add tests
Manishearth Jan 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
179 changes: 138 additions & 41 deletions src/librustdoc/clean/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,8 @@ pub struct Attributes {
pub other_attrs: Vec<ast::Attribute>,
pub cfg: Option<Rc<Cfg>>,
pub span: Option<syntax_pos::Span>,
pub links: Vec<(String, DefId)>,
/// map from Rust paths to resolved defs and potential URL fragments
pub links: Vec<(String, DefId, Option<String>)>,
}

impl Attributes {
Expand Down Expand Up @@ -820,8 +821,12 @@ impl Attributes {
/// Cache must be populated before call
pub fn links(&self) -> Vec<(String, String)> {
use html::format::href;
self.links.iter().filter_map(|&(ref s, did)| {
if let Some((href, ..)) = href(did) {
self.links.iter().filter_map(|&(ref s, did, ref fragment)| {
if let Some((mut href, ..)) = href(did) {
if let Some(ref fragment) = *fragment {
href.push_str("#");
href.push_str(fragment);
}
Some((s.clone(), href))
} else {
None
Expand All @@ -843,10 +848,8 @@ impl AttributesExt for Attributes {
/// they exist in both namespaces (structs and modules)
fn value_ns_kind(def: Def, path_str: &str) -> Option<(&'static str, String)> {
match def {
// structs and mods exist in both namespaces. skip them
Def::StructCtor(..) | Def::Mod(..) => None,
Def::Variant(..) | Def::VariantCtor(..)
=> Some(("variant", format!("{}()", path_str))),
// structs, variants, and mods exist in both namespaces. skip them
Def::StructCtor(..) | Def::Mod(..) | Def::Variant(..) | Def::VariantCtor(..) => None,
Def::Fn(..)
=> Some(("function", format!("{}()", path_str))),
Def::Method(..)
Expand Down Expand Up @@ -880,10 +883,10 @@ fn ambiguity_error(cx: &DocContext, attrs: &Attributes,
let sp = attrs.doc_strings.first()
.map_or(DUMMY_SP, |a| a.span());
cx.sess()
.struct_span_err(sp,
&format!("`{}` is both {} {} and {} {}",
path_str, article1, kind1,
article2, kind2))
.struct_span_warn(sp,
&format!("`{}` is both {} {} and {} {}",
path_str, article1, kind1,
article2, kind2))
.help(&format!("try `{}` if you want to select the {}, \
or `{}` if you want to \
select the {}",
Expand All @@ -892,21 +895,114 @@ fn ambiguity_error(cx: &DocContext, attrs: &Attributes,
.emit();
}

/// Given an enum variant's def, return the def of its enum and the associated fragment
fn handle_variant(cx: &DocContext, def: Def) -> Result<(Def, Option<String>), ()> {
use rustc::ty::DefIdTree;

let parent = if let Some(parent) = cx.tcx.parent(def.def_id()) {
parent
} else {
return Err(())
};
let parent_def = Def::Enum(parent);
let variant = cx.tcx.expect_variant_def(def);
Ok((parent_def, Some(format!("{}.v", variant.name))))
}

/// Resolve a given string as a path, along with whether or not it is
/// in the value namespace
fn resolve(cx: &DocContext, path_str: &str, is_val: bool) -> Result<hir::Path, ()> {
/// in the value namespace. Also returns an optional URL fragment in the case
/// of variants and methods
fn resolve(cx: &DocContext, path_str: &str, is_val: bool) -> Result<(Def, Option<String>), ()> {
// In case we're in a module, try to resolve the relative
// path
if let Some(id) = cx.mod_ids.borrow().last() {
cx.resolver.borrow_mut()
.with_scope(*id, |resolver| {
resolver.resolve_str_path_error(DUMMY_SP,
&path_str, is_val)
})
let result = cx.resolver.borrow_mut()
.with_scope(*id,
|resolver| {
resolver.resolve_str_path_error(DUMMY_SP,
&path_str, is_val)
});

if let Ok(result) = result {
// In case this is a trait item, skip the
// early return and try looking for the trait
let value = match result.def {
Def::Method(_) | Def::AssociatedConst(_) => true,
Def::AssociatedTy(_) => false,
Def::Variant(_) => return handle_variant(cx, result.def),
// not a trait item, just return what we found
_ => return Ok((result.def, None))
};

if value != is_val {
return Err(())
}
} else {
// If resolution failed, it may still be a method
// because methods are not handled by the resolver
// If so, bail when we're not looking for a value
if !is_val {
return Err(())
}
}

// Try looking for methods and associated items
let mut split = path_str.rsplitn(2, "::");
let mut item_name = if let Some(first) = split.next() {
first
} else {
return Err(())
};

let mut path = if let Some(second) = split.next() {
second
} else {
return Err(())
};

let ty = cx.resolver.borrow_mut()
.with_scope(*id,
|resolver| {
resolver.resolve_str_path_error(DUMMY_SP,
&path, false)
})?;
match ty.def {
Def::Struct(did) | Def::Union(did) | Def::Enum(did) | Def::TyAlias(did) => {
let item = cx.tcx.inherent_impls(did).iter()
.flat_map(|imp| cx.tcx.associated_items(*imp))
.find(|item| item.name == item_name);
if let Some(item) = item {
if item.kind == ty::AssociatedKind::Method && is_val {
Ok((ty.def, Some(format!("method.{}", item_name))))
} else {
Err(())
}
} else {
Err(())
}
}
Def::Trait(did) => {
let item = cx.tcx.associated_item_def_ids(did).iter()
.map(|item| cx.tcx.associated_item(*item))
.find(|item| item.name == item_name);
if let Some(item) = item {
let kind = match item.kind {
ty::AssociatedKind::Const if is_val => "associatedconstant",
ty::AssociatedKind::Type if !is_val => "associatedtype",
ty::AssociatedKind::Method if is_val => "tymethod",
_ => return Err(())
};

Ok((ty.def, Some(format!("{}.{}", kind, item_name))))
} else {
Err(())
}
}
_ => Err(())
}

} else {
// FIXME(Manishearth) this branch doesn't seem to ever be hit, really
cx.resolver.borrow_mut()
.resolve_str_path_error(DUMMY_SP, &path_str, is_val)
Err(())
}
}

Expand Down Expand Up @@ -955,7 +1051,7 @@ impl Clean<Attributes> for [ast::Attribute] {
if UnstableFeatures::from_environment().is_nightly_build() {
let dox = attrs.collapsed_doc_value().unwrap_or_else(String::new);
for link in markdown_links(&dox, cx.render_type) {
let def = {
let (def, fragment) = {
let mut kind = PathKind::Unknown;
let path_str = if let Some(prefix) =
["struct@", "enum@", "type@",
Expand All @@ -965,7 +1061,8 @@ impl Clean<Attributes> for [ast::Attribute] {
link.trim_left_matches(prefix)
} else if let Some(prefix) =
["const@", "static@",
"value@", "function@", "mod@", "fn@", "module@"]
"value@", "function@", "mod@",
"fn@", "module@", "method@"]
.iter().find(|p| link.starts_with(**p)) {
kind = PathKind::Value;
link.trim_left_matches(prefix)
Expand Down Expand Up @@ -993,8 +1090,8 @@ impl Clean<Attributes> for [ast::Attribute] {

match kind {
PathKind::Value => {
if let Ok(path) = resolve(cx, path_str, true) {
path.def
if let Ok(def) = resolve(cx, path_str, true) {
def
} else {
// this could just be a normal link or a broken link
// we could potentially check if something is
Expand All @@ -1003,8 +1100,8 @@ impl Clean<Attributes> for [ast::Attribute] {
}
}
PathKind::Type => {
if let Ok(path) = resolve(cx, path_str, false) {
path.def
if let Ok(def) = resolve(cx, path_str, false) {
def
} else {
// this could just be a normal link
continue;
Expand All @@ -1013,50 +1110,50 @@ impl Clean<Attributes> for [ast::Attribute] {
PathKind::Unknown => {
// try everything!
if let Some(macro_def) = macro_resolve(cx, path_str) {
if let Ok(type_path) = resolve(cx, path_str, false) {
if let Ok(type_def) = resolve(cx, path_str, false) {
let (type_kind, article, type_disambig)
= type_ns_kind(type_path.def, path_str);
= type_ns_kind(type_def.0, path_str);
ambiguity_error(cx, &attrs, path_str,
article, type_kind, &type_disambig,
"a", "macro", &format!("macro@{}", path_str));
continue;
} else if let Ok(value_path) = resolve(cx, path_str, true) {
} else if let Ok(value_def) = resolve(cx, path_str, true) {
let (value_kind, value_disambig)
= value_ns_kind(value_path.def, path_str)
= value_ns_kind(value_def.0, path_str)
.expect("struct and mod cases should have been \
caught in previous branch");
ambiguity_error(cx, &attrs, path_str,
"a", value_kind, &value_disambig,
"a", "macro", &format!("macro@{}", path_str));
}
macro_def
} else if let Ok(type_path) = resolve(cx, path_str, false) {
(macro_def, None)
} else if let Ok(type_def) = resolve(cx, path_str, false) {
// It is imperative we search for not-a-value first
// Otherwise we will find struct ctors for when we are looking
// for structs, and the link won't work.
// if there is something in both namespaces
if let Ok(value_path) = resolve(cx, path_str, true) {
let kind = value_ns_kind(value_path.def, path_str);
if let Ok(value_def) = resolve(cx, path_str, true) {
let kind = value_ns_kind(value_def.0, path_str);
if let Some((value_kind, value_disambig)) = kind {
let (type_kind, article, type_disambig)
= type_ns_kind(type_path.def, path_str);
= type_ns_kind(type_def.0, path_str);
ambiguity_error(cx, &attrs, path_str,
article, type_kind, &type_disambig,
"a", value_kind, &value_disambig);
continue;
}
}
type_path.def
} else if let Ok(value_path) = resolve(cx, path_str, true) {
value_path.def
type_def
} else if let Ok(value_def) = resolve(cx, path_str, true) {
value_def
} else {
// this could just be a normal link
continue;
}
}
PathKind::Macro => {
if let Some(def) = macro_resolve(cx, path_str) {
def
(def, None)
} else {
continue
}
Expand All @@ -1066,7 +1163,7 @@ impl Clean<Attributes> for [ast::Attribute] {


let id = register_def(cx, def);
attrs.links.push((link, id));
attrs.links.push((link, id, fragment));
}

cx.sess().abort_if_errors();
Expand Down
21 changes: 20 additions & 1 deletion src/test/rustdoc/intra-links.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@

// @has intra_links/index.html
// @has - '//a/@href' '../intra_links/struct.ThisType.html'
// @has - '//a/@href' '../intra_links/struct.ThisType.html#method.this_method'
// @has - '//a/@href' '../intra_links/enum.ThisEnum.html'
// @has - '//a/@href' '../intra_links/enum.ThisEnum.html#ThisVariant.v'
// @has - '//a/@href' '../intra_links/trait.ThisTrait.html'
// @has - '//a/@href' '../intra_links/trait.ThisTrait.html#tymethod.this_associated_method'
// @has - '//a/@href' '../intra_links/trait.ThisTrait.html#associatedtype.ThisAssociatedType'
// @has - '//a/@href' '../intra_links/trait.ThisTrait.html#associatedconstant.THIS_ASSOCIATED_CONST'
// @has - '//a/@href' '../intra_links/trait.ThisTrait.html'
// @has - '//a/@href' '../intra_links/type.ThisAlias.html'
// @has - '//a/@href' '../intra_links/union.ThisUnion.html'
Expand All @@ -23,8 +29,13 @@
//! In this crate we would like to link to:
//!
//! * [`ThisType`](ThisType)
//! * [`ThisType::this_method`](ThisType::this_method)
//! * [`ThisEnum`](ThisEnum)
//! * [`ThisEnum::ThisVariant`](ThisEnum::ThisVariant)
//! * [`ThisTrait`](ThisTrait)
//! * [`ThisTrait::this_associated_method`](ThisTrait::this_associated_method)
//! * [`ThisTrait::ThisAssociatedType`](ThisTrait::ThisAssociatedType)
//! * [`ThisTrait::THIS_ASSOCIATED_CONST`](ThisTrait::THIS_ASSOCIATED_CONST)
//! * [`ThisAlias`](ThisAlias)
//! * [`ThisUnion`](ThisUnion)
//! * [`this_function`](this_function())
Expand All @@ -45,8 +56,16 @@ macro_rules! this_macro {
}

pub struct ThisType;

impl ThisType {
pub fn this_method() {}
}
pub enum ThisEnum { ThisVariant, }
pub trait ThisTrait {}
pub trait ThisTrait {
type ThisAssociatedType;
const THIS_ASSOCIATED_CONST: u8;
fn this_associated_method();
}
pub type ThisAlias = Result<(), ()>;
pub union ThisUnion { this_field: usize, }

Expand Down