Skip to content

Commit

Permalink
Unrolled build for rust-lang#120596
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#120596 - GuillaumeGomez:jump-to-def-non-local-link, r=notriddle,fmease

[rustdoc] Correctly generate path for non-local items in source code pages

While browsing some crates using the "jump to def" feature, I realized that a lot of items didn't have a link generated. The reason is because we only cache foreign items if they appear in the documented API. This means that for the others, we need to infer them.

r? ``@notriddle``
  • Loading branch information
rust-timer committed Feb 10, 2024
2 parents d44e3b9 + 14e0dab commit fd39182
Show file tree
Hide file tree
Showing 4 changed files with 219 additions and 79 deletions.
19 changes: 16 additions & 3 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,20 @@ pub(crate) fn load_attrs<'hir>(cx: &DocContext<'hir>, did: DefId) -> &'hir [ast:
cx.tcx.get_attrs_unchecked(did)
}

pub(crate) fn item_relative_path(tcx: TyCtxt<'_>, def_id: DefId) -> Vec<Symbol> {
tcx.def_path(def_id)
.data
.into_iter()
.filter_map(|elem| {
// extern blocks (and a few others things) have an empty name.
match elem.data.get_opt_name() {
Some(s) if !s.is_empty() => Some(s),
_ => None,
}
})
.collect()
}

