Clean up RelPath API (#38912)

Consolidate constructors and accessors.

Release Notes:

- N/A

---------

Co-authored-by: Cole Miller <cole@zed.dev>

crates/util/src/rel_path.rs

@@ -1,9 +1,8 @@
 use crate::paths::{PathStyle, is_absolute};
-use anyhow::{Context as _, Result, anyhow, bail};
+use anyhow::{Context as _, Result, anyhow};
 use serde::{Deserialize, Serialize};
 use std::{
-    borrow::Cow,
-    ffi::OsStr,
+    borrow::{Borrow, Cow},
     fmt,
     ops::Deref,
     path::{Path, PathBuf},
@@ -23,51 +22,71 @@ impl RelPath {
     }
 
     #[track_caller]
-    pub fn new<S: AsRef<str> + ?Sized>(s: &S) -> anyhow::Result<&Self> {
-        let this = unsafe { Self::new_unchecked(s) };
-        if this.0.starts_with("/")
-            || this.0.ends_with("/")
-            || this
-                .components()
-                .any(|component| component == ".." || component == "." || component.is_empty())
+    pub fn new<'a>(path: &'a Path, path_style: PathStyle) -> Result<Cow<'a, Self>> {
+        let mut path = path.to_str().context("non utf-8 path")?;
+
+        let (prefixes, suffixes): (&[_], &[_]) = match path_style {
+            PathStyle::Posix => (&["./"], &['/']),
+            PathStyle::Windows => (&["./", ".\\"], &['/', '\\']),
+        };
+
+        while prefixes.iter().any(|prefix| path.starts_with(prefix)) {
+            path = &path[prefixes[0].len()..];
+        }
+        while let Some(prefix) = path.strip_suffix(suffixes)
+            && !prefix.is_empty()
         {
-            bail!("invalid relative path: {:?}", &this.0);
+            path = prefix;
         }
-        Ok(this)
-    }
 
-    #[track_caller]
-    pub fn from_std_path(path: &Path, path_style: PathStyle) -> Result<Arc<Self>> {
-        let path = path.to_str().context("non utf-8 path")?;
-        let mut string = Cow::Borrowed(path);
-
-        if is_absolute(&string, path_style) {
+        if is_absolute(&path, path_style) {
             return Err(anyhow!("absolute path not allowed: {path:?}"));
         }
 
-        if path_style == PathStyle::Windows {
+        let mut string = Cow::Borrowed(path);
+        if path_style == PathStyle::Windows && path.contains('\\') {
             string = Cow::Owned(string.as_ref().replace('\\', "/"))
         }
 
-        let mut this = RelPathBuf::new();
-        for component in unsafe { Self::new_unchecked(string.as_ref()) }.components() {
-            match component {
-                "" => {}
-                "." => {}
-                ".." => {
-                    if !this.pop() {
-                        return Err(anyhow!("path is not relative: {string:?}"));
+        let mut result = match string {
+            Cow::Borrowed(string) => Cow::Borrowed(unsafe { Self::new_unchecked(string) }),
+            Cow::Owned(string) => Cow::Owned(RelPathBuf(string)),
+        };
+
+        if result
+            .components()
+            .any(|component| component == "" || component == "." || component == "..")
+        {
+            let mut normalized = RelPathBuf::new();
+            for component in result.components() {
+                match component {
+                    "" => {}
+                    "." => {}
+                    ".." => {
+                        if !normalized.pop() {
+                            return Err(anyhow!("path is not relative: {result:?}"));
+                        }
                     }
+                    other => normalized.push(&RelPath::unix(other)?),
                 }
-                other => this.push(RelPath::new(other)?),
             }
+            result = Cow::Owned(normalized)
         }
 
-        Ok(this.into())
+        Ok(result)
     }
 
-    pub unsafe fn new_unchecked<S: AsRef<str> + ?Sized>(s: &S) -> &Self {
-        unsafe { &*(s.as_ref() as *const str as *const Self) }
+    #[track_caller]
+    pub fn unix<S: AsRef<Path> + ?Sized>(path: &S) -> anyhow::Result<&Self> {
+        let path = path.as_ref();
+        match Self::new(path, PathStyle::Posix)? {
+            Cow::Borrowed(path) => Ok(path),
+            Cow::Owned(_) => Err(anyhow!("invalid relative path {path:?}")),
+        }
+    }
+
+    unsafe fn new_unchecked(s: &str) -> &Self {
+        unsafe { &*(s as *const str as *const Self) }
     }
 
     pub fn is_empty(&self) -> bool {
@@ -115,7 +134,7 @@ impl RelPath {
         false
     }
 
-    pub fn strip_prefix(&self, other: &Self) -> Result<&Self> {
+    pub fn strip_prefix<'a>(&'a self, other: &Self) -> Result<&'a Self> {
         if other.is_empty() {
             return Ok(self);
         }
@@ -146,21 +165,6 @@ impl RelPath {
         }
     }
 
-    pub fn push(&self, component: &str) -> Result<Arc<Self>> {
-        if component.is_empty() {
-            bail!("pushed component is empty");
-        } else if component.contains('/') {
-            bail!("pushed component contains a separator: {component:?}");
-        }
-        let path = format!(
-            "{}{}{}",
-            &self.0,
-            if self.is_empty() { "" } else { "/" },
-            component
-        );
-        Ok(Arc::from(unsafe { Self::new_unchecked(&path) }))
-    }
-
     pub fn join(&self, other: &Self) -> Arc<Self> {
         let result = if self.0.is_empty() {
             Cow::Borrowed(&other.0)
@@ -173,7 +177,7 @@ impl RelPath {
     }
 
     pub fn to_proto(&self) -> String {
-        self.0.to_owned()
+        self.as_unix_str().to_owned()
     }
 
     pub fn to_rel_path_buf(&self) -> RelPathBuf {
@@ -181,13 +185,17 @@ impl RelPath {
     }
 
     pub fn from_proto(path: &str) -> Result<Arc<Self>> {
-        Ok(Arc::from(Self::new(path)?))
+        Ok(Arc::from(Self::unix(path)?))
     }
 
-    pub fn as_str(&self) -> &str {
+    pub fn as_unix_str(&self) -> &str {
         &self.0
     }
 
+    pub fn into_arc(&self) -> Arc<Self> {
+        Arc::from(self)
+    }
+
     pub fn display(&self, style: PathStyle) -> Cow<'_, str> {
         match style {
             PathStyle::Posix => Cow::Borrowed(&self.0),
@@ -195,19 +203,25 @@ impl RelPath {
         }
     }
 
-    pub fn as_bytes(&self) -> &[u8] {
-        &self.0.as_bytes()
-    }
-
-    pub fn as_os_str(&self) -> &OsStr {
-        self.0.as_ref()
-    }
-
     pub fn as_std_path(&self) -> &Path {
         Path::new(&self.0)
     }
 }
 
+impl ToOwned for RelPath {
+    type Owned = RelPathBuf;
+
+    fn to_owned(&self) -> Self::Owned {
+        self.to_rel_path_buf()
+    }
+}
+
+impl Borrow<RelPath> for RelPathBuf {
+    fn borrow(&self) -> &RelPath {
+        self.as_rel_path()
+    }
+}
+
 impl PartialOrd for RelPath {
     fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
         Some(self.cmp(other))
@@ -293,9 +307,9 @@ impl Deref for RelPathBuf {
     }
 }
 
-impl AsRef<Path> for RelPath {
-    fn as_ref(&self) -> &Path {
-        Path::new(&self.0)
+impl<'a> From<&'a RelPath> for Cow<'a, RelPath> {
+    fn from(value: &'a RelPath) -> Self {
+        Self::Borrowed(value)
     }
 }
 
@@ -309,7 +323,7 @@ impl From<&RelPath> for Arc<RelPath> {
 #[cfg(any(test, feature = "test-support"))]
 #[track_caller]
 pub fn rel_path(path: &str) -> &RelPath {
-    RelPath::new(path).unwrap()
+    RelPath::unix(path).unwrap()
 }
 
 impl PartialEq<str> for RelPath {
@@ -384,35 +398,52 @@ impl<'a> DoubleEndedIterator for RelPathComponents<'a> {
 mod tests {
     use super::*;
     use itertools::Itertools;
-    use std::path::PathBuf;
+    use pretty_assertions::assert_matches;
 
     #[test]
-    fn test_path_construction() {
-        assert!(RelPath::new("/").is_err());
-        assert!(RelPath::new("/foo").is_err());
-        assert!(RelPath::new("foo/").is_err());
-        assert!(RelPath::new("foo//bar").is_err());
-        assert!(RelPath::new("foo/../bar").is_err());
-        assert!(RelPath::new("./foo/bar").is_err());
-        assert!(RelPath::new("..").is_err());
+    fn test_rel_path_new() {
+        assert!(RelPath::new(Path::new("/"), PathStyle::local()).is_err());
+        assert!(RelPath::new(Path::new("//"), PathStyle::local()).is_err());
+        assert!(RelPath::new(Path::new("/foo/"), PathStyle::local()).is_err());
 
-        assert!(RelPath::from_std_path(Path::new("/"), PathStyle::local()).is_err());
-        assert!(RelPath::from_std_path(Path::new("//"), PathStyle::local()).is_err());
-        assert!(RelPath::from_std_path(Path::new("/foo/"), PathStyle::local()).is_err());
-        assert_eq!(
-            RelPath::from_std_path(&PathBuf::from_iter(["foo", ""]), PathStyle::local()).unwrap(),
-            Arc::from(rel_path("foo"))
-        );
-    }
+        let path = RelPath::new("foo/".as_ref(), PathStyle::local()).unwrap();
+        assert_eq!(path, rel_path("foo").into());
+        assert_matches!(path, Cow::Borrowed(_));
+
+        let path = RelPath::new("foo\\".as_ref(), PathStyle::Windows).unwrap();
+        assert_eq!(path, rel_path("foo").into());
+        assert_matches!(path, Cow::Borrowed(_));
 
-    #[test]
-    fn test_rel_path_from_std_path() {
         assert_eq!(
-            RelPath::from_std_path(Path::new("foo/bar/../baz/./quux/"), PathStyle::local())
+            RelPath::new("foo/bar/../baz/./quux/".as_ref(), PathStyle::local())
                 .unwrap()
                 .as_ref(),
             rel_path("foo/baz/quux")
         );
+
+        let path = RelPath::new("./foo/bar".as_ref(), PathStyle::Posix).unwrap();
+        assert_eq!(path.as_ref(), rel_path("foo/bar"));
+        assert_matches!(path, Cow::Borrowed(_));
+
+        let path = RelPath::new(".\\foo".as_ref(), PathStyle::Windows).unwrap();
+        assert_eq!(path, rel_path("foo").into());
+        assert_matches!(path, Cow::Borrowed(_));
+
+        let path = RelPath::new("./.\\./foo/\\/".as_ref(), PathStyle::Windows).unwrap();
+        assert_eq!(path, rel_path("foo").into());
+        assert_matches!(path, Cow::Borrowed(_));
+
+        let path = RelPath::new("foo/./bar".as_ref(), PathStyle::Posix).unwrap();
+        assert_eq!(path.as_ref(), rel_path("foo/bar"));
+        assert_matches!(path, Cow::Owned(_));
+
+        let path = RelPath::new("./foo/bar".as_ref(), PathStyle::Windows).unwrap();
+        assert_eq!(path.as_ref(), rel_path("foo/bar"));
+        assert_matches!(path, Cow::Borrowed(_));
+
+        let path = RelPath::new(".\\foo\\bar".as_ref(), PathStyle::Windows).unwrap();
+        assert_eq!(path.as_ref(), rel_path("foo/bar"));
+        assert_matches!(path, Cow::Owned(_));
     }
 
     #[test]
@@ -456,10 +487,7 @@ mod tests {
 
     #[test]
     fn test_rel_path_parent() {
-        assert_eq!(
-            rel_path("foo/bar/baz").parent(),
-            Some(RelPath::new("foo/bar").unwrap())
-        );
+        assert_eq!(rel_path("foo/bar/baz").parent(), Some(rel_path("foo/bar")));
         assert_eq!(rel_path("foo").parent(), Some(RelPath::empty()));
         assert_eq!(rel_path("").parent(), None);
     }
@@ -470,7 +498,9 @@ mod tests {
         for [lhs, rhs] in test_cases.iter().array_combinations::<2>() {
             assert_eq!(
                 Path::new(lhs).cmp(Path::new(rhs)),
-                RelPath::new(lhs).unwrap().cmp(RelPath::new(rhs).unwrap())
+                RelPath::unix(lhs)
+                    .unwrap()
+                    .cmp(&RelPath::unix(rhs).unwrap())
             );
         }
     }
@@ -486,30 +516,22 @@ mod tests {
 
     #[test]
     fn test_rel_path_constructors_absolute_path() {
-        assert!(RelPath::from_std_path(Path::new("/a/b"), PathStyle::Windows).is_err());
-        assert!(RelPath::from_std_path(Path::new("\\a\\b"), PathStyle::Windows).is_err());
-        assert!(RelPath::from_std_path(Path::new("/a/b"), PathStyle::Posix).is_err());
-        assert!(RelPath::from_std_path(Path::new("C:/a/b"), PathStyle::Windows).is_err());
-        assert!(RelPath::from_std_path(Path::new("C:\\a\\b"), PathStyle::Windows).is_err());
-        assert!(RelPath::from_std_path(Path::new("C:/a/b"), PathStyle::Posix).is_ok());
-    }
-
-    #[test]
-    fn test_push() {
-        assert_eq!(rel_path("a/b").push("c").unwrap().as_str(), "a/b/c");
-        assert_eq!(rel_path("").push("c").unwrap().as_str(), "c");
-        assert!(rel_path("a/b").push("").is_err());
-        assert!(rel_path("a/b").push("c/d").is_err());
+        assert!(RelPath::new(Path::new("/a/b"), PathStyle::Windows).is_err());
+        assert!(RelPath::new(Path::new("\\a\\b"), PathStyle::Windows).is_err());
+        assert!(RelPath::new(Path::new("/a/b"), PathStyle::Posix).is_err());
+        assert!(RelPath::new(Path::new("C:/a/b"), PathStyle::Windows).is_err());
+        assert!(RelPath::new(Path::new("C:\\a\\b"), PathStyle::Windows).is_err());
+        assert!(RelPath::new(Path::new("C:/a/b"), PathStyle::Posix).is_ok());
     }
 
     #[test]
     fn test_pop() {
         let mut path = rel_path("a/b").to_rel_path_buf();
         path.pop();
-        assert_eq!(path.as_rel_path().as_str(), "a");
+        assert_eq!(path.as_rel_path().as_unix_str(), "a");
         path.pop();
-        assert_eq!(path.as_rel_path().as_str(), "");
+        assert_eq!(path.as_rel_path().as_unix_str(), "");
         path.pop();
-        assert_eq!(path.as_rel_path().as_str(), "");
+        assert_eq!(path.as_rel_path().as_unix_str(), "");
     }
 }