Skip to content

Commit 706e51d

Browse files
brsonclaude
andcommitted
Fix broken internal links in API doc generator
Re-exported items from private internal modules (e.g. std::thread::Builder from std::thread::builder) were missing pages and had broken links because the doc generator skipped page creation for "LocalPublic" re-exports, assuming pages existed at the original definition path. Three categories of fixes: - Create pages at re-export locations for LocalPublic items, not just NeedsPage. Only skip page creation for External (cross-crate) items. Fix html_path computation for Use items to include the target's kind prefix (struct., fn., etc.). - Use rustdoc JSON's pre-resolved `links` field for intra-doc link resolution, so links like `spawn` and `Builder` resolve to the correct item in the current module rather than an arbitrary match from another crate (e.g. tokio::spawn or regex_automata::Builder). - Handle methods and enum variants in link resolution: methods link to their parent type page with #method.name anchors, and enum variants link to the parent enum page with #variant.Name anchors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 66c9879 commit 706e51d

5 files changed

Lines changed: 217 additions & 17 deletions

File tree

crates/rustmax-rustdoc/src/output.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,15 @@ fn write_item(ctx: &RenderContext, item: &crate::types::RenderableItem) -> AnyRe
6060
}
6161
let target_id = use_item.id.as_ref().unwrap();
6262

