From f7ba00c6365e34f7541a25dfb0bb0669b64daf9d Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 24 Jan 2018 13:49:57 +0530 Subject: [PATCH 1/7] Turn error into warning --- src/librustdoc/clean/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 78b24a8d18ab4..ae4ff10243e3b 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -880,10 +880,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 {}", From 6b26b7472b6c33b39d2dd12d20d1e92622261859 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 24 Jan 2018 15:54:45 +0530 Subject: [PATCH 2/7] Return Err() if resolve() is called before modules are set up --- src/librustdoc/clean/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index ae4ff10243e3b..d37015c99642a 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -904,9 +904,7 @@ fn resolve(cx: &DocContext, path_str: &str, is_val: bool) -> Result Date: Wed, 24 Jan 2018 15:57:31 +0530 Subject: [PATCH 3/7] Return Def from resolve() --- src/librustdoc/clean/mod.rs | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index d37015c99642a..7835cc175a8e6 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -894,7 +894,7 @@ fn ambiguity_error(cx: &DocContext, attrs: &Attributes, /// 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 { +fn resolve(cx: &DocContext, path_str: &str, is_val: bool) -> Result { // In case we're in a module, try to resolve the relative // path if let Some(id) = cx.mod_ids.borrow().last() { @@ -902,7 +902,7 @@ fn resolve(cx: &DocContext, path_str: &str, is_val: bool) -> Result 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 @@ -1001,8 +1001,8 @@ impl Clean 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; @@ -1011,16 +1011,16 @@ impl Clean 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, 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, path_str) .expect("struct and mod cases should have been \ caught in previous branch"); ambiguity_error(cx, &attrs, path_str, @@ -1028,25 +1028,25 @@ impl Clean for [ast::Attribute] { "a", "macro", &format!("macro@{}", path_str)); } macro_def - } else if let Ok(type_path) = resolve(cx, path_str, false) { + } 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, 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, 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; From 5a89f4011605d6636359335e65120d6b7360ef3e Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 24 Jan 2018 16:15:36 +0530 Subject: [PATCH 4/7] Handle methods --- src/librustdoc/clean/mod.rs | 95 +++++++++++++++++++++++++++++-------- 1 file changed, 76 insertions(+), 19 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 7835cc175a8e6..fdd54ba8e826a 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -659,7 +659,8 @@ pub struct Attributes { pub other_attrs: Vec, pub cfg: Option>, pub span: Option, - pub links: Vec<(String, DefId)>, + /// map from Rust paths to resolved defs and potential URL fragments + pub links: Vec<(String, DefId, Option)>, } impl Attributes { @@ -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 @@ -893,16 +898,67 @@ fn ambiguity_error(cx: &DocContext, attrs: &Attributes, } /// 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 { +/// 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), ()> { // 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) - }).map(|path| path.def) + let result = cx.resolver.borrow_mut() + .with_scope(*id, + |resolver| { + resolver.resolve_str_path_error(DUMMY_SP, + &path_str, is_val) + }); + + if result.is_ok() { + return result.map(|path| (path.def, None)); + } + + // Try looking for methods and other 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(_) => { + // XXXManishearth todo + Err(()) + } + _ => Err(()) + } + } else { Err(()) } @@ -953,7 +1009,7 @@ impl Clean 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@", @@ -963,7 +1019,8 @@ impl Clean 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) @@ -1013,31 +1070,31 @@ impl Clean for [ast::Attribute] { if let Some(macro_def) = macro_resolve(cx, path_str) { if let Ok(type_def) = resolve(cx, path_str, false) { let (type_kind, article, type_disambig) - = type_ns_kind(type_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_def) = resolve(cx, path_str, true) { let (value_kind, value_disambig) - = value_ns_kind(value_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 + (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_def) = resolve(cx, path_str, true) { - let kind = value_ns_kind(value_def, path_str); + 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_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); @@ -1054,7 +1111,7 @@ impl Clean for [ast::Attribute] { } PathKind::Macro => { if let Some(def) = macro_resolve(cx, path_str) { - def + (def, None) } else { continue } @@ -1064,7 +1121,7 @@ impl Clean 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(); From a4d36928fd872c146b15cc8aa2211267441cffc1 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 24 Jan 2018 16:44:26 +0530 Subject: [PATCH 5/7] Make it work for traits --- src/librustdoc/clean/mod.rs | 45 ++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index fdd54ba8e826a..7b851665d3ea2 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -911,11 +911,29 @@ fn resolve(cx: &DocContext, path_str: &str, is_val: bool) -> Result<(Def, Option &path_str, is_val) }); - if result.is_ok() { - return result.map(|path| (path.def, None)); + 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, + // 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 other associated items + // 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 @@ -935,8 +953,6 @@ fn resolve(cx: &DocContext, path_str: &str, is_val: bool) -> Result<(Def, Option 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() @@ -952,9 +968,22 @@ fn resolve(cx: &DocContext, path_str: &str, is_val: bool) -> Result<(Def, Option Err(()) } } - Def::Trait(_) => { - // XXXManishearth todo - 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(()) } From c26f8877c71c2f6c7b73074358ae114f3e0a4eab Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 24 Jan 2018 17:35:31 +0530 Subject: [PATCH 6/7] Handle variants --- src/librustdoc/clean/mod.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 7b851665d3ea2..0929b833c1965 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -848,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(..) @@ -897,6 +895,20 @@ 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), ()> { + 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. Also returns an optional URL fragment in the case /// of variants and methods @@ -917,6 +929,7 @@ fn resolve(cx: &DocContext, path_str: &str, is_val: bool) -> Result<(Def, Option 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)) }; From 08ca4fd1358e172de351df475ea55b4e52605a21 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Thu, 25 Jan 2018 10:11:25 +0530 Subject: [PATCH 7/7] Add tests --- src/test/rustdoc/intra-links.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/test/rustdoc/intra-links.rs b/src/test/rustdoc/intra-links.rs index aa6f553875441..4726323e11cef 100644 --- a/src/test/rustdoc/intra-links.rs +++ b/src/test/rustdoc/intra-links.rs @@ -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' @@ -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()) @@ -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, }