/// Record an external fully qualified name in the external_paths cache.
///
/// These names are used later on by HTML rendering to generate things like
Expand All @@ -206,8 +220,7 @@ pub(crate) fn record_extern_fqn(cx: &mut DocContext<'_>, did: DefId, kind: ItemT

let crate_name = cx.tcx.crate_name(did.krate);

let relative =
cx.tcx.def_path(did).data.into_iter().filter_map(|elem| elem.data.get_opt_name());
let relative = item_relative_path(cx.tcx, did);
let fqn = if let ItemType::Macro = kind {
// Check to see if it is a macro 2.0 or built-in macro
if matches!(
Expand All @@ -218,7 +231,7 @@ pub(crate) fn record_extern_fqn(cx: &mut DocContext<'_>, did: DefId, kind: ItemT
) {
once(crate_name).chain(relative).collect()
} else {
vec![crate_name, relative.last().expect("relative was empty")]
vec![crate_name, *relative.last().expect("relative was empty")]
}
} else {
once(crate_name).chain(relative).collect()
Expand Down
37 changes: 25 additions & 12 deletions src/librustdoc/formats/item_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::fmt;

use serde::{Serialize, Serializer};

use rustc_hir::def::DefKind;
use rustc_hir::def::{CtorOf, DefKind};
use rustc_span::hygiene::MacroKind;

use crate::clean;
Expand Down Expand Up @@ -115,7 +115,15 @@ impl<'a> From<&'a clean::Item> for ItemType {

impl From<DefKind> for ItemType {
fn from(other: DefKind) -> Self {
match other {
Self::from_def_kind(other, None)
}
}

impl ItemType {
/// Depending on the parent kind, some variants have a different translation (like a `Method`
/// becoming a `TyMethod`).
pub(crate) fn from_def_kind(kind: DefKind, parent_kind: Option<DefKind>) -> Self {
match kind {
DefKind::Enum => Self::Enum,
DefKind::Fn => Self::Function,
DefKind::Mod => Self::Module,
Expand All @@ -131,30 +139,35 @@ impl From<DefKind> for ItemType {
MacroKind::Attr => ItemType::ProcAttribute,
MacroKind::Derive => ItemType::ProcDerive,
},
DefKind::ForeignTy
| DefKind::Variant
| DefKind::AssocTy
| DefKind::TyParam
DefKind::ForeignTy => Self::ForeignType,
DefKind::Variant => Self::Variant,
DefKind::Field => Self::StructField,
DefKind::AssocTy => Self::AssocType,
DefKind::AssocFn => {
if let Some(DefKind::Trait) = parent_kind {
Self::TyMethod
} else {
Self::Method
}
}
DefKind::Ctor(CtorOf::Struct, _) => Self::Struct,
DefKind::Ctor(CtorOf::Variant, _) => Self::Variant,
DefKind::AssocConst => Self::AssocConst,
DefKind::TyParam
| DefKind::ConstParam
| DefKind::Ctor(..)
| DefKind::AssocFn
| DefKind::AssocConst
| DefKind::ExternCrate
| DefKind::Use
| DefKind::ForeignMod
| DefKind::AnonConst
| DefKind::InlineConst
| DefKind::OpaqueTy
| DefKind::Field
| DefKind::LifetimeParam
| DefKind::GlobalAsm
| DefKind::Impl { .. }
| DefKind::Closure => Self::ForeignType,
}
}
}

impl ItemType {
pub(crate) fn as_str(&self) -> &'static str {
match *self {
ItemType::Module => "mod",
Expand Down
194 changes: 130 additions & 64 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use crate::clean::{
self, types::ExternalLocation, utils::find_nearest_parent_module, ExternalCrate, ItemId,
PrimitiveType,
};
use crate::formats::cache::Cache;
use crate::formats::item_type::ItemType;
use crate::html::escape::Escape;
use crate::html::render::Context;
Expand Down Expand Up @@ -581,22 +582,11 @@ fn generate_macro_def_id_path(
cx: &Context<'_>,
root_path: Option<&str>,
) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
let tcx = cx.shared.tcx;
let tcx = cx.tcx();
let crate_name = tcx.crate_name(def_id.krate);
let cache = cx.cache();

let fqp: Vec<Symbol> = tcx
.def_path(def_id)
.data
.into_iter()
.filter_map(|elem| {
// extern blocks (and a few others things) have an empty name.
match elem.data.get_opt_name() {
Some(s) if !s.is_empty() => Some(s),
_ => None,
}
})
.collect();
let fqp = clean::inline::item_relative_path(tcx, def_id);
let mut relative = fqp.iter().copied();
let cstore = CStore::from_tcx(tcx);
// We need this to prevent a `panic` when this function is used from intra doc links...
Expand Down Expand Up @@ -651,92 +641,168 @@ fn generate_macro_def_id_path(
Ok((url, ItemType::Macro, fqp))
}

fn generate_item_def_id_path(
mut def_id: DefId,
original_def_id: DefId,
cx: &Context<'_>,
root_path: Option<&str>,
original_def_kind: DefKind,
) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
use crate::rustc_trait_selection::infer::TyCtxtInferExt;
use crate::rustc_trait_selection::traits::query::normalize::QueryNormalizeExt;
use rustc_middle::traits::ObligationCause;

let tcx = cx.tcx();
let crate_name = tcx.crate_name(def_id.krate);

// No need to try to infer the actual parent item if it's not an associated item from the `impl`
// block.
if def_id != original_def_id && matches!(tcx.def_kind(def_id), DefKind::Impl { .. }) {
let infcx = tcx.infer_ctxt().build();
def_id = infcx
.at(&ObligationCause::dummy(), tcx.param_env(def_id))
.query_normalize(ty::Binder::dummy(tcx.type_of(def_id).instantiate_identity()))
.map(|resolved| infcx.resolve_vars_if_possible(resolved.value))
.ok()
.and_then(|normalized| normalized.skip_binder().ty_adt_def())
.map(|adt| adt.did())
.unwrap_or(def_id);
}

let relative = clean::inline::item_relative_path(tcx, def_id);
let fqp: Vec<Symbol> = once(crate_name).chain(relative).collect();

let def_kind = tcx.def_kind(def_id);
let shortty = def_kind.into();
let module_fqp = to_module_fqp(shortty, &fqp);
let mut is_remote = false;

let url_parts = url_parts(cx.cache(), def_id, &module_fqp, &cx.current, &mut is_remote)?;
let (url_parts, shortty, fqp) = make_href(root_path, shortty, url_parts, &fqp, is_remote)?;
if def_id == original_def_id {
return Ok((url_parts, shortty, fqp));
}
let kind = ItemType::from_def_kind(original_def_kind, Some(def_kind));
Ok((format!("{url_parts}#{kind}.{}", tcx.item_name(original_def_id)), shortty, fqp))
}

fn to_module_fqp(shortty: ItemType, fqp: &[Symbol]) -> &[Symbol] {
if shortty == ItemType::Module { fqp } else { &fqp[..fqp.len() - 1] }
}

fn url_parts(
cache: &Cache,
def_id: DefId,
module_fqp: &[Symbol],
relative_to: &[Symbol],
is_remote: &mut bool,
) -> Result<UrlPartsBuilder, HrefError> {
match cache.extern_locations[&def_id.krate] {
ExternalLocation::Remote(ref s) => {
*is_remote = true;
let s = s.trim_end_matches('/');
let mut builder = UrlPartsBuilder::singleton(s);
builder.extend(module_fqp.iter().copied());
Ok(builder)
}
ExternalLocation::Local => Ok(href_relative_parts(module_fqp, relative_to).collect()),
ExternalLocation::Unknown => Err(HrefError::DocumentationNotBuilt),
}
}

fn make_href(
root_path: Option<&str>,
shortty: ItemType,
mut url_parts: UrlPartsBuilder,
fqp: &[Symbol],
is_remote: bool,
) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
if !is_remote && let Some(root_path) = root_path {
let root = root_path.trim_end_matches('/');
url_parts.push_front(root);
}
debug!(?url_parts);
match shortty {
ItemType::Module => {
url_parts.push("index.html");
}
_ => {
let prefix = shortty.as_str();
let last = fqp.last().unwrap();
url_parts.push_fmt(format_args!("{prefix}.{last}.html"));
}
}
Ok((url_parts.finish(), shortty, fqp.to_vec()))
}

pub(crate) fn href_with_root_path(
did: DefId,
original_did: DefId,
cx: &Context<'_>,
root_path: Option<&str>,
) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
let tcx = cx.tcx();
let def_kind = tcx.def_kind(did);
let def_kind = tcx.def_kind(original_did);
let did = match def_kind {
DefKind::AssocTy | DefKind::AssocFn | DefKind::AssocConst | DefKind::Variant => {
// documented on their parent's page
tcx.parent(did)
tcx.parent(original_did)
}
// If this a constructor, we get the parent (either a struct or a variant) and then
// generate the link for this item.
DefKind::Ctor(..) => return href_with_root_path(tcx.parent(original_did), cx, root_path),
DefKind::ExternCrate => {
// Link to the crate itself, not the `extern crate` item.
if let Some(local_did) = did.as_local() {
if let Some(local_did) = original_did.as_local() {
tcx.extern_mod_stmt_cnum(local_did).unwrap_or(LOCAL_CRATE).as_def_id()
} else {
did
original_did
}
}
_ => did,
_ => original_did,
};
let cache = cx.cache();
let relative_to = &cx.current;
fn to_module_fqp(shortty: ItemType, fqp: &[Symbol]) -> &[Symbol] {
if shortty == ItemType::Module { fqp } else { &fqp[..fqp.len() - 1] }
}

if !did.is_local()
&& !cache.effective_visibilities.is_directly_public(tcx, did)
&& !cache.document_private
&& !cache.primitive_locations.values().any(|&id| id == did)
{
return Err(HrefError::Private);
if !original_did.is_local() {
// If we are generating an href for the "jump to def" feature, then the only case we want
// to ignore is if the item is `doc(hidden)` because we can't link to it.
if root_path.is_some() {
if tcx.is_doc_hidden(original_did) {
return Err(HrefError::Private);
}
} else if !cache.effective_visibilities.is_directly_public(tcx, did)
&& !cache.document_private
&& !cache.primitive_locations.values().any(|&id| id == did)
{
return Err(HrefError::Private);
}
}

let mut is_remote = false;
let (fqp, shortty, mut url_parts) = match cache.paths.get(&did) {
let (fqp, shortty, url_parts) = match cache.paths.get(&did) {
Some(&(ref fqp, shortty)) => (fqp, shortty, {
let module_fqp = to_module_fqp(shortty, fqp.as_slice());
debug!(?fqp, ?shortty, ?module_fqp);
href_relative_parts(module_fqp, relative_to).collect()
}),
None => {
if let Some(&(ref fqp, shortty)) = cache.external_paths.get(&did) {
// Associated items are handled differently with "jump to def". The anchor is generated
// directly here whereas for intra-doc links, we have some extra computation being
// performed there.
let def_id_to_get = if root_path.is_some() { original_did } else { did };
if let Some(&(ref fqp, shortty)) = cache.external_paths.get(&def_id_to_get) {
let module_fqp = to_module_fqp(shortty, fqp);
(
fqp,
shortty,
match cache.extern_locations[&did.krate] {
ExternalLocation::Remote(ref s) => {
is_remote = true;
let s = s.trim_end_matches('/');
let mut builder = UrlPartsBuilder::singleton(s);
builder.extend(module_fqp.iter().copied());
builder
}
ExternalLocation::Local => {
href_relative_parts(module_fqp, relative_to).collect()
}
ExternalLocation::Unknown => return Err(HrefError::DocumentationNotBuilt),
},
)
(fqp, shortty, url_parts(cache, did, module_fqp, relative_to, &mut is_remote)?)
} else if matches!(def_kind, DefKind::Macro(_)) {
return generate_macro_def_id_path(did, cx, root_path);
} else if did.is_local() {
return Err(HrefError::Private);
} else {
return Err(HrefError::NotInExternalCache);
return generate_item_def_id_path(did, original_did, cx, root_path, def_kind);
}
}
};
if !is_remote && let Some(root_path) = root_path {
let root = root_path.trim_end_matches('/');
url_parts.push_front(root);
}
debug!(?url_parts);
match shortty {
ItemType::Module => {
url_parts.push("index.html");
}
_ => {
let prefix = shortty.as_str();
let last = fqp.last().unwrap();
url_parts.push_fmt(format_args!("{prefix}.{last}.html"));
}
}
Ok((url_parts.finish(), shortty, fqp.to_vec()))
make_href(root_path, shortty, url_parts, fqp, is_remote)
}

pub(crate) fn href(
Expand Down
Loading

0 comments on commit fd39182

Please sign in to comment.