Skip to content

Commit 372a8c7

Browse files
committed
Fix relative extern URL depth on source pages
Signed-off-by: arferreira <arfs.antonio@gmail.com>
1 parent 6a979b3 commit 372a8c7

File tree

2 files changed

+40
-13
lines changed

2 files changed

+40
-13
lines changed

src/librustdoc/html/format.rs

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,8 @@ fn generate_macro_def_id_path(
403403

404404
let url = match cache.extern_locations[&def_id.krate] {
405405
ExternalLocation::Remote { ref url, is_absolute } => {
406-
let mut prefix = remote_url_prefix(url, is_absolute, cx.current.len());
406+
let depth = remote_item_depth(root_path, cx.current.len());
407+
let mut prefix = remote_url_prefix(url, is_absolute, depth);
407408
prefix.extend(path.iter().copied());
408409
prefix.finish()
409410
}
@@ -460,8 +461,8 @@ fn generate_item_def_id_path(
460461
let shortty = ItemType::from_def_id(def_id, tcx);
461462
let module_fqp = to_module_fqp(shortty, &fqp);
462463

463-
let (parts, is_absolute) = url_parts(cx.cache(), def_id, module_fqp, &cx.current)?;
464-
let mut url = make_href(root_path, shortty, parts, &fqp, is_absolute);
464+
let (parts, is_remote) = url_parts(cx.cache(), def_id, module_fqp, &cx.current, root_path)?;
465+
let mut url = make_href(root_path, shortty, parts, &fqp, is_remote);
465466

466467
if def_id != original_def_id {
467468
let kind = ItemType::from_def_id(original_def_id, tcx);
@@ -494,6 +495,16 @@ fn to_module_fqp(shortty: ItemType, fqp: &[Symbol]) -> &[Symbol] {
494495
if shortty == ItemType::Module { fqp } else { &fqp[..fqp.len() - 1] }
495496
}
496497

498+
/// Computes the directory depth for relative extern URL resolution.
499+
///
500+
/// Doc pages use `relative_to.len()` (the module nesting depth).
501+
/// Source pages (jump-to-def) live under `src/<crate_name>/` — a different directory
502+
/// hierarchy — so their depth is derived from `root_path` (e.g. `../../` → depth 2)
503+
/// rather than the module path.
504+
fn remote_item_depth(root_path: Option<&str>, doc_depth: usize) -> usize {
505+
root_path.map(|rp| rp.matches("../").count()).unwrap_or(doc_depth)
506+
}
507+
497508
fn remote_url_prefix(url: &str, is_absolute: bool, depth: usize) -> UrlPartsBuilder {
498509
let url = url.trim_end_matches('/');
499510
if is_absolute {
@@ -506,17 +517,27 @@ fn remote_url_prefix(url: &str, is_absolute: bool, depth: usize) -> UrlPartsBuil
506517
}
507518
}
508519

520+
/// Returns `(url_parts, is_remote)` where `is_remote` indicates the URL points to an
521+
/// external location and should not be prefixed with `root_path`.
522+
///
523+
/// `root_path` is provided when generating URLs from source pages (jump-to-def).
524+
/// Source pages live under `src/<crate_name>/` and use a different depth than doc pages,
525+
/// so when present, we derive the depth from `root_path` instead of `relative_to`.
509526
fn url_parts(
510527
cache: &Cache,
511528
def_id: DefId,
512529
module_fqp: &[Symbol],
513530
relative_to: &[Symbol],
531+
root_path: Option<&str>,
514532
) -> Result<(UrlPartsBuilder, bool), HrefError> {
515533
match cache.extern_locations[&def_id.krate] {
516534
ExternalLocation::Remote { ref url, is_absolute } => {
517-
let mut builder = remote_url_prefix(url, is_absolute, relative_to.len());
535+
let depth = remote_item_depth(root_path, relative_to.len());
536+
let mut builder = remote_url_prefix(url, is_absolute, depth);
518537
builder.extend(module_fqp.iter().copied());
519-
Ok((builder, is_absolute))
538+
// Remote URLs (both absolute and relative) already encode the correct path;
539+
// they must not be prefixed with root_path.
540+
Ok((builder, true))
520541
}
521542
ExternalLocation::Local => Ok((href_relative_parts(module_fqp, relative_to), false)),
522543
ExternalLocation::Unknown => Err(HrefError::DocumentationNotBuilt),
@@ -528,10 +549,11 @@ fn make_href(
528549
shortty: ItemType,
529550
mut url_parts: UrlPartsBuilder,
530551
fqp: &[Symbol],
531-
is_absolute: bool,
552+
is_remote: bool,
532553
) -> String {
533-
// FIXME: relative extern URLs may break when prefixed with root_path
534-
if !is_absolute && let Some(root_path) = root_path {
554+
// Remote URLs already have the correct depth via `remote_url_prefix`;
555+
// only local items need `root_path` to bridge from source pages to the doc tree.
556+
if !is_remote && let Some(root_path) = root_path {
535557
let root = root_path.trim_end_matches('/');
536558
url_parts.push_front(root);
537559
}
@@ -594,7 +616,7 @@ pub(crate) fn href_with_root_path(
594616
}
595617
}
596618

597-
let (fqp, shortty, url_parts, is_absolute) = match cache.paths.get(&did) {
619+
let (fqp, shortty, url_parts, is_remote) = match cache.paths.get(&did) {
598620
Some(&(ref fqp, shortty)) => (
599621
fqp,
600622
shortty,
@@ -612,8 +634,9 @@ pub(crate) fn href_with_root_path(
612634
let def_id_to_get = if root_path.is_some() { original_did } else { did };
613635
if let Some(&(ref fqp, shortty)) = cache.external_paths.get(&def_id_to_get) {
614636
let module_fqp = to_module_fqp(shortty, fqp);
615-
let (parts, is_absolute) = url_parts(cache, did, module_fqp, relative_to)?;
616-
(fqp, shortty, parts, is_absolute)
637+
let (parts, is_remote) =
638+
url_parts(cache, did, module_fqp, relative_to, root_path)?;
639+
(fqp, shortty, parts, is_remote)
617640
} else if matches!(def_kind, DefKind::Macro(_)) {
618641
return generate_macro_def_id_path(did, cx, root_path);
619642
} else if did.is_local() {
@@ -624,7 +647,7 @@ pub(crate) fn href_with_root_path(
624647
}
625648
};
626649
Ok(HrefInfo {
627-
url: make_href(root_path, shortty, url_parts, fqp, is_absolute),
650+
url: make_href(root_path, shortty, url_parts, fqp, is_remote),
628651
kind: shortty,
629652
rust_path: fqp.clone(),
630653
})

tests/rustdoc-html/extern/extern-html-root-url-relative.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ pub mod nested {
2121
pub mod intra_doc_link {
2222
}
2323

24-
// link-to-definition
24+
// link-to-definition: source pages live under src/<crate_name>/ so they need
25+
// ../../ to reach the doc root. Previously these links were incorrectly generated
26+
// as ../../../ (one level too deep) which passed the `has` check by substring match.
2527
//@ has src/extern_html_root_url_relative/extern-html-root-url-relative.rs.html
2628
//@ has - '//a/@href' '../../core/iter/index.html'
2729
//@ has - '//a/@href' '../../core/future/index.html'
30+
//@ !has - '//a/@href' '../../../core/iter/index.html'
31+
//@ !has - '//a/@href' '../../../core/future/index.html'

0 commit comments

Comments
 (0)