Skip to content

Commit

Permalink
Rollup merge of rust-lang#47701 - Manishearth:intra-fixes, r=QuietMis…
Browse files Browse the repository at this point in the history 
…dreavus

Fixes for intra-doc-links

Turn errors into warnings, also handle methods, trait items, and variants.

r? @killercup @QuietMisdreavus
  • Loading branch information
@alexcrichton
alexcrichton committed Jan 25, 2018
2 parents 024e3aa + 08ca4fd commit b335b10
Show file tree
Hide file tree
Showing 2 changed files with 158 additions and 42 deletions.
179 changes: 138 additions & 41 deletions src/librustdoc/clean/mod.rs
View file
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
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

0 comments on commit b335b10

Please sign in to comment.