From 9069a58e82953bb87c92e0222332e69a987efbe0 Mon Sep 17 00:00:00 2001
From: Itay Tsabary <itayt@starkware.co>
Date: Sun, 24 Nov 2024 21:03:03 +0200
Subject: [PATCH] chore(infra): canonicalize path resolution

commit-id:c2547652
---
 Cargo.lock                          |  3 ---
 crates/infra_utils/Cargo.toml       |  3 ---
 crates/infra_utils/src/path.rs      | 27 ++++++---------------------
 crates/infra_utils/src/path_test.rs |  9 ++-------
 crates/papyrus_config/src/lib.rs    |  3 ---
 5 files changed, 8 insertions(+), 37 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index c312f12818..5e7278717e 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -5163,9 +5163,6 @@ checksum = "b248f5224d1d606005e02c97f5aa4e88eeb230488bcc03bc9ca4d7991399f2b5"
 [[package]]
 name = "infra_utils"
 version = "0.0.0"
-dependencies = [
- "thiserror",
-]
 
 [[package]]
 name = "inout"
diff --git a/crates/infra_utils/Cargo.toml b/crates/infra_utils/Cargo.toml
index ed18931577..69c33687b6 100644
--- a/crates/infra_utils/Cargo.toml
+++ b/crates/infra_utils/Cargo.toml
@@ -8,6 +8,3 @@ description = "Infrastructure utility."
 
 [lints]
 workspace = true
-
-[dependencies]
-thiserror.workspace = true
diff --git a/crates/infra_utils/src/path.rs b/crates/infra_utils/src/path.rs
index cfa2444a24..f3ceeb0802 100644
--- a/crates/infra_utils/src/path.rs
+++ b/crates/infra_utils/src/path.rs
@@ -1,28 +1,16 @@
-use std::env;
 use std::path::{Path, PathBuf};
 use std::sync::LazyLock;
-
-use thiserror::Error;
+use std::{env, fs};
 
 #[cfg(test)]
 #[path = "path_test.rs"]
 mod path_test;
 
-#[derive(Debug, Error)]
-pub enum PathResolutionError {
-    // TODO(Arni): Handle manifest dir not exist here?
-    #[error("No file exists at '{path}'")]
-    PathDoesNotExist { path: PathBuf },
-    /// This error is raised when file existence can be neither confirmed nor denied. See
-    /// [`std::path::Path::try_exists`] for more information.
-    #[error(transparent)]
-    IoError(#[from] std::io::Error),
-}
-
 // TODO(tsabary): wrap path-related env::* invocations in the repo as utility functions
 static PATH_TO_CARGO_MANIFEST_DIR: LazyLock<Option<PathBuf>> =
     LazyLock::new(|| env::var("CARGO_MANIFEST_DIR").ok().map(|dir| Path::new(&dir).into()));
 
+// TODO(Tsabary): should not be public. Use a getter instead.
 pub fn cargo_manifest_dir() -> Option<PathBuf> {
     PATH_TO_CARGO_MANIFEST_DIR.clone()
 }
@@ -34,16 +22,13 @@ pub fn cargo_manifest_dir() -> Option<PathBuf> {
 /// * `relative_path` - A string slice representing the relative path from the project root.
 ///
 /// # Returns
-/// * An absolute `PathBuf` representing the resolved path starting from the project root.
-pub fn resolve_project_relative_path(relative_path: &str) -> Result<PathBuf, PathResolutionError> {
+/// * A `PathBuf` representing the resolved path starting from the project root.
+pub fn resolve_project_relative_path(relative_path: &str) -> Result<PathBuf, std::io::Error> {
     let base_dir = path_of_project_root();
-
     let path = base_dir.join(relative_path);
-    if !path.try_exists()? {
-        return Err(PathResolutionError::PathDoesNotExist { path });
-    }
+    let absolute_path = fs::canonicalize(path)?;
 
-    Ok(path)
+    Ok(absolute_path)
 }
 
 fn path_of_project_root() -> PathBuf {
diff --git a/crates/infra_utils/src/path_test.rs b/crates/infra_utils/src/path_test.rs
index 88d7f5ff23..8133007262 100644
--- a/crates/infra_utils/src/path_test.rs
+++ b/crates/infra_utils/src/path_test.rs
@@ -1,6 +1,5 @@
-use crate::path::{path_of_project_root, resolve_project_relative_path, PathResolutionError};
+use crate::path::{path_of_project_root, resolve_project_relative_path};
 
-// TODO: Add a test for PathResolutionError::IoError.
 #[test]
 fn resolve_project_relative_path_on_non_existent_path() {
     let relative_path = "does_not_exist.txt";
@@ -8,11 +7,7 @@ fn resolve_project_relative_path_on_non_existent_path() {
     assert!(!expected_path.exists());
     let result = resolve_project_relative_path(relative_path);
 
-    if let Err(PathResolutionError::PathDoesNotExist { path }) = result {
-        assert_eq!(path, expected_path);
-    } else {
-        panic!("Expected PathDoesNotExist error, got {:?}", result);
-    }
+    assert!(result.is_err(), "Expected an non-existent path error, got {:?}", result);
 }
 
 #[test]
diff --git a/crates/papyrus_config/src/lib.rs b/crates/papyrus_config/src/lib.rs
index 77e4106f16..7a383cef54 100644
--- a/crates/papyrus_config/src/lib.rs
+++ b/crates/papyrus_config/src/lib.rs
@@ -49,7 +49,6 @@
 
 use clap::parser::MatchesError;
 use dumping::REQUIRED_PARAM_DESCRIPTION_PREFIX;
-use infra_utils::path::PathResolutionError;
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
 use validator::ValidationError;
@@ -180,8 +179,6 @@ pub enum ConfigError {
     #[error(transparent)]
     CommandMatches(#[from] MatchesError),
     #[error(transparent)]
-    GetPathError(#[from] PathResolutionError),
-    #[error(transparent)]
     IOError(#[from] std::io::Error),
     // TODO(Eitan): Improve error message
     #[error("Insert a new param is not allowed: {param_path}.")]