nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2025-02-03 18:43:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

Auto merge of #93805 - petrochenkov:doclinkself, r=camelid,GuillaumeGomez

Browse Source 
rustdoc: Stop textually replacing `Self` in doc links before resolving them

Resolve it directly to a type / def-id instead.

Also never pass `Self` to `Resolver`, it is useless because it's guaranteed that no resolution will be found.

This is a pre-requisite for https://github.com/rust-lang/rust/issues/83761.
This commit is contained in:
bors 2022-03-06 02:14:49 +00:00
commit 1661e4c7e0
2 changed files with 134 additions and 91 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

217
src/librustdoc/passes/collect_intra_doc_links.rs
View File

@ -26,7 +26,8 @@ use std::fmt::Write;
use std::mem;
use std::ops::Range;
use crate::clean::{self, utils::find_nearest_parent_module, Crate, Item, ItemLink, PrimitiveType};
use crate::clean::{self, utils::find_nearest_parent_module};
use crate::clean::{Crate, Item, ItemId, ItemLink, PrimitiveType};
use crate::core::DocContext;
use crate::html::markdown::{markdown_links, MarkdownLink};
use crate::lint::{BROKEN_INTRA_DOC_LINKS, PRIVATE_INTRA_DOC_LINKS};
@ -177,6 +178,8 @@ enum ResolutionFailure<'a> {
/// The link failed to resolve. [`resolution_failure`] should look to see if there's
/// a more helpful error that can be given.
NotResolved {
/// Item on which the link is resolved, used for resolving `Self`.
item_id: ItemId,
/// The scope the link was resolved in.
module_id: DefId,
/// If part of the link resolved, this has the `Res`.
@ -343,6 +346,7 @@ impl ItemFragment {
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
struct ResolutionInfo {
item_id: ItemId,
module_id: DefId,
dis: Option<Disambiguator>,
path_str: String,
@ -384,10 +388,12 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
fn variant_field<'path>(
&self,
path_str: &'path str,
item_id: ItemId,
module_id: DefId,
) -> Result<(Res, Option<ItemFragment>), ErrorKind<'path>> {
let tcx = self.cx.tcx;
let no_res = || ResolutionFailure::NotResolved {
item_id,
module_id,
partial_res: None,
unresolved: path_str.into(),
@ -410,13 +416,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
// If there's no third component, we saw `[a::b]` before and it failed to resolve.
// So there's no partial res.
.ok_or_else(no_res)?;
let ty_res = self
.cx
.enter_resolver(|resolver| {
resolver.resolve_str_path_error(DUMMY_SP, &path, TypeNS, module_id)
})
.and_then(|(_, res)| res.try_into())
.map_err(|()| no_res())?;
let ty_res = self.resolve_path(&path, TypeNS, item_id, module_id).ok_or_else(no_res)?;
match ty_res {
Res::Def(DefKind::Enum, did) => {
@ -437,6 +437,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
Ok((ty_res, Some(ItemFragment(FragmentKind::VariantField, field.did))))
} else {
Err(ResolutionFailure::NotResolved {
item_id,
module_id,
partial_res: Some(Res::Def(DefKind::Enum, def.did)),
unresolved: variant_field_name.to_string().into(),
@ -448,6 +449,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
}
}
_ => Err(ResolutionFailure::NotResolved {
item_id,
module_id,
partial_res: Some(ty_res),
unresolved: variant_name.to_string().into(),
@ -481,6 +483,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
fn resolve_macro(
&self,
path_str: &'a str,
item_id: ItemId,
module_id: DefId,
) -> Result<Res, ResolutionFailure<'a>> {
self.cx.enter_resolver(|resolver| {
@ -499,6 +502,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
return Ok(res.try_into().unwrap());
}
Err(ResolutionFailure::NotResolved {
item_id,
module_id,
partial_res: None,
unresolved: path_str.into(),
@ -506,12 +510,52 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
})
}
fn resolve_self_ty(&self, path_str: &str, ns: Namespace, item_id: ItemId) -> Option<Res> {
if ns != TypeNS || path_str != "Self" {
return None;
}
let tcx = self.cx.tcx;
item_id
.as_def_id()
.map(|def_id| match tcx.def_kind(def_id) {
def_kind @ (DefKind::AssocFn
| DefKind::AssocConst
| DefKind::AssocTy
| DefKind::Variant
| DefKind::Field) => {
let parent_def_id = tcx.parent(def_id).expect("nested item has no parent");
if def_kind == DefKind::Field && tcx.def_kind(parent_def_id) == DefKind::Variant
{
tcx.parent(parent_def_id).expect("variant has no parent")
} else {
parent_def_id
}
}
_ => def_id,
})
.and_then(|self_id| match tcx.def_kind(self_id) {
DefKind::Impl => self.def_id_to_res(self_id),
def_kind => Some(Res::Def(def_kind, self_id)),
})
}
/// Convenience wrapper around `resolve_str_path_error`.
///
/// This also handles resolving `true` and `false` as booleans.
/// NOTE: `resolve_str_path_error` knows only about paths, not about types.
/// Associated items will never be resolved by this function.
fn resolve_path(&self, path_str: &str, ns: Namespace, module_id: DefId) -> Option<Res> {
fn resolve_path(
&self,
path_str: &str,
ns: Namespace,
item_id: ItemId,
module_id: DefId,
) -> Option<Res> {
if let res @ Some(..) = self.resolve_self_ty(path_str, ns, item_id) {
return res;
}
let result = self.cx.enter_resolver(|resolver| {
resolver
.resolve_str_path_error(DUMMY_SP, path_str, ns, module_id)
@ -532,10 +576,11 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
&mut self,
path_str: &'path str,
ns: Namespace,
item_id: ItemId,
module_id: DefId,
user_fragment: &Option<String>,
) -> Result<(Res, Option<UrlFragment>), ErrorKind<'path>> {
let (res, rustdoc_fragment) = self.resolve_inner(path_str, ns, module_id)?;
let (res, rustdoc_fragment) = self.resolve_inner(path_str, ns, item_id, module_id)?;
let chosen_fragment = match (user_fragment, rustdoc_fragment) {
(Some(_), Some(r_frag)) => {
let diag_res = match r_frag {
@ -555,9 +600,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
&mut self,
path_str: &'path str,
ns: Namespace,
item_id: ItemId,
module_id: DefId,
) -> Result<(Res, Option<ItemFragment>), ErrorKind<'path>> {
if let Some(res) = self.resolve_path(path_str, ns, module_id) {
if let Some(res) = self.resolve_path(path_str, ns, item_id, module_id) {
match res {
// FIXME(#76467): make this fallthrough to lookup the associated
// item a separate function.
@ -585,6 +631,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
.ok_or_else(|| {
debug!("found no `::`, assumming {} was correctly not in scope", item_name);
ResolutionFailure::NotResolved {
item_id,
module_id,
partial_res: None,
unresolved: item_str.into(),
@ -596,7 +643,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
// error instead and special case *only* modules with `#[doc(primitive)]`, not all
// primitives.
resolve_primitive(&path_root, TypeNS)
.or_else(|| self.resolve_path(&path_root, TypeNS, module_id))
.or_else(|| self.resolve_path(&path_root, TypeNS, item_id, module_id))
.and_then(|ty_res| {
let (res, fragment) =
self.resolve_associated_item(ty_res, item_name, ns, module_id)?;
@ -605,9 +652,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
})
.unwrap_or_else(|| {
if ns == Namespace::ValueNS {
self.variant_field(path_str, module_id)
self.variant_field(path_str, item_id, module_id)
} else {
Err(ResolutionFailure::NotResolved {
item_id,
module_id,
partial_res: None,
unresolved: path_root.into(),
@ -723,10 +771,27 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
self.resolve_associated_item(res, item_name, ns, module_id)
}
Res::Def(
DefKind::Struct | DefKind::Union | DefKind::Enum | DefKind::ForeignTy,
def_kind @ (DefKind::Struct | DefKind::Union | DefKind::Enum | DefKind::ForeignTy),
did,
) => {
debug!("looking for associated item named {} for item {:?}", item_name, did);
// Checks if item_name is a variant of the `SomeItem` enum
if ns == TypeNS && def_kind == DefKind::Enum {
match tcx.type_of(did).kind() {
ty::Adt(adt_def, _) => {
for variant in &adt_def.variants {
if variant.name == item_name {
return Some((
root_res,
ItemFragment(FragmentKind::Variant, variant.def_id),
));
}
}
}
_ => unreachable!(),
}
}
// Checks if item_name belongs to `impl SomeItem`
let assoc_item = tcx
.inherent_impls(did)
@ -813,17 +878,18 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
&mut self,
ns: Namespace,
path_str: &str,
item_id: ItemId,
module_id: DefId,
extra_fragment: &Option<String>,
) -> Option<Res> {
// resolve() can't be used for macro namespace
let result = match ns {
Namespace::MacroNS => self
.resolve_macro(path_str, module_id)
.resolve_macro(path_str, item_id, module_id)
.map(|res| (res, None))
.map_err(ErrorKind::from),
Namespace::TypeNS | Namespace::ValueNS => {
self.resolve(path_str, ns, module_id, extra_fragment)
self.resolve(path_str, ns, item_id, module_id, extra_fragment)
}
};
@ -970,53 +1036,6 @@ impl<'a, 'tcx> DocVisitor for LinkCollector<'a, 'tcx> {
trace!("got parent node for {:?} {:?}, id {:?}", item.type_(), item.name, item.def_id);
}
// find item's parent to resolve `Self` in item's docs below
debug!("looking for the `Self` type");
let self_id = match item.def_id.as_def_id() {
None => None,
Some(did)
if (matches!(self.cx.tcx.def_kind(did), DefKind::Field)
&& matches!(
self.cx.tcx.def_kind(self.cx.tcx.parent(did).unwrap()),
DefKind::Variant
)) =>
{
self.cx.tcx.parent(did).and_then(|item_id| self.cx.tcx.parent(item_id))
}
Some(did)
if matches!(
self.cx.tcx.def_kind(did),
DefKind::AssocConst
| DefKind::AssocFn
| DefKind::AssocTy
| DefKind::Variant
| DefKind::Field
) =>
{
self.cx.tcx.parent(did)
}
Some(did) => Some(did),
};
// FIXME(jynelson): this shouldn't go through stringification, rustdoc should just use the DefId directly
let self_name = self_id.and_then(|self_id| {
if matches!(self.cx.tcx.def_kind(self_id), DefKind::Impl) {
// using `ty.to_string()` (or any variant) has issues with raw idents
let ty = self.cx.tcx.type_of(self_id);
let name = match ty.kind() {
ty::Adt(def, _) => Some(self.cx.tcx.item_name(def.did).to_string()),
other if other.is_primitive() => Some(ty.to_string()),
_ => None,
};
debug!("using type_of(): {:?}", name);
name
} else {
let name = self.cx.tcx.opt_item_name(self_id).map(|sym| sym.to_string());
debug!("using item_name(): {:?}", name);
name
}
});
let inner_docs = item.inner_docs(self.cx.tcx);
if item.is_mod() && inner_docs {
@ -1038,7 +1057,7 @@ impl<'a, 'tcx> DocVisitor for LinkCollector<'a, 'tcx> {
// NOTE: if there are links that start in one crate and end in another, this will not resolve them.
// This is a degenerate case and it's not supported by rustdoc.
for md_link in markdown_links(&doc) {
let link = self.resolve_link(&item, &doc, &self_name, parent_node, krate, md_link);
let link = self.resolve_link(&item, &doc, parent_node, krate, md_link);
if let Some(link) = link {
self.cx.cache.intra_doc_links.entry(item.def_id).or_default().push(link);
}
@ -1172,7 +1191,6 @@ impl LinkCollector<'_, '_> {
&mut self,
item: &Item,
dox: &str,
self_name: &Option<String>,
parent_node: Option<DefId>,
krate: CrateNum,
ori_link: MarkdownLink,
@ -1240,19 +1258,8 @@ impl LinkCollector<'_, '_> {
};
let resolved_self;
// replace `Self` with suitable item's parent name
let is_lone_self = path_str == "Self";
let is_lone_crate = path_str == "crate";
if path_str.starts_with("Self::") || is_lone_self {
if let Some(ref name) = self_name {
if is_lone_self {
path_str = name;
} else {
resolved_self = format!("{}::{}", name, &path_str[6..]);
path_str = &resolved_self;
}
}
} else if path_str.starts_with("crate::") || is_lone_crate {
if path_str.starts_with("crate::") || is_lone_crate {
use rustc_span::def_id::CRATE_DEF_INDEX;
// HACK(jynelson): rustc_resolve thinks that `crate` is the crate currently being documented.
@ -1272,6 +1279,7 @@ impl LinkCollector<'_, '_> {
let (mut res, fragment) = self.resolve_with_disambiguator_cached(
ResolutionInfo {
item_id: item.def_id,
module_id,
dis: disambiguator,
path_str: path_str.to_owned(),
@ -1514,12 +1522,13 @@ impl LinkCollector<'_, '_> {
) -> Option<(Res, Option<UrlFragment>)> {
let disambiguator = key.dis;
let path_str = &key.path_str;
let item_id = key.item_id;
let base_node = key.module_id;
let extra_fragment = &key.extra_fragment;
match disambiguator.map(Disambiguator::ns) {
Some(expected_ns @ (ValueNS | TypeNS)) => {
match self.resolve(path_str, expected_ns, base_node, extra_fragment) {
match self.resolve(path_str, expected_ns, item_id, base_node, extra_fragment) {
Ok(res) => Some(res),
Err(ErrorKind::Resolve(box mut kind)) => {
// We only looked in one namespace. Try to give a better error if possible.
@ -1528,9 +1537,13 @@ impl LinkCollector<'_, '_> {
// FIXME: really it should be `resolution_failure` that does this, not `resolve_with_disambiguator`
// See https://github.com/rust-lang/rust/pull/76955#discussion_r493953382 for a good approach
for new_ns in [other_ns, MacroNS] {
if let Some(res) =
self.check_full_res(new_ns, path_str, base_node, extra_fragment)
{
if let Some(res) = self.check_full_res(
new_ns,
path_str,
item_id,
base_node,
extra_fragment,
) {
kind = ResolutionFailure::WrongNamespace { res, expected_ns };
break;
}
@ -1552,9 +1565,15 @@ impl LinkCollector<'_, '_> {
// Try everything!
let mut candidates = PerNS {
macro_ns: self
.resolve_macro(path_str, base_node)
.resolve_macro(path_str, item_id, base_node)
.map(|res| (res, extra_fragment.clone().map(UrlFragment::UserWritten))),
type_ns: match self.resolve(path_str, TypeNS, base_node, extra_fragment) {
type_ns: match self.resolve(
path_str,
TypeNS,
item_id,
base_node,
extra_fragment,
) {
Ok(res) => {
debug!("got res in TypeNS: {:?}", res);
Ok(res)
@ -1565,7 +1584,13 @@ impl LinkCollector<'_, '_> {
}
Err(ErrorKind::Resolve(box kind)) => Err(kind),
},
value_ns: match self.resolve(path_str, ValueNS, base_node, extra_fragment) {
value_ns: match self.resolve(
path_str,
ValueNS,
item_id,
base_node,
extra_fragment,
) {
Ok(res) => Ok(res),
Err(ErrorKind::AnchorFailure(msg)) => {
anchor_failure(self.cx, diag, msg);
@ -1624,14 +1649,18 @@ impl LinkCollector<'_, '_> {
}
}
Some(MacroNS) => {
match self.resolve_macro(path_str, base_node) {
match self.resolve_macro(path_str, item_id, base_node) {
Ok(res) => Some((res, extra_fragment.clone().map(UrlFragment::UserWritten))),
Err(mut kind) => {
// `resolve_macro` only looks in the macro namespace. Try to give a better error if possible.
for ns in [TypeNS, ValueNS] {
if let Some(res) =
self.check_full_res(ns, path_str, base_node, extra_fragment)
{
if let Some(res) = self.check_full_res(
ns,
path_str,
item_id,
base_node,
extra_fragment,
) {
kind =
ResolutionFailure::WrongNamespace { res, expected_ns: MacroNS };
break;
@ -1958,11 +1987,16 @@ fn resolution_failure(
}
variants_seen.push(variant);
if let ResolutionFailure::NotResolved { module_id, partial_res, unresolved } =
&mut failure
if let ResolutionFailure::NotResolved {
item_id,
module_id,
partial_res,
unresolved,
} = &mut failure
{
use DefKind::*;
let item_id = *item_id;
let module_id = *module_id;
// FIXME(jynelson): this might conflict with my `Self` fix in #76467
// FIXME: maybe use itertools `collect_tuple` instead?
@ -1984,7 +2018,8 @@ fn resolution_failure(
};
name = start;
for ns in [TypeNS, ValueNS, MacroNS] {
if let Some(res) = collector.check_full_res(ns, start, module_id, &None)
if let Some(res) =
collector.check_full_res(ns, start, item_id, module_id, &None)
{
debug!("found partial_res={:?}", res);
*partial_res = Some(res);

8
src/test/rustdoc/intra-doc/associated-items.rs
View File

@ -57,4 +57,12 @@ impl T2 for S {
fn ambiguous_method() {}
}
// @has associated_items/enum.MyEnum.html '//a/@href' 'enum.MyEnum.html#variant.MyVariant'
/// Link to [MyEnumAlias::MyVariant]
pub enum MyEnum {
MyVariant,
}
pub type MyEnumAlias = MyEnum;
fn main() {}