Correctly render sidebar for relative module paths

Author: clubby789

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| {
-                path.push_str(&component.to_string_lossy());
+            let href = RefCell::new(PathBuf::new());
+            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>,
-        elems: FxHashSet<OsString>,
+        children: RefCell<FxHashMap<OsString, Rc<Self>>>,
+        elems: RefCell<FxHashSet<OsString>>,
     }
 
     impl Hierarchy {
-        fn new(elem: OsString) -> Hierarchy {
-            Hierarchy { elem, children: FxHashMap::default(), elems: FxHashSet::default() }
+        fn with_parent(elem: OsString, parent: &Rc<Self>) -> Self {
+            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
             )
         }
-    }
 
-    if cx.include_sources {
-        let mut hierarchy = Hierarchy::new(OsString::new());
-        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;
-            let mut elems = source
+        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.insert(cur_elem);
+                    h.elems.borrow_mut().insert(cur_elem);
                     break;
                 } else {
-                    let e = cur_elem.clone();
-                    h = h.children.entry(cur_elem.clone()).or_insert_with(|| Hierarchy::new(e));
+                    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 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())
+        {
+            hierarchy.add_path(source);
+        }
+        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();
-        clean_path(self.src_root, &p, false, |component| {
-            href.push_str(&component.to_string_lossy());
-            href.push('/');
-        });
+        let href = RefCell::new(PathBuf::new());
+        clean_path(
+            &self.src_root,
+            &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 mut root_path = String::from("../../");
-        clean_path(&shared.src_root, &p, false, |component| {
-            cur.push(component);
-            root_path.push_str("../");
-        });
+        let cur = RefCell::new(PathBuf::new());
+        let root_path = RefCell::new(PathBuf::new());
+
+        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,24 +258,26 @@ 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
-/// be passed to `f` if `keep_filename` is true, and ignored otherwise.
-pub(crate) fn clean_path<F>(src_root: &Path, p: &Path, keep_filename: bool, mut f: F)
+/// argument to `f`. The very last component of the path (ie the file name) is ignored.
+/// If a `..` is encountered, the `parent` closure will be called to allow the callee to
+/// 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);
 
     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,
         }