From 016fc2b5cc965e061a6f2f44dae5b638f5473377 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Mon, 27 Nov 2023 12:26:19 +0900
Subject: [PATCH] repo_path: change .split() and .parent() to return &RepoPath

---
 lib/src/repo_path.rs    | 28 +++++++++++++---------------
 lib/src/tree_builder.rs | 16 ++++++++--------
 2 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/lib/src/repo_path.rs b/lib/src/repo_path.rs
index 461efa9fd..f38ede712 100644
--- a/lib/src/repo_path.rs
+++ b/lib/src/repo_path.rs
@@ -118,12 +118,10 @@ pub struct RepoPathComponentsIter<'a> {
     value: &'a str,
 }
 
-impl RepoPathComponentsIter<'_> {
-    // TODO: add borrowed RepoPath type and implement as_path() instead
-    fn to_path(&self) -> RepoPathBuf {
-        RepoPathBuf {
-            value: self.value.to_owned(),
-        }
+impl<'a> RepoPathComponentsIter<'a> {
+    /// Returns the remaining part as repository path.
+    pub fn as_path(&self) -> &'a RepoPath {
+        RepoPath::from_internal_string_unchecked(self.value)
     }
 }
 
@@ -315,16 +313,16 @@ impl RepoPath {
         }
     }
 
-    // TODO: make it return borrowed RepoPath type
-    pub fn parent(&self) -> Option<RepoPathBuf> {
+    /// Returns the parent path without the base name component.
+    pub fn parent(&self) -> Option<&RepoPath> {
         self.split().map(|(parent, _)| parent)
     }
 
-    // TODO: make it return borrowed RepoPath type
-    pub fn split(&self) -> Option<(RepoPathBuf, &RepoPathComponent)> {
+    /// Splits this into the parent path and base name component.
+    pub fn split(&self) -> Option<(&RepoPath, &RepoPathComponent)> {
         let mut components = self.components();
         let basename = components.next_back()?;
-        Some((components.to_path(), basename))
+        Some((components.as_path(), basename))
     }
 
     pub fn components(&self) -> RepoPathComponentsIter<'_> {
@@ -527,8 +525,8 @@ mod tests {
         let subdir = dir.join(subdir_component);
 
         assert_eq!(root.parent(), None);
-        assert_eq!(dir.parent().as_deref(), Some(root));
-        assert_eq!(subdir.parent(), Some(dir));
+        assert_eq!(dir.parent(), Some(root));
+        assert_eq!(subdir.parent(), Some(dir.as_ref()));
     }
 
     #[test]
@@ -541,8 +539,8 @@ mod tests {
         let file = dir.join(file_component);
 
         assert_eq!(root.split(), None);
-        assert_eq!(dir.split(), Some((root.to_owned(), dir_component)));
-        assert_eq!(file.split(), Some((dir, file_component)));
+        assert_eq!(dir.split(), Some((root, dir_component)));
+        assert_eq!(file.split(), Some((dir.as_ref(), file_component)));
     }
 
     #[test]
diff --git a/lib/src/tree_builder.rs b/lib/src/tree_builder.rs
index 644f3ddfa..30ddfd33d 100644
--- a/lib/src/tree_builder.rs
+++ b/lib/src/tree_builder.rs
@@ -19,7 +19,7 @@ use std::sync::Arc;
 
 use crate::backend;
 use crate::backend::{TreeId, TreeValue};
-use crate::repo_path::RepoPathBuf;
+use crate::repo_path::{RepoPath, RepoPathBuf};
 use crate::store::Store;
 use crate::tree::Tree;
 
@@ -79,7 +79,7 @@ impl TreeBuilder {
         // Update entries in parent trees for file overrides
         for (path, file_override) in self.overrides {
             let (dir, basename) = path.split().unwrap();
-            let tree = trees_to_write.get_mut(&dir).unwrap();
+            let tree = trees_to_write.get_mut(dir).unwrap();
             match file_override {
                 Override::Replace(value) => {
                     tree.set(basename.to_owned(), value);
@@ -95,7 +95,7 @@ impl TreeBuilder {
         let store = &self.store;
         while let Some((dir, tree)) = trees_to_write.pop_last() {
             if let Some((parent, basename)) = dir.split() {
-                let parent_tree = trees_to_write.get_mut(&parent).unwrap();
+                let parent_tree = trees_to_write.get_mut(parent).unwrap();
                 if tree.is_empty() {
                     if let Some(TreeValue::Tree(_)) = parent_tree.value(basename) {
                         parent_tree.remove(basename);
@@ -127,17 +127,17 @@ impl TreeBuilder {
         fn populate_trees<'a>(
             tree_cache: &'a mut BTreeMap<RepoPathBuf, Tree>,
             store: &Arc<Store>,
-            dir: RepoPathBuf,
+            dir: &RepoPath,
         ) -> &'a Tree {
             // `if let Some(tree) = ...` doesn't pass lifetime check as of Rust 1.69.0
-            if tree_cache.contains_key(&dir) {
-                return tree_cache.get(&dir).unwrap();
+            if tree_cache.contains_key(dir) {
+                return tree_cache.get(dir).unwrap();
             }
             let (parent, basename) = dir.split().expect("root must be populated");
             let tree = populate_trees(tree_cache, store, parent)
                 .sub_tree(basename)
-                .unwrap_or_else(|| Tree::null(store.clone(), dir.clone()));
-            tree_cache.entry(dir).or_insert(tree)
+                .unwrap_or_else(|| Tree::null(store.clone(), dir.to_owned()));
+            tree_cache.entry(dir.to_owned()).or_insert(tree)
         }
 
         for path in self.overrides.keys() {