squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

intra-doc: Use an enum to represent URL fragments

Browse source 
This is a step in the direction of computing the links more lazily,
which I think will simplify the implementation of intra-doc links.
This will also make it easier to eventually use the actual `Res` for
associated items, enum variants, and fields, rather than their HTML
page's `Res`.
This commit is contained in:
Noah Lev 2021-12-18 13:33:01 -08:00
parent 4472d9755b
commit ae2bc69250
2 changed files with 113 additions and 78 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
src/librustdoc/clean/types.rs
View file

@ -1,5 +1,6 @@
use std::cell::RefCell; use std::cell::RefCell;
use std::default::Default; use std::default::Default;
use std::fmt::Write;
use std::hash::Hash; use std::hash::Hash;
use std::lazy::SyncOnceCell as OnceCell; use std::lazy::SyncOnceCell as OnceCell;
use std::path::PathBuf; use std::path::PathBuf;
@ -40,6 +41,7 @@ use crate::formats::cache::Cache;
use crate::formats::item_type::ItemType; use crate::formats::item_type::ItemType;
use crate::html::render::cache::ExternalLocation; use crate::html::render::cache::ExternalLocation;
use crate::html::render::Context; use crate::html::render::Context;
use crate::passes::collect_intra_doc_links::UrlFragment;
crate use self::FnRetTy::*; crate use self::FnRetTy::*;
crate use self::ItemKind::*; crate use self::ItemKind::*;
@ -485,8 +487,7 @@ impl Item {
if let Ok((mut href, ..)) = href(*did, cx) { if let Ok((mut href, ..)) = href(*did, cx) {
debug!(?href); debug!(?href);
if let Some(ref fragment) = *fragment { if let Some(ref fragment) = *fragment {
href.push('#'); write!(href, "{}", fragment).unwrap()
href.push_str(fragment);
} }
Some(RenderedLink { Some(RenderedLink {
original_text: s.clone(), original_text: s.clone(),
@ -977,7 +978,7 @@ crate struct ItemLink {
pub(crate) link_text: String, pub(crate) link_text: String,
pub(crate) did: DefId, pub(crate) did: DefId,
/// The url fragment to append to the link /// The url fragment to append to the link
pub(crate) fragment: Option<String>, pub(crate) fragment: Option<UrlFragment>,
} }
pub struct RenderedLink { pub struct RenderedLink {

184
src/librustdoc/passes/collect_intra_doc_links.rs
View file

@ -238,12 +238,65 @@ enum AnchorFailure {
RustdocAnchorConflict(Res), RustdocAnchorConflict(Res),
} }
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
crate enum UrlFragment {
Method(Symbol),
TyMethod(Symbol),
AssociatedConstant(Symbol),
AssociatedType(Symbol),
StructField(Symbol),
Variant(Symbol),
VariantField { variant: Symbol, field: Symbol },
UserWritten(String),
}
impl UrlFragment {
/// Create a fragment for an associated item.
///
/// `is_prototype` is whether this associated item is a trait method
/// without a default definition.
fn from_assoc_item(name: Symbol, kind: ty::AssocKind, is_prototype: bool) -> Self {
match kind {
ty::AssocKind::Fn => {
if is_prototype {
UrlFragment::TyMethod(name)
} else {
UrlFragment::Method(name)
}
}
ty::AssocKind::Const => UrlFragment::AssociatedConstant(name),
ty::AssocKind::Type => UrlFragment::AssociatedType(name),
}
}
}
/// Render the fragment, including the leading `#`.
impl std::fmt::Display for UrlFragment {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "#")?;
match self {
UrlFragment::Method(name) => write!(f, "method.{}", name),
UrlFragment::TyMethod(name) => write!(f, "tymethod.{}", name),
UrlFragment::AssociatedConstant(name) => write!(f, "associatedconstant.{}", name),
UrlFragment::AssociatedType(name) => write!(f, "associatedtype.{}", name),
UrlFragment::StructField(name) => write!(f, "structfield.{}", name),
UrlFragment::Variant(name) => write!(f, "variant.{}", name),
UrlFragment::VariantField { variant, field } => {
write!(f, "variant.{}.field.{}", variant, field)
}
UrlFragment::UserWritten(raw) => write!(f, "{}", raw),
}
}
}
#[derive(Clone, Debug, Hash, PartialEq, Eq)] #[derive(Clone, Debug, Hash, PartialEq, Eq)]
struct ResolutionInfo { struct ResolutionInfo {
module_id: DefId, module_id: DefId,
dis: Option<Disambiguator>, dis: Option<Disambiguator>,
path_str: String, path_str: String,
extra_fragment: Option<String>, extra_fragment: Option<UrlFragment>,
} }
#[derive(Clone)] #[derive(Clone)]
@ -256,7 +309,7 @@ struct DiagnosticInfo<'a> {
#[derive(Clone, Debug, Hash)] #[derive(Clone, Debug, Hash)]
struct CachedLink { struct CachedLink {
pub res: (Res, Option<String>), pub res: (Res, Option<UrlFragment>),
pub side_channel: Option<(DefKind, DefId)>, pub side_channel: Option<(DefKind, DefId)>,
} }
@ -287,7 +340,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
&self, &self,
path_str: &'path str, path_str: &'path str,
module_id: DefId, module_id: DefId,
) -> Result<(Res, Option<String>), ErrorKind<'path>> { ) -> Result<(Res, Option<UrlFragment>), ErrorKind<'path>> {
let tcx = self.cx.tcx; let tcx = self.cx.tcx;
let no_res = || ResolutionFailure::NotResolved { let no_res = || ResolutionFailure::NotResolved {
module_id, module_id,
@ -297,15 +350,15 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
debug!("looking for enum variant {}", path_str); debug!("looking for enum variant {}", path_str);
let mut split = path_str.rsplitn(3, "::"); let mut split = path_str.rsplitn(3, "::");
let (variant_field_str, variant_field_name) = split let variant_field_name = split
.next() .next()
.map(|f| (f, Symbol::intern(f))) .map(|f| Symbol::intern(f))
.expect("fold_item should ensure link is non-empty"); .expect("fold_item should ensure link is non-empty");
let (variant_str, variant_name) = let variant_name =
// we're not sure this is a variant at all, so use the full string // we're not sure this is a variant at all, so use the full string
// If there's no second component, the link looks like `[path]`. // If there's no second component, the link looks like `[path]`.
// So there's no partial res and we should say the whole link failed to resolve. // So there's no partial res and we should say the whole link failed to resolve.
split.next().map(|f| (f, Symbol::intern(f))).ok_or_else(no_res)?; split.next().map(|f| Symbol::intern(f)).ok_or_else(no_res)?;
let path = split let path = split
.next() .next()
.map(|f| f.to_owned()) .map(|f| f.to_owned())
@ -337,16 +390,16 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
if def.all_fields().any(|item| item.ident.name == variant_field_name) { if def.all_fields().any(|item| item.ident.name == variant_field_name) {
Ok(( Ok((
ty_res, ty_res,
Some(format!( Some(UrlFragment::VariantField {
"variant.{}.field.{}", variant: variant_name,
variant_str, variant_field_name field: variant_field_name,
)), }),
)) ))
} else { } else {
Err(ResolutionFailure::NotResolved { Err(ResolutionFailure::NotResolved {
module_id, module_id,
partial_res: Some(Res::Def(DefKind::Enum, def.did)), partial_res: Some(Res::Def(DefKind::Enum, def.did)),
unresolved: variant_field_str.into(), unresolved: variant_field_name.to_string().into(),
} }
.into()) .into())
} }
@ -357,7 +410,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
_ => Err(ResolutionFailure::NotResolved { _ => Err(ResolutionFailure::NotResolved {
module_id, module_id,
partial_res: Some(ty_res), partial_res: Some(ty_res),
unresolved: variant_str.into(), unresolved: variant_name.to_string().into(),
} }
.into()), .into()),
} }
@ -369,7 +422,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
prim_ty: PrimitiveType, prim_ty: PrimitiveType,
ns: Namespace, ns: Namespace,
item_name: Symbol, item_name: Symbol,
) -> Option<(Res, String, Option<(DefKind, DefId)>)> { ) -> Option<(Res, UrlFragment, Option<(DefKind, DefId)>)> {
let tcx = self.cx.tcx; let tcx = self.cx.tcx;
prim_ty.impls(tcx).into_iter().find_map(|&impl_| { prim_ty.impls(tcx).into_iter().find_map(|&impl_| {
@ -377,12 +430,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
.find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, impl_) .find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, impl_)
.map(|item| { .map(|item| {
let kind = item.kind; let kind = item.kind;
let out = match kind { let fragment = UrlFragment::from_assoc_item(item_name, kind, false);
ty::AssocKind::Fn => "method",
ty::AssocKind::Const => "associatedconstant",
ty::AssocKind::Type => "associatedtype",
};
let fragment = format!("{}.{}", out, item_name);
(Res::Primitive(prim_ty), fragment, Some((kind.as_def_kind(), item.def_id))) (Res::Primitive(prim_ty), fragment, Some((kind.as_def_kind(), item.def_id)))
}) })
}) })
@ -457,8 +505,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
path_str: &'path str, path_str: &'path str,
ns: Namespace, ns: Namespace,
module_id: DefId, module_id: DefId,
extra_fragment: &Option<String>, extra_fragment: &Option<UrlFragment>,
) -> Result<(Res, Option<String>), ErrorKind<'path>> { ) -> Result<(Res, Option<UrlFragment>), ErrorKind<'path>> {
if let Some(res) = self.resolve_path(path_str, ns, module_id) { if let Some(res) = self.resolve_path(path_str, ns, module_id) {
match res { match res {
// FIXME(#76467): make this fallthrough to lookup the associated // FIXME(#76467): make this fallthrough to lookup the associated
@ -580,7 +628,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
item_name: Symbol, item_name: Symbol,
ns: Namespace, ns: Namespace,
module_id: DefId, module_id: DefId,
) -> Option<(Res, String, Option<(DefKind, DefId)>)> { ) -> Option<(Res, UrlFragment, Option<(DefKind, DefId)>)> {
let tcx = self.cx.tcx; let tcx = self.cx.tcx;
match root_res { match root_res {
@ -609,7 +657,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
imp, imp,
) )
}) })
.map(|item| (item.kind, item.def_id)) .copied()
// There should only ever be one associated item that matches from any inherent impl // There should only ever be one associated item that matches from any inherent impl
.next() .next()
// Check if item_name belongs to `impl SomeTrait for SomeItem` // Check if item_name belongs to `impl SomeTrait for SomeItem`
@ -618,26 +666,19 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
// To handle that properly resolve() would have to support // To handle that properly resolve() would have to support
// something like [`ambi_fn`](<SomeStruct as SomeTrait>::ambi_fn) // something like [`ambi_fn`](<SomeStruct as SomeTrait>::ambi_fn)
.or_else(|| { .or_else(|| {
let kind = let item =
resolve_associated_trait_item(did, module_id, item_name, ns, self.cx); resolve_associated_trait_item(did, module_id, item_name, ns, self.cx);
debug!("got associated item kind {:?}", kind); debug!("got associated item {:?}", item);
kind item
}); });
if let Some((kind, id)) = assoc_item { if let Some(item) = assoc_item {
let out = match kind { let kind = item.kind;
ty::AssocKind::Fn => "method", let fragment = UrlFragment::from_assoc_item(item_name, kind, false);
ty::AssocKind::Const => "associatedconstant",
ty::AssocKind::Type => "associatedtype",
};
// HACK(jynelson): `clean` expects the type, not the associated item // HACK(jynelson): `clean` expects the type, not the associated item
// but the disambiguator logic expects the associated item. // but the disambiguator logic expects the associated item.
// Store the kind in a side channel so that only the disambiguator logic looks at it. // Store the kind in a side channel so that only the disambiguator logic looks at it.
return Some(( return Some((root_res, fragment, Some((kind.as_def_kind(), item.def_id))));
root_res,
format!("{}.{}", out, item_name),
Some((kind.as_def_kind(), id)),
));
} }
if ns != Namespace::ValueNS { if ns != Namespace::ValueNS {
@ -657,34 +698,25 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
def.non_enum_variant().fields.iter().find(|item| item.ident.name == item_name) def.non_enum_variant().fields.iter().find(|item| item.ident.name == item_name)
}?; }?;
let kind = if def.is_enum() { DefKind::Variant } else { DefKind::Field }; let kind = if def.is_enum() { DefKind::Variant } else { DefKind::Field };
Some(( let fragment = if def.is_enum() {
root_res, // FIXME: how can the field be a variant?
format!( UrlFragment::Variant(field.ident.name)
"{}.{}", } else {
if def.is_enum() { "variant" } else { "structfield" }, UrlFragment::StructField(field.ident.name)
field.ident };
), Some((root_res, fragment, Some((kind, field.did))))
Some((kind, field.did)),
))
} }
Res::Def(DefKind::Trait, did) => tcx Res::Def(DefKind::Trait, did) => tcx
.associated_items(did) .associated_items(did)
.find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, did) .find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, did)
.map(|item| { .map(|item| {
let kind = match item.kind { let fragment = UrlFragment::from_assoc_item(
ty::AssocKind::Const => "associatedconstant", item_name,
ty::AssocKind::Type => "associatedtype", item.kind,
ty::AssocKind::Fn => { !item.defaultness.has_value(),
if item.defaultness.has_value() { );
"method"
} else {
"tymethod"
}
}
};
let res = Res::Def(item.kind.as_def_kind(), item.def_id); let res = Res::Def(item.kind.as_def_kind(), item.def_id);
(res, format!("{}.{}", kind, item_name), None) (res, fragment, None)
}), }),
_ => None, _ => None,
} }
@ -701,7 +733,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
ns: Namespace, ns: Namespace,
path_str: &str, path_str: &str,
module_id: DefId, module_id: DefId,
extra_fragment: &Option<String>, extra_fragment: &Option<UrlFragment>,
) -> Option<Res> { ) -> Option<Res> {
// resolve() can't be used for macro namespace // resolve() can't be used for macro namespace
let result = match ns { let result = match ns {
@ -732,7 +764,7 @@ fn resolve_associated_trait_item(
item_name: Symbol, item_name: Symbol,
ns: Namespace, ns: Namespace,
cx: &mut DocContext<'_>, cx: &mut DocContext<'_>,
) -> Option<(ty::AssocKind, DefId)> { ) -> Option<ty::AssocItem> {
// FIXME: this should also consider blanket impls (`impl<T> X for T`). Unfortunately // FIXME: this should also consider blanket impls (`impl<T> X for T`). Unfortunately
// `get_auto_trait_and_blanket_impls` is broken because the caching behavior is wrong. In the // `get_auto_trait_and_blanket_impls` is broken because the caching behavior is wrong. In the
// meantime, just don't look for these blanket impls. // meantime, just don't look for these blanket impls.
@ -742,14 +774,16 @@ fn resolve_associated_trait_item(
let traits = traits_implemented_by(cx, did, module); let traits = traits_implemented_by(cx, did, module);
debug!("considering traits {:?}", traits); debug!("considering traits {:?}", traits);
let mut candidates = traits.iter().filter_map(|&trait_| { let mut candidates = traits.iter().filter_map(|&trait_| {
cx.tcx cx.tcx.associated_items(trait_).find_by_name_and_namespace(
.associated_items(trait_) cx.tcx,
.find_by_name_and_namespace(cx.tcx, Ident::with_dummy_span(item_name), ns, trait_) Ident::with_dummy_span(item_name),
.map(|assoc| (assoc.kind, assoc.def_id)) ns,
trait_,
)
}); });
// FIXME(#74563): warn about ambiguity // FIXME(#74563): warn about ambiguity
debug!("the candidates were {:?}", candidates.clone().collect::<Vec<_>>()); debug!("the candidates were {:?}", candidates.clone().collect::<Vec<_>>());
candidates.next() candidates.next().copied()
} }
/// Given a type, return all traits in scope in `module` implemented by that type. /// Given a type, return all traits in scope in `module` implemented by that type.
@ -940,7 +974,7 @@ impl From<AnchorFailure> for PreprocessingError<'_> {
struct PreprocessingInfo { struct PreprocessingInfo {
path_str: String, path_str: String,
disambiguator: Option<Disambiguator>, disambiguator: Option<Disambiguator>,
extra_fragment: Option<String>, extra_fragment: Option<UrlFragment>,
link_text: String, link_text: String,
} }
@ -1026,7 +1060,7 @@ fn preprocess_link<'a>(
Some(Ok(PreprocessingInfo { Some(Ok(PreprocessingInfo {
path_str, path_str,
disambiguator, disambiguator,
extra_fragment: extra_fragment.map(String::from), extra_fragment: extra_fragment.map(|frag| UrlFragment::UserWritten(frag.to_owned())),
link_text: link_text.to_owned(), link_text: link_text.to_owned(),
})) }))
} }
@ -1144,7 +1178,7 @@ impl LinkCollector<'_, '_> {
module_id, module_id,
dis: disambiguator, dis: disambiguator,
path_str: path_str.to_owned(), path_str: path_str.to_owned(),
extra_fragment: extra_fragment.map(String::from), extra_fragment,
}, },
diag_info.clone(), // this struct should really be Copy, but Range is not :( diag_info.clone(), // this struct should really be Copy, but Range is not :(
matches!(ori_link.kind, LinkType::Reference | LinkType::Shortcut), matches!(ori_link.kind, LinkType::Reference | LinkType::Shortcut),
@ -1301,7 +1335,7 @@ impl LinkCollector<'_, '_> {
key: ResolutionInfo, key: ResolutionInfo,
diag: DiagnosticInfo<'_>, diag: DiagnosticInfo<'_>,
cache_resolution_failure: bool, cache_resolution_failure: bool,
) -> Option<(Res, Option<String>)> { ) -> Option<(Res, Option<UrlFragment>)> {
// Try to look up both the result and the corresponding side channel value // Try to look up both the result and the corresponding side channel value
if let Some(ref cached) = self.visited_links.get(&key) { if let Some(ref cached) = self.visited_links.get(&key) {
match cached { match cached {
@ -1349,7 +1383,7 @@ impl LinkCollector<'_, '_> {
&mut self, &mut self,
key: &ResolutionInfo, key: &ResolutionInfo,
diag: DiagnosticInfo<'_>, diag: DiagnosticInfo<'_>,
) -> Option<(Res, Option<String>)> { ) -> Option<(Res, Option<UrlFragment>)> {
let disambiguator = key.dis; let disambiguator = key.dis;
let path_str = &key.path_str; let path_str = &key.path_str;
let base_node = key.module_id; let base_node = key.module_id;
@ -2179,8 +2213,8 @@ fn privacy_error(cx: &DocContext<'_>, diag_info: &DiagnosticInfo<'_>, path_str:
fn handle_variant( fn handle_variant(
cx: &DocContext<'_>, cx: &DocContext<'_>,
res: Res, res: Res,
extra_fragment: &Option<String>, extra_fragment: &Option<UrlFragment>,
) -> Result<(Res, Option<String>), ErrorKind<'static>> { ) -> Result<(Res, Option<UrlFragment>), ErrorKind<'static>> {
use rustc_middle::ty::DefIdTree; use rustc_middle::ty::DefIdTree;
if extra_fragment.is_some() { if extra_fragment.is_some() {
@ -2192,7 +2226,7 @@ fn handle_variant(
.map(|parent| { .map(|parent| {
let parent_def = Res::Def(DefKind::Enum, parent); let parent_def = Res::Def(DefKind::Enum, parent);
let variant = cx.tcx.expect_variant_res(res.as_hir_res().unwrap()); let variant = cx.tcx.expect_variant_res(res.as_hir_res().unwrap());
(parent_def, Some(format!("variant.{}", variant.ident.name))) (parent_def, Some(UrlFragment::Variant(variant.ident.name)))
}) })
.ok_or_else(|| ResolutionFailure::NoParentItem.into()) .ok_or_else(|| ResolutionFailure::NoParentItem.into())
} }