Rollup merge of #106466 - clubby789:relative-module-fix, r=notriddle

Fix rustdoc source code rendering for `#[path = "../path/to/mod.rs"]` links

Fixes #103517

While generating the location for modules source HTML to be saved at, a `..` path component appeared to be translated to `/up/`.
Additionally, while generating the navigation sidebar, `..` path components were ignored. This means that (as in the issue above), a *real* directory structure of:
```
sys/
  unix/
    mod.rs  <-- contains #![path = "../unix/mod.rs]
    cmath.rs
```
was rendered as:
```
sys/
  unix/
    mod.rs
    unix/
      cmath.rs  <-- links to sys/unix/unix/cmath.rs.html, 404
```
While the *files* were stored as
```
sys/
  unix/
    mod.rs.html
    up/
      unix/
        cmath.rs.html
```

src/librustdoc/html/render/context.rs

@@ -309,7 +309,7 @@ impl<'tcx> Context<'tcx> {
 
     pub(crate) fn href_from_span(&self, span: clean::Span, with_lines: bool) -> Option<String> {
         let mut root = self.root_path();
-        let mut path = String::new();
+        let mut path: String;
         let cnum = span.cnum(self.sess());
 
         // We can safely ignore synthetic `SourceFile`s.
@@ -340,10 +340,24 @@ impl<'tcx> Context<'tcx> {
                 ExternalLocation::Unknown => return None,
             };
 
-            sources::clean_path(&src_root, file, false, |component| {
+            let href = RefCell::new(PathBuf::new());
-                path.push_str(&component.to_string_lossy());
+            sources::clean_path(
+                &src_root,
+                file,
+                |component| {
+                    href.borrow_mut().push(component);
+                },
+                || {
+                    href.borrow_mut().pop();
+                },
+            );
+
+            path = href.into_inner().to_string_lossy().to_string();
+
+            if let Some(c) = path.as_bytes().last() && *c != b'/' {
                 path.push('/');
-            });
+            }
+
             let mut fname = file.file_name().expect("source has no filename").to_os_string();
             fname.push(".html");
             path.push_str(&fname.to_string_lossy());

src/librustdoc/html/render/write_shared.rs

@@ -1,8 +1,9 @@
+use std::cell::RefCell;
 use std::fs::{self, File};
 use std::io::prelude::*;
 use std::io::{self, BufReader};
 use std::path::{Component, Path};
-use std::rc::Rc;
+use std::rc::{Rc, Weak};
 
 use itertools::Itertools;
 use rustc_data_structures::flock;
@@ -184,23 +185,26 @@ pub(super) fn write_shared(
 
     use std::ffi::OsString;
 
-    #[derive(Debug)]
+    #[derive(Debug, Default)]
     struct Hierarchy {
+        parent: Weak<Self>,
         elem: OsString,
-        children: FxHashMap<OsString, Hierarchy>,
+        children: RefCell<FxHashMap<OsString, Rc<Self>>>,
-        elems: FxHashSet<OsString>,
+        elems: RefCell<FxHashSet<OsString>>,
     }
 
     impl Hierarchy {
-        fn new(elem: OsString) -> Hierarchy {
+        fn with_parent(elem: OsString, parent: &Rc<Self>) -> Self {
-            Hierarchy { elem, children: FxHashMap::default(), elems: FxHashSet::default() }
+            Self { elem, parent: Rc::downgrade(parent), ..Self::default() }
         }
 
         fn to_json_string(&self) -> String {
-            let mut subs: Vec<&Hierarchy> = self.children.values().collect();
+            let borrow = self.children.borrow();
+            let mut subs: Vec<_> = borrow.values().collect();
             subs.sort_unstable_by(|a, b| a.elem.cmp(&b.elem));
             let mut files = self
                 .elems
+                .borrow()
                 .iter()
                 .map(|s| format!("\"{}\"", s.to_str().expect("invalid osstring conversion")))
                 .collect::<Vec<_>>();
@@ -220,36 +224,52 @@ pub(super) fn write_shared(
                 files = files
             )
         }
+
+        fn add_path(self: &Rc<Self>, path: &Path) {
+            let mut h = Rc::clone(&self);
+            let mut elems = path
+                .components()
+                .filter_map(|s| match s {
+                    Component::Normal(s) => Some(s.to_owned()),
+                    Component::ParentDir => Some(OsString::from("..")),
+                    _ => None,
+                })
+                .peekable();
+            loop {
+                let cur_elem = elems.next().expect("empty file path");
+                if cur_elem == ".." {
+                    if let Some(parent) = h.parent.upgrade() {
+                        h = parent;
+                    }
+                    continue;
+                }
+                if elems.peek().is_none() {
+                    h.elems.borrow_mut().insert(cur_elem);
+                    break;
+                } else {
+                    let entry = Rc::clone(
+                        h.children
+                            .borrow_mut()
+                            .entry(cur_elem.clone())
+                            .or_insert_with(|| Rc::new(Self::with_parent(cur_elem, &h))),
+                    );
+                    h = entry;
+                }
+            }
+        }
     }
 
     if cx.include_sources {
-        let mut hierarchy = Hierarchy::new(OsString::new());
+        let hierarchy = Rc::new(Hierarchy::default());
         for source in cx
             .shared
             .local_sources
             .iter()
             .filter_map(|p| p.0.strip_prefix(&cx.shared.src_root).ok())
         {
-            let mut h = &mut hierarchy;
+            hierarchy.add_path(source);
-            let mut elems = source
-                .components()
-                .filter_map(|s| match s {
-                    Component::Normal(s) => Some(s.to_owned()),
-                    _ => None,
-                })
-                .peekable();
-            loop {
-                let cur_elem = elems.next().expect("empty file path");
-                if elems.peek().is_none() {
-                    h.elems.insert(cur_elem);
-                    break;
-                } else {
-                    let e = cur_elem.clone();
-                    h = h.children.entry(cur_elem.clone()).or_insert_with(|| Hierarchy::new(e));
-                }
-            }
         }
-
+        let hierarchy = Rc::try_unwrap(hierarchy).unwrap();
         let dst = cx.dst.join(&format!("source-files{}.js", cx.shared.resource_suffix));
         let make_sources = || {
             let (mut all_sources, _krates) =

src/librustdoc/html/sources.rs

@@ -13,6 +13,7 @@ use rustc_middle::ty::TyCtxt;
 use rustc_session::Session;
 use rustc_span::source_map::FileName;
 
+use std::cell::RefCell;
 use std::ffi::OsStr;
 use std::fs;
 use std::path::{Component, Path, PathBuf};
@@ -72,12 +73,22 @@ impl LocalSourcesCollector<'_, '_> {
             return;
         }
 
-        let mut href = String::new();
+        let href = RefCell::new(PathBuf::new());
-        clean_path(self.src_root, &p, false, |component| {
+        clean_path(
-            href.push_str(&component.to_string_lossy());
+            &self.src_root,
-            href.push('/');
+            &p,
-        });
+            |component| {
+                href.borrow_mut().push(component);
+            },
+            || {
+                href.borrow_mut().pop();
+            },
+        );
 
+        let mut href = href.into_inner().to_string_lossy().to_string();
+        if let Some(c) = href.as_bytes().last() && *c != b'/' {
+            href.push('/');
+        }
         let mut src_fname = p.file_name().expect("source has no filename").to_os_string();
         src_fname.push(".html");
         href.push_str(&src_fname.to_string_lossy());
@@ -180,13 +191,28 @@ impl SourceCollector<'_, '_> {
 
         let shared = Rc::clone(&self.cx.shared);
         // Create the intermediate directories
-        let mut cur = self.dst.clone();
+        let cur = RefCell::new(PathBuf::new());
-        let mut root_path = String::from("../../");
+        let root_path = RefCell::new(PathBuf::new());
-        clean_path(&shared.src_root, &p, false, |component| {
-            cur.push(component);
-            root_path.push_str("../");
-        });
 
+        clean_path(
+            &shared.src_root,
+            &p,
+            |component| {
+                cur.borrow_mut().push(component);
+                root_path.borrow_mut().push("..");
+            },
+            || {
+                cur.borrow_mut().pop();
+                root_path.borrow_mut().pop();
+            },
+        );
+
+        let root_path = PathBuf::from("../../").join(root_path.into_inner());
+        let mut root_path = root_path.to_string_lossy();
+        if let Some(c) = root_path.as_bytes().last() && *c != b'/' {
+            root_path += "/";
+        }
+        let mut cur = self.dst.join(cur.into_inner());
         shared.ensure_dir(&cur)?;
 
         let src_fname = p.file_name().expect("source has no filename").to_os_string();
@@ -232,11 +258,13 @@ impl SourceCollector<'_, '_> {
 /// Takes a path to a source file and cleans the path to it. This canonicalizes
 /// things like ".." to components which preserve the "top down" hierarchy of a
 /// static HTML tree. Each component in the cleaned path will be passed as an
-/// argument to `f`. The very last component of the path (ie the file name) will
+/// argument to `f`. The very last component of the path (ie the file name) is ignored.
-/// be passed to `f` if `keep_filename` is true, and ignored otherwise.
+/// If a `..` is encountered, the `parent` closure will be called to allow the callee to
-pub(crate) fn clean_path<F>(src_root: &Path, p: &Path, keep_filename: bool, mut f: F)
+/// handle it.
+pub(crate) fn clean_path<F, P>(src_root: &Path, p: &Path, mut f: F, mut parent: P)
 where
     F: FnMut(&OsStr),
+    P: FnMut(),
 {
     // make it relative, if possible
     let p = p.strip_prefix(src_root).unwrap_or(p);
@@ -244,12 +272,12 @@ where
     let mut iter = p.components().peekable();
 
     while let Some(c) = iter.next() {
-        if !keep_filename && iter.peek().is_none() {
+        if iter.peek().is_none() {
             break;
         }
 
         match c {
-            Component::ParentDir => f("up".as_ref()),
+            Component::ParentDir => parent(),
             Component::Normal(c) => f(c),
             _ => continue,
         }

src/test/rustdoc/src-links.rs

@@ -7,6 +7,11 @@
 #[path = "src-links/mod.rs"]
 pub mod qux;
 
+// @has src/foo/src-links.rs.html
+// @has foo/fizz/index.html '//a/@href' '../src/foo/src-links/fizz.rs.html'
+#[path = "src-links/../src-links/fizz.rs"]
+pub mod fizz;
+
 // @has foo/bar/index.html '//a/@href' '../../src/foo/src-links.rs.html'
 pub mod bar {
 

src/test/rustdoc/src-links/fizz.rs

@@ -0,0 +1 @@
+pub struct Buzz;