63-
// Skip page creation if target has a public page elsewhere.
63+
// Skip page creation only for external crates (they have their own pages).
64+
// LocalPublic items may be defined in private modules with no rendered page,
65+
// so we always create a page at the re-export location.
6466
match ctx.reexport_target(target_id) {
65-
ReexportTarget::LocalPublic { .. } | ReexportTarget::External { .. } => {
67+
ReexportTarget::External { .. } => {
6668
return Ok(());
6769
}
68-
ReexportTarget::NeedsPage => {
69-
// Proceed with page creation.
70+
ReexportTarget::LocalPublic { .. } | ReexportTarget::NeedsPage => {
71+
// Proceed with page creation at the re-export location.
7072
}
7173
}
7274

crates/rustmax-rustdoc/src/render/markdown.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
//! Markdown to HTML rendering with syntax highlighting and intra-doc link resolution.
22
3+
use std::collections::HashMap;
4+
35
use comrak::{markdown_to_html_with_plugins, Arena, Options, Plugins};
46
use comrak::adapters::SyntaxHighlighterAdapter;
57
use comrak::nodes::NodeValue;
@@ -23,12 +25,16 @@ pub fn render_markdown(md: &str, highlighter: &Highlighter) -> String {
2325
///
2426
/// Resolves Rust path references like `` [`Vec`] `` or `[text](std::option::Option)`
2527
/// to actual HTML documentation URLs.
28+
///
29+
/// `pre_resolved_links` maps link text to pre-resolved URLs from the item's
30+
/// rustdoc JSON `links` field. These take priority over global index resolution.
2631
pub fn render_markdown_with_links(
2732
md: &str,
2833
highlighter: &Highlighter,
2934
global_index: Option<&GlobalItemIndex>,
3035
current_crate: &str,
3136
current_depth: usize,
37+
pre_resolved_links: &HashMap<String, String>,
3238
) -> String {
3339
let Some(index) = global_index else {
3440
return render_markdown(md, highlighter);
@@ -46,7 +52,10 @@ pub fn render_markdown_with_links(
4652
for node in root.descendants() {
4753
let mut data = node.data.borrow_mut();
4854
if let NodeValue::Link(ref mut link) = data.value {
49-
if is_rust_path(&link.url) {
55+
// Check pre-resolved links first (from rustdoc's links field).
56+
if let Some(url) = pre_resolved_links.get(&link.url) {
57+
link.url = url.clone();
58+
} else if is_rust_path(&link.url) {
5059
if let Some(resolved) = resolve_rust_path(&link.url, index, current_crate, current_depth) {
5160
link.url = resolved;
5261
} else {
@@ -459,6 +468,7 @@ pub fn render_short_doc(
459468
global_index: Option<&GlobalItemIndex>,
460469
current_crate: &str,
461470
current_depth: usize,
471+
pre_resolved_links: &HashMap<String, String>,
462472
) -> String {
463473
let ref_defs = extract_reference_definitions(full_docs);
464474
let first_para = extract_first_paragraph(full_docs);
@@ -475,6 +485,7 @@ pub fn render_short_doc(
475485
global_index,
476486
current_crate,
477487
current_depth,
488+
pre_resolved_links,
478489
);
479490

480491
strip_paragraph_wrapper(&html)

crates/rustmax-rustdoc/src/render/mod.rs

Lines changed: 174 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@ pub mod sidebar;
99

1010
use rmx::prelude::*;
1111
use rmx::tera::Tera;
12-
use rustdoc_types::{Crate, Id, ItemKind, Visibility};
12+
use rustdoc_types::{Crate, Id, ItemEnum, ItemKind, Visibility};
1313
use std::collections::HashMap;
14+
use std::hash::BuildHasher;
1415
use std::path::PathBuf;
1516

1617
use crate::{RenderConfig, ModuleTree, GlobalItemIndex};
17-
use crate::types::{build_module_tree, build_impl_index, ImplIndex};
18+
use crate::types::{build_module_tree, build_impl_index, get_type_id, ImplIndex};
1819

1920
/// Where a re-export target's page exists.
2021
pub enum ReexportTarget {
@@ -177,30 +178,201 @@ impl<'a> RenderContext<'a> {
177178
Some(url)
178179
}
179180

181+
/// Pre-resolve an item's `links` field into a URL map.
182+
///
183+
/// Takes the item's `links` field (mapping link text to item IDs) and resolves
184+
/// each ID to an actual HTML URL. The keys are normalized to strip backtick
185+
/// wrappers since the markdown preprocessor strips them.
186+
pub fn resolve_item_links<S: BuildHasher>(
187+
&self,
188+
links: &std::collections::HashMap<String, Id, S>,
189+
current_depth: usize,
190+
) -> HashMap<String, String> {
191+
let mut resolved = HashMap::new();
192+
for (text, id) in links {
193+
if let Some(url) = self.resolve_reexport_url(id, current_depth) {
194+
// The links field keys may have backtick wrappers (`` `Builder` ``).
195+
// Strip them since preprocess_shortcut_links removes them from URLs.
196+
let clean_key = text.trim_matches('`').to_string();
197+
resolved.insert(clean_key, url);
198+
}
199+
}
200+
resolved
201+
}
202+
203+
/// Resolve an item ID to a URL, preferring the re-export location.
204+
///
205+
/// Handles types, modules, methods, and enum variants.
206+
fn resolve_reexport_url(&self, id: &Id, current_depth: usize) -> Option<String> {
207+
// First check krate.paths (has types, modules, variants, but not methods).
208+
if let Some(summary) = self.krate.paths.get(id) {
209+
// Handle enum variants: link to parent enum page with #variant.Name anchor.
210+
if summary.kind == ItemKind::Variant {
211+
return self.resolve_variant_url(&summary.path, summary.crate_id, current_depth);
212+
}
213+
214+
if summary.crate_id == 0 {
215+
// Local item. Check if it's re-exported to a public-facing location.
216+
if let Some(url) = self.find_reexport_url(id, summary.kind, current_depth) {
217+
return Some(url);
218+
}
219+
// Fall back to definition path.
220+
return self.build_item_url(&summary.path, summary.kind, current_depth);
221+
} else {
222+
// Cross-crate item.
223+
return self.resolve_cross_crate_url(&summary.path, current_depth);
224+
}
225+
}
226+
227+
// Not in paths - check if it's a method/associated item in the index.
228+
if let Some(item) = self.krate.index.get(id) {
229+
if matches!(item.inner, ItemEnum::Function(_)) {
230+
return self.resolve_method_url(id, item, current_depth);
231+
}
232+
}
233+
234+
None
235+
}
236+
237+
/// Resolve an enum variant to a URL with #variant.Name anchor.
238+
fn resolve_variant_url(
239+
&self,
240+
variant_path: &[String],
241+
crate_id: u32,
242+
current_depth: usize,
243+
) -> Option<String> {
244+
if variant_path.len() < 2 {
245+
return None;
246+
}
247+
// Path is like ["core", "result", "Result", "Ok"].
248+
// Parent enum path is everything except last segment.
249+
let enum_path = &variant_path[..variant_path.len() - 1];
250+
let variant_name = variant_path.last()?;
251+
252+
let enum_url = if crate_id == 0 {
253+
self.build_item_url(enum_path, ItemKind::Enum, current_depth)?
254+
} else {
255+
self.resolve_cross_crate_url(enum_path, current_depth)
256+
.or_else(|| self.build_item_url(enum_path, ItemKind::Enum, current_depth))?
257+
};
258+
259+
Some(format!("{}#variant.{}", enum_url, variant_name))
260+
}
261+
262+
/// Resolve a method/associated function to a URL with #method.name anchor.
263+
fn resolve_method_url(
264+
&self,
265+
method_id: &Id,
266+
method_item: &rustdoc_types::Item,
267+
current_depth: usize,
268+
) -> Option<String> {
269+
let method_name = method_item.name.as_deref()?;
270+
271+
// Find the parent type by searching impl blocks.
272+
for (_impl_id, impl_item) in &self.krate.index {
273+
let ItemEnum::Impl(impl_) = &impl_item.inner else {
274+
continue;
275+
};
276+
if !impl_.items.contains(method_id) {
277+
continue;
278+
}
279+
// Found the impl containing this method.
280+
// Get the type ID from the impl's for_ type.
281+
if let Some(type_id) = get_type_id(&impl_.for_) {
282+
if let Some(type_url) = self.resolve_reexport_url(type_id, current_depth) {
283+
// Strip any existing fragment.
284+
let base_url = type_url.split('#').next().unwrap_or(&type_url);
285+
return Some(format!("{}#method.{}", base_url, method_name));
286+
}
287+
}
288+
break;
289+
}
290+
291+
None
292+
}
293+
294+
/// Find a re-export URL for a local item by checking if it appears as a
295+
/// Use target in the module tree.
296+
fn find_reexport_url(
297+
&self,
298+
target_id: &Id,
299+
kind: ItemKind,
300+
current_depth: usize,
301+
) -> Option<String> {
302+
// Walk the module tree looking for Use items targeting this ID.
303+
self.find_reexport_in_tree(&self.module_tree, target_id, kind, current_depth)
304+
}
305+
306+
fn find_reexport_in_tree(
307+
&self,
308+
tree: &crate::types::ModuleTree,
309+
target_id: &Id,
310+
kind: ItemKind,
311+
current_depth: usize,
312+
) -> Option<String> {
313+
for item in &tree.items {
314+
if let ItemEnum::Use(use_item) = &item.item.inner {
315+
if use_item.id.as_ref() == Some(target_id) {
316+
return self.build_item_url(&item.path, kind, current_depth);
317+
}
318+
}
319+
}
320+
for sub in &tree.submodules {
321+
if let Some(url) = self.find_reexport_in_tree(sub, target_id, kind, current_depth) {
322+
return Some(url);
323+
}
324+
}
325+
None
326+
}
327+
180328
/// Render markdown to HTML.
181329
pub fn render_markdown(&self, md: &str) -> String {
182330
markdown::render_markdown(md, &self.highlighter)
183331
}
184332

185333
/// Render markdown to HTML with intra-doc link resolution.
186334
pub fn render_markdown_with_links(&self, md: &str, current_depth: usize) -> String {
335+
let empty = HashMap::new();
336+
self.render_markdown_with_item_links(md, current_depth, &empty)
337+
}
338+
339+
/// Render markdown to HTML with intra-doc link resolution and pre-resolved item links.
340+
pub fn render_markdown_with_item_links(
341+
&self,
342+
md: &str,
343+
current_depth: usize,
344+
pre_resolved_links: &HashMap<String, String>,
345+
) -> String {
187346
markdown::render_markdown_with_links(
188347
md,
189348
&self.highlighter,
190349
self.global_index,
191350
self.crate_name(),
192351
current_depth,
352+
pre_resolved_links,
193353
)
194354
}
195355

196356
/// Render a short documentation string (first paragraph only) as inline HTML.
197357
pub fn render_short_doc(&self, full_docs: &str, current_depth: usize) -> String {
358+
let empty = HashMap::new();
359+
self.render_short_doc_with_item_links(full_docs, current_depth, &empty)
360+
}
361+
362+
/// Render a short doc string with pre-resolved item links.
363+
pub fn render_short_doc_with_item_links(
364+
&self,
365+
full_docs: &str,
366+
current_depth: usize,
367+
pre_resolved_links: &HashMap<String, String>,
368+
) -> String {
198369
markdown::render_short_doc(
199370
full_docs,
200371
&self.highlighter,
201372
self.global_index,
202373
self.crate_name(),
203374
current_depth,
375+
pre_resolved_links,
204376
)
205377
}
206378

crates/rustmax-rustdoc/src/render/module.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,15 @@ pub fn render_module(ctx: &RenderContext, tree: &ModuleTree) -> AnyResult<String
4242
let depth = path.len();
4343
let path_to_root = if depth == 0 { String::new() } else { "../".repeat(depth) };
4444

45+
// Pre-resolve links from the module item's links field.
46+
let item_links = tree.module_item.as_ref()
47+
.map(|m| ctx.resolve_item_links(&m.item.links, depth))
48+
.unwrap_or_default();
49+
4550
// Module documentation.
4651
let docs = tree.module_item.as_ref()
4752
.and_then(|m| m.item.docs.as_ref())
48-
.map(|d| ctx.render_markdown_with_links(d, depth))
53+
.map(|d| ctx.render_markdown_with_item_links(d, depth, &item_links))
4954
.unwrap_or_default();
5055
tera_ctx.insert("docs", &docs);
5156

@@ -84,24 +89,24 @@ pub fn render_module(ctx: &RenderContext, tree: &ModuleTree) -> AnyResult<String
8489

8590
// Determine link URL.
8691
let item_url = match ctx.reexport_target(target_id) {
87-
ReexportTarget::LocalPublic { ref path, kind }
88-
| ReexportTarget::External { ref path, kind } => {
89-
// Link to original.
92+
ReexportTarget::External { ref path, kind } => {
93+
// Link to external crate's page.
9094
ctx.build_item_url(path, kind, depth).unwrap_or_default()
9195
}
92-
ReexportTarget::NeedsPage => {
93-
// Link to re-export's page.
96+
ReexportTarget::LocalPublic { .. } | ReexportTarget::NeedsPage => {
97+
// Link to re-export's page at re-export location.
9498
format!("{}{}", path_to_root, item.html_path.display())
9599
}
96100
};
97101

98102
// Try to get target from index first, then fall back to paths.
99103
if let Some(target_item) = ctx.krate.index.get(target_id) {
104+
let target_links = ctx.resolve_item_links(&target_item.links, depth);
100105
let summary = ItemSummary {
101106
name: use_item.name.clone(),
102107
path: item_url,
103108
short_doc: target_item.docs.as_ref()
104-
.map(|d| ctx.render_short_doc(d, depth))
109+
.map(|d| ctx.render_short_doc_with_item_links(d, depth, &target_links))
105110
.unwrap_or_default(),
106111
};
107112

crates/rustmax-rustdoc/src/types.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,18 @@ fn build_tree_recursive<'a>(
145145
submodules.push(subtree);
146146
}
147147
_ => {
148-
// Regular item.
149-
let html_path = path_to_html(&child_path, item_kind(&child_item.inner));
148+
// Regular item. For re-exports, determine kind from target.
149+
let kind = match &child_item.inner {
150+
ItemEnum::Use(use_item) => {
151+
use_item.id.as_ref().and_then(|target_id| {
152+
krate.index.get(target_id)
153+
.and_then(|t| item_kind(&t.inner))
154+
.or_else(|| krate.paths.get(target_id).map(|s| s.kind))
155+
})
156+
}
157+
other => item_kind(other),
158+
};
159+
let html_path = path_to_html(&child_path, kind);
150160
items.push(RenderableItem {
151161
id: child_id,
152162
item: child_item,
@@ -336,7 +346,7 @@ pub fn build_impl_index<'a>(krate: &'a Crate) -> ImplIndex<'a> {
336346
}
337347

338348
/// Try to extract the ID from a Type (works for resolved paths).
339-
fn get_type_id(ty: &Type) -> Option<&Id> {
349+
pub fn get_type_id(ty: &Type) -> Option<&Id> {
340350
match ty {
341351
Type::ResolvedPath(path) => Some(&path.id),
342352
_ => None,

0 commit comments

Comments
 (0)