[codex] Make AbsolutePathBuf joins infallible (#16981)

Having to check for errors every time join is called is painful and
unnecessary.

codex-rs/utils/absolute-path/src/absolutize.rs

@@ -0,0 +1,167 @@
+// Adapted from path-absolutize 3.1.1:
+// Copyright (c) 2018 magiclen.org (Ron Li)
+// Licensed under the MIT License.
+//
+// Keep this implementation local so explicit-base normalization can be
+// infallible for `AbsolutePathBuf::resolve_path_against_base` and
+// `AbsolutePathBuf::join`; only current-working-directory lookup remains
+// fallible.
+
+use std::path::Component;
+use std::path::Path;
+use std::path::PathBuf;
+
+pub(super) fn absolutize(path: &Path) -> std::io::Result<PathBuf> {
+    Ok(absolutize_from(path, &std::env::current_dir()?))
+}
+
+pub(super) fn absolutize_from(path: &Path, base_path: &Path) -> PathBuf {
+    normalize_path(&path_with_base(path, base_path))
+}
+
+fn normalize_path(path: &Path) -> PathBuf {
+    let mut normalized = PathBuf::new();
+    for component in path.components() {
+        match component {
+            Component::CurDir => {}
+            Component::ParentDir => {
+                normalized.pop();
+            }
+            Component::Prefix(_) | Component::RootDir | Component::Normal(_) => {
+                normalized.push(component.as_os_str());
+            }
+        }
+    }
+
+    if normalized.as_os_str().is_empty() {
+        PathBuf::from(".")
+    } else {
+        normalized
+    }
+}
+
+#[cfg(not(windows))]
+fn path_with_base(path: &Path, base_path: &Path) -> PathBuf {
+    if path.is_absolute() {
+        path.to_path_buf()
+    } else {
+        base_path.join(path)
+    }
+}
+
+#[cfg(windows)]
+fn path_with_base(path: &Path, base_path: &Path) -> PathBuf {
+    if path.is_absolute() || path.has_root() {
+        return base_path.join(path);
+    }
+
+    let mut components = path.components();
+    let Some(Component::Prefix(prefix)) = components.next() else {
+        return base_path.join(path);
+    };
+
+    let mut path = PathBuf::new();
+    path.push(prefix.as_os_str());
+
+    if components.clone().next().is_none() {
+        path.push(std::path::MAIN_SEPARATOR_STR);
+        return path;
+    }
+
+    let skip_base_prefix = matches!(base_path.components().next(), Some(Component::Prefix(_)));
+    for component in base_path
+        .components()
+        .skip(usize::from(skip_base_prefix))
+        .chain(components)
+    {
+        path.push(component.as_os_str());
+    }
+    path
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use pretty_assertions::assert_eq;
+
+    #[cfg(unix)]
+    #[test]
+    fn absolute_path_without_dots_is_unchanged() {
+        assert_eq!(
+            absolutize_from(Path::new("/path/to/123/456"), Path::new("/base")),
+            PathBuf::from("/path/to/123/456")
+        );
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn absolute_path_dots_are_removed() {
+        assert_eq!(
+            absolutize_from(Path::new("/path/to/./123/../456"), Path::new("/base")),
+            PathBuf::from("/path/to/456")
+        );
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn relative_path_without_dot_uses_base() {
+        assert_eq!(
+            absolutize_from(Path::new("path/to/123/456"), Path::new("/base")),
+            PathBuf::from("/base/path/to/123/456")
+        );
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn relative_path_with_current_dir_uses_base() {
+        assert_eq!(
+            absolutize_from(Path::new("./path/to/123/456"), Path::new("/base")),
+            PathBuf::from("/base/path/to/123/456")
+        );
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn relative_path_with_parent_dir_uses_base_parent() {
+        assert_eq!(
+            absolutize_from(Path::new("../path/to/123/456"), Path::new("/base/cwd")),
+            PathBuf::from("/base/path/to/123/456")
+        );
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn parent_dir_above_root_stays_at_root() {
+        assert_eq!(
+            absolutize_from(Path::new("../../path/to/123/456"), Path::new("/")),
+            PathBuf::from("/path/to/123/456")
+        );
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn empty_path_uses_base() {
+        assert_eq!(
+            absolutize_from(Path::new(""), Path::new("/base/cwd")),
+            PathBuf::from("/base/cwd")
+        );
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn windows_root_relative_path_uses_base_prefix() {
+        assert_eq!(
+            absolutize_from(Path::new(r"\path\to\file"), Path::new(r"C:\base\cwd")),
+            PathBuf::from(r"C:\path\to\file")
+        );
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn windows_drive_relative_path_uses_path_prefix_and_base_tail() {
+        assert_eq!(
+            absolutize_from(Path::new(r"D:path\to\file"), Path::new(r"C:\base\cwd")),
+            PathBuf::from(r"D:\base\cwd\path\to\file")
+        );
+    }
+}

codex-rs/utils/absolute-path/src/lib.rs

@@ -1,5 +1,4 @@
 use dirs::home_dir;
-use path_absolutize::Absolutize;
 use schemars::JsonSchema;
 use serde::Deserialize;
 use serde::Deserializer;
@@ -11,6 +10,8 @@ use std::path::Path;
 use std::path::PathBuf;
 use ts_rs::TS;
 
+mod absolutize;
+
 /// A path that is guaranteed to be absolute and normalized (though it is not
 /// guaranteed to be canonicalized or exist on the filesystem).
 ///
@@ -43,30 +44,34 @@ impl AbsolutePathBuf {
     pub fn resolve_path_against_base<P: AsRef<Path>, B: AsRef<Path>>(
         path: P,
         base_path: B,
-    ) -> std::io::Result<Self> {
+    ) -> Self {
         let expanded = Self::maybe_expand_home_directory(path.as_ref());
-        let absolute_path = expanded.absolutize_from(base_path.as_ref())?;
-        Ok(Self(absolute_path.into_owned()))
+        Self(absolutize::absolutize_from(&expanded, base_path.as_ref()))
     }
 
     pub fn from_absolute_path<P: AsRef<Path>>(path: P) -> std::io::Result<Self> {
         let expanded = Self::maybe_expand_home_directory(path.as_ref());
-        let absolute_path = expanded.absolutize()?;
-        Ok(Self(absolute_path.into_owned()))
+        Ok(Self(absolutize::absolutize(&expanded)?))
     }
 
     pub fn current_dir() -> std::io::Result<Self> {
         let current_dir = std::env::current_dir()?;
-        Self::from_absolute_path(current_dir)
+        Ok(Self(absolutize::absolutize_from(
+            &current_dir,
+            &current_dir,
+        )))
     }
 
     /// Construct an absolute path from `path`, resolving relative paths against
     /// the process current working directory.
     pub fn relative_to_current_dir<P: AsRef<Path>>(path: P) -> std::io::Result<Self> {
-        Self::resolve_path_against_base(path, std::env::current_dir()?)
+        Ok(Self::resolve_path_against_base(
+            path,
+            std::env::current_dir()?,
+        ))
     }
 
-    pub fn join<P: AsRef<Path>>(&self, path: P) -> std::io::Result<Self> {
+    pub fn join<P: AsRef<Path>>(&self, path: P) -> Self {
         Self::resolve_path_against_base(path, &self.0)
     }
 
@@ -187,9 +192,7 @@ impl<'de> Deserialize<'de> for AbsolutePathBuf {
     {
         let path = PathBuf::deserialize(deserializer)?;
         ABSOLUTE_PATH_BASE.with(|cell| match cell.borrow().as_deref() {
-            Some(base) => {
-                Ok(Self::resolve_path_against_base(path, base).map_err(SerdeError::custom)?)
-            }
+            Some(base) => Ok(Self::resolve_path_against_base(path, base)),
             None if path.is_absolute() => {
                 Self::from_absolute_path(path).map_err(SerdeError::custom)
             }
@@ -213,8 +216,7 @@ mod tests {
         let base_path = base_dir.path();
         let absolute_path = absolute_dir.path().join("file.txt");
         let abs_path_buf =
-            AbsolutePathBuf::resolve_path_against_base(absolute_path.clone(), base_path)
-                .expect("failed to create");
+            AbsolutePathBuf::resolve_path_against_base(absolute_path.clone(), base_path);
         assert_eq!(abs_path_buf.as_path(), absolute_path.as_path());
     }
 
@@ -222,8 +224,16 @@ mod tests {
     fn relative_path_is_resolved_against_base_path() {
         let temp_dir = tempdir().expect("base dir");
         let base_dir = temp_dir.path();
-        let abs_path_buf = AbsolutePathBuf::resolve_path_against_base("file.txt", base_dir)
-            .expect("failed to create");
+        let abs_path_buf = AbsolutePathBuf::resolve_path_against_base("file.txt", base_dir);
+        assert_eq!(abs_path_buf.as_path(), base_dir.join("file.txt").as_path());
+    }
+
+    #[test]
+    fn relative_path_dots_are_normalized_against_base_path() {
+        let temp_dir = tempdir().expect("base dir");
+        let base_dir = temp_dir.path();
+        let abs_path_buf =
+            AbsolutePathBuf::resolve_path_against_base("./nested/../file.txt", base_dir);
         assert_eq!(abs_path_buf.as_path(), base_dir.join("file.txt").as_path());
     }