Skip to content

[rustdoc] Correctly handle intra-doc links on inlined same item with different names #162669

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
GuillaumeGomez wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[rustdoc] Correctly handle intra-doc links on inlined same item with different names #162669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe47365
Add the alternatives parts in `Cache::paths`
GuillaumeGomez Sep 11, 2026
14548bf
Provide the "preferred name" when generating `href` and `title` for i…
GuillaumeGomez Sep 11, 2026
c9a5de5
Add regression test for inlined same item with different names
GuillaumeGomez Sep 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/librustdoc/clean/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ impl Item {
}

pub(crate) fn links(&self, cx: &Context<'_>) -> Vec<RenderedLink> {
use crate::html::format::{href, link_tooltip};
use crate::html::format::{href_with_path_check, link_tooltip};

let Some(links) = cx.cache().intra_doc_links.get(&self.item_or_reexport_id()) else {
return vec![];
Expand All @@ -609,7 +609,7 @@ impl Item {
.iter()
.filter_map(|ItemLink { link: s, link_text, page_id: id, fragment }| {
debug!(?id);
if let Ok(HrefInfo { mut url, .. }) = href(*id, cx) {
if let Ok(HrefInfo { mut url, .. }) = href_with_path_check(*id, cx, link_text) {
debug!(?url);
match fragment {
Some(UrlFragment::Item(def_id)) => {
Expand All @@ -625,7 +625,7 @@ impl Item {
Some(RenderedLink {
original_text: s.clone(),
new_text: link_text.clone(),
tooltip: link_tooltip(*id, fragment, cx).to_string(),
tooltip: link_tooltip(*id, fragment, cx, Some(link_text)).to_string(),
href: url,
})
} else {
Expand Down
54 changes: 43 additions & 11 deletions src/librustdoc/formats/cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,27 @@ use crate::formats::item_type::ItemType;
use crate::html::render::{IndexItem, IndexItemInfo};
use crate::visit_lib::RustdocEffectiveVisibilities;

pub(crate) struct PathInfo {
/// Parts of the path. So in `foo::bar::bib`, it will be `["foo", "bar", "bib"]`.
pub(crate) parts: Vec<Symbol>,
pub(crate) ty: ItemType,
/// When a reexport inline an item, we can end up with the same `DefId` with multiple local
/// targets. So in case like:
///
/// ```
/// /// Link to [`a2`].
/// pub use std::ffi::os_str::OsString as a1;
/// /// Link to [`a1`].
/// pub use std::ffi::os_str::OsString as a2;
/// /// Link to [`a2`].
/// pub use std::ffi::os_str::OsString as a3;
/// ```
///
/// To ensure that `a1` and `a2` links to `a1` and `a2` which have the same `DefId`, we need
/// to store both paths.
pub(crate) alternatives: Vec<Vec<Symbol>>,
}

/// This cache is used to store information about the [`clean::Crate`] being
/// rendered in order to provide more useful documentation. This contains
/// information like all implementors of a trait, all traits a type implements,
Expand All @@ -42,7 +63,7 @@ pub(crate) struct Cache {
/// URLs when a type is being linked to. External paths are not located in
/// this map because the `External` type itself has all the information
/// necessary.
pub(crate) paths: FxIndexMap<DefId, (Vec<Symbol>, ItemType)>,
pub(crate) paths: FxIndexMap<DefId, PathInfo>,

/// Similar to `paths`, but only holds external paths. This is only used for
/// generating explicit hyperlinks to other crates.
Expand Down Expand Up @@ -358,7 +379,8 @@ impl DocFolder for CacheBuilder<'_, '_> {
| clean::ForeignTypeItem
| clean::MacroItem(..)
| clean::ProcMacroItem(..)
| clean::VariantItem(..) => {
| clean::VariantItem(..)
| clean::PrimitiveItem(..) => {
use rustc_data_structures::fx::IndexEntry as Entry;

let skip_because_unstable = matches!(
Expand All @@ -376,21 +398,31 @@ impl DocFolder for CacheBuilder<'_, '_> {
let item_def_id = item.item_id.expect_def_id();
match self.cache.paths.entry(item_def_id) {
Entry::Vacant(entry) => {
entry.insert((self.cache.stack.clone(), item.type_()));
entry.insert(PathInfo {
parts: self.cache.stack.clone(),
ty: item.type_(),
alternatives: Vec::new(),
});
}
Entry::Occupied(mut entry) => {
if entry.get().0.len() > self.cache.stack.len() {
entry.insert((self.cache.stack.clone(), item.type_()));
// Shorter paths are preferred by default.
if entry.get().parts.len() > self.cache.stack.len() {
let old_parts = std::mem::replace(
&mut entry.get_mut().parts,
self.cache.stack.clone(),
);
// We only keep the old path if it's a different (final) name.
if old_parts.last() != self.cache.stack.last() {
entry.get_mut().alternatives.push(old_parts);
}
}
if !entry.get().alternatives.contains(&self.cache.stack) {
entry.get_mut().alternatives.push(self.cache.stack.clone());
}
}
}
}
}
clean::PrimitiveItem(..) => {
self.cache
.paths
.insert(item.item_id.expect_def_id(), (self.cache.stack.clone(), item.type_()));
}

clean::ExternCrateItem { .. }
| clean::ImportItem(..)
Expand Down Expand Up @@ -570,7 +602,7 @@ fn add_item_to_search_index(tcx: TyCtxt<'_>, cache: &mut Cache, item: &clean::It
// in a field of the cache whose elements are added to the search index later,
// after cache building is complete (see `handle_orphan_impl_child`).
match cache.paths.get(&parent_did) {
Some((fqp, _)) => (Some(parent_did), &fqp[..fqp.len() - 1]),
Some(info) => (Some(parent_did), &info.parts[..info.parts.len() - 1]),
None => {
handle_orphan_impl_child(cache, item, parent_did);
return;
Expand Down
105 changes: 71 additions & 34 deletions src/librustdoc/html/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,7 @@ pub(crate) fn href_with_root_path(
original_did: DefId,
cx: &Context<'_>,
root_path: Option<&str>,
preferred_name: Option<&str>,
) -> Result<HrefInfo, HrefError> {
let tcx = cx.tcx();
let def_kind = tcx.def_kind(original_did);
Expand All @@ -553,7 +554,9 @@ pub(crate) fn href_with_root_path(
}
// 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::Ctor(..) => {
return href_with_root_path(tcx.parent(original_did), cx, root_path, preferred_name);
}
DefKind::ExternCrate => {
// Link to the crate itself, not the `extern crate` item.
if let Some(local_did) = original_did.as_local() {
Expand All @@ -564,7 +567,7 @@ pub(crate) fn href_with_root_path(
}
_ => original_did,
};
if is_unnamable(cx.tcx(), did) {
if is_unnamable(tcx, did) {
return Err(HrefError::UnnamableItem);
}
let cache = cx.cache();
Expand All @@ -585,35 +588,46 @@ pub(crate) fn href_with_root_path(
}
}

let (fqp, shortty, url_parts, is_absolute) = 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)
},
false,
),
None => {
// 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);
let (parts, is_absolute) = url_parts(cache, did, module_fqp, relative_to)?;
(fqp, shortty, parts, is_absolute)
} 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 generate_item_def_id_path(did, original_did, cx, root_path);
let (fqp, shortty, url_parts, is_absolute) =
match cache.paths.get(&did) {
Some(info) => {
let path = if let Some(preferred_name) = preferred_name
&& let Some(alternative_path) = info.alternatives.iter().find(|path| {
path.last().is_some_and(|last| last.as_str() == preferred_name)
}) {
alternative_path
} else {
&info.parts
};
(
path,
info.ty,
{
let module_fqp = to_module_fqp(info.ty, info.parts.as_slice());
debug!(?info.parts, ?info.ty, ?module_fqp);
href_relative_parts(module_fqp, relative_to)
},
false,
)
}
}
};
None => {
// 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);
let (parts, is_absolute) = url_parts(cache, did, module_fqp, relative_to)?;
(fqp, shortty, parts, is_absolute)
} 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 generate_item_def_id_path(did, original_did, cx, root_path);
}
}
};
Ok(HrefInfo {
url: make_href(root_path, shortty, url_parts, fqp, is_absolute),
kind: shortty,
Expand All @@ -622,7 +636,15 @@ pub(crate) fn href_with_root_path(
}

pub(crate) fn href(did: DefId, cx: &Context<'_>) -> Result<HrefInfo, HrefError> {
href_with_root_path(did, cx, None)
href_with_root_path(did, cx, None, None)
}

pub(crate) fn href_with_path_check(
did: DefId,
cx: &Context<'_>,
text: &str,
) -> Result<HrefInfo, HrefError> {
href_with_root_path(did, cx, None, Some(text))
}

/// Both paths should only be modules.
Expand Down Expand Up @@ -660,14 +682,29 @@ pub(crate) fn link_tooltip(
did: DefId,
fragment: &Option<UrlFragment>,
cx: &Context<'_>,
preferred_name: Option<&str>,
) -> impl fmt::Display {
fmt::from_fn(move |f| {
let cache = cx.cache();
let Some((fqp, shortty)) = cache.paths.get(&did).or_else(|| cache.external_paths.get(&did))
let Some((fqp, shortty)) = cache
.paths
.get(&did)
.map(|info| {
if let Some(preferred_name) = preferred_name
&& let Some(path) = info.alternatives.iter().find(|path| {
path.last().is_some_and(|last| last.as_str() == preferred_name)
})
{
(path, info.ty)
} else {
(&info.parts, info.ty)
}
})
.or_else(|| cache.external_paths.get(&did).map(|(fqp, shortty)| (fqp, *shortty)))
else {
return Ok(());
};
let fqp = if *shortty == ItemType::Primitive {
let fqp = if shortty == ItemType::Primitive {
// primitives are documented in a crate, but not actually part of it
slice::from_ref(fqp.last().unwrap())
} else {
Expand All @@ -679,7 +716,7 @@ pub(crate) fn link_tooltip(
for component in fqp {
write!(f, "{component}::")?;
}
if *shortty == ItemType::Enum && tcx.def_kind(id) == DefKind::Field {
if shortty == ItemType::Enum && tcx.def_kind(id) == DefKind::Field {
write!(f, "{}::", tcx.item_name(tcx.parent(id)))?;
}
write!(f, "{}", tcx.item_name(id))?;
Expand Down
27 changes: 17 additions & 10 deletions src/librustdoc/html/highlight.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1406,23 +1406,30 @@ fn generate_link_to_def(
LinkFromSrc::Local(span) => {
context.href_from_span_relative(*span, &href_context.current_href)
}
LinkFromSrc::External(def_id) => {
format::href_with_root_path(*def_id, context, Some(href_context.root_path))
.ok()
.map(|HrefInfo { url, .. }| url)
}
LinkFromSrc::External(def_id) => format::href_with_root_path(
*def_id,
context,
Some(href_context.root_path),
None,
)
.ok()
.map(|HrefInfo { url, .. }| url),
LinkFromSrc::Primitive(prim) => format::href_with_root_path(
PrimitiveType::primitive_locations(context.tcx())[prim],
context,
Some(href_context.root_path),
None,
)
.ok()
.map(|HrefInfo { url, .. }| url),
LinkFromSrc::Doc(def_id) => format::href_with_root_path(
*def_id,
context,
Some(href_context.root_path),
None,
)
.ok()
.map(|HrefInfo { url, .. }| url),
LinkFromSrc::Doc(def_id) => {
format::href_with_root_path(*def_id, context, Some(href_context.root_path))
.ok()
.map(|HrefInfo { url, .. }| url)
}
}
})
{
Expand Down
12 changes: 6 additions & 6 deletions src/librustdoc/html/render/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,19 +296,19 @@ impl<'tcx> Context<'tcx> {
&self.shared.style_files,
)
} else {
if let Some(&(ref names, ty)) = self.cache().paths.get(&it.item_id.expect_def_id())
&& (self.current.len() + 1 != names.len()
|| self.current.iter().zip(names.iter()).any(|(a, b)| a != b))
if let Some(info) = self.cache().paths.get(&it.item_id.expect_def_id())
&& (self.current.len() + 1 != info.parts.len()
|| self.current.iter().zip(info.parts.iter()).any(|(a, b)| a != b))
{
// We checked that the redirection isn't pointing to the current file,
// preventing an infinite redirection loop in the generated
// documentation.

let path = fmt::from_fn(|f| {
for name in &names[..names.len() - 1] {
for name in &info.parts[..info.parts.len() - 1] {
write!(f, "{name}/")?;
}
write!(f, "{}", print_ty_path(ty, names.last().unwrap().as_str()))
write!(f, "{}", print_ty_path(info.ty, info.parts.last().unwrap().as_str()))
});
match self.shared.redirections {
Some(ref redirections) => {
Expand All @@ -320,7 +320,7 @@ impl<'tcx> Context<'tcx> {
let _ = write!(
current_path,
"{}",
print_ty_path(ty, names.last().unwrap().as_str())
print_ty_path(info.ty, info.parts.last().unwrap().as_str())
);
redirections.borrow_mut().insert(current_path, path.to_string());
}
Expand Down
8 changes: 4 additions & 4 deletions src/librustdoc/html/render/print_item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1455,12 +1455,12 @@ fn item_type_alias(cx: &Context<'_>, it: &clean::Item, t: &clean::TypeAlias) ->
// [^115718]: https://github.com/rust-lang/rust/issues/115718
let cache = &cx.shared.cache;
if let Some(target_did) = t.type_.def_id(cache)
&& let get_extern = { || cache.external_paths.get(&target_did) }
&& let Some(&(ref target_fqp, target_type)) =
cache.paths.get(&target_did).or_else(get_extern)
&& let get_extern = { || cache.external_paths.get(&target_did).map(|(fqp, shortty)| (fqp, *shortty)) }
&& let Some((target_fqp, target_type)) =
cache.paths.get(&target_did).map(|info| (&info.parts, info.ty)).or_else(get_extern)
&& target_type.is_adt() // primitives cannot be inlined
&& let Some(self_did) = it.item_id.as_def_id()
&& let get_local = { || cache.paths.get(&self_did).map(|(p, _)| p) }
&& let get_local = { || cache.paths.get(&self_did).map(|info| &info.parts) }
&& let Some(self_fqp) = cache.exact_paths.get(&self_did).or_else(get_local)
{
let mut js_src_path: UrlPartsBuilder =
Expand Down
Loading
Loading