Only store absolute paths in Files

crates/red_knot/src/main.rs

@@ -35,7 +35,9 @@ pub fn main() -> anyhow::Result<()> {
         return Err(anyhow::anyhow!("Invalid arguments"));
     }
 
-    let system = OsSystem;
+    let cwd = std::env::current_dir().unwrap();
+    let cwd = SystemPath::from_std_path(&cwd).unwrap();
+    let system = OsSystem::new(cwd);
     let entry_point = SystemPath::new(&arguments[1]);
 
     if !system.path_exists(entry_point) {

crates/red_knot_module_resolver/src/db.rs

@@ -189,9 +189,9 @@ pub(crate) mod tests {
 
         let db = TestDb::new();
 
-        let src = SystemPathBuf::from("src");
-        let site_packages = SystemPathBuf::from("site_packages");
-        let custom_typeshed = SystemPathBuf::from("typeshed");
+        let src = SystemPathBuf::from("/src");
+        let site_packages = SystemPathBuf::from("/site_packages");
+        let custom_typeshed = SystemPathBuf::from("/typeshed");
 
         let fs = db.memory_file_system();
 

crates/red_knot_module_resolver/src/resolver.rs

@@ -387,7 +387,7 @@ impl PackageKind {
 #[cfg(test)]
 mod tests {
     use ruff_db::files::{system_path_to_file, File, FilePath};
-    use ruff_db::system::{DbWithTestSystem, SystemPath};
+    use ruff_db::system::DbWithTestSystem;
 
     use crate::db::tests::{create_resolver_builder, TestCase};
     use crate::module::ModuleKind;
@@ -826,21 +826,26 @@ mod tests {
     #[test]
     #[cfg(target_family = "unix")]
     fn symlink() -> anyhow::Result<()> {
+        use ruff_db::system::{OsSystem, SystemPath};
+
+        fn make_relative(path: &SystemPath) -> &SystemPath {
+            path.strip_prefix("/").unwrap_or(path)
+        }
+
         let TestCase {
             mut db,
             src,
             site_packages,
             custom_typeshed,
         } = setup_resolver_test();
 
-        db.use_os_system();
-
         let temp_dir = tempfile::tempdir()?;
         let root = SystemPath::from_std_path(temp_dir.path()).unwrap();
+        db.use_os_system(OsSystem::new(root));
 
-        let src = root.join(src);
-        let site_packages = root.join(site_packages);
-        let custom_typeshed = root.join(custom_typeshed);
+        let src = root.join(make_relative(&src));
+        let site_packages = root.join(make_relative(&site_packages));
+        let custom_typeshed = root.join(make_relative(&custom_typeshed));
 
         let foo = src.join("foo.py");
         let bar = src.join("bar.py");

crates/ruff_db/src/files.rs

@@ -60,25 +60,28 @@ impl Files {
     /// In these cases, a file with status [`FileStatus::Deleted`] is returned.
     #[tracing::instrument(level = "debug", skip(self, db))]
     fn system(&self, db: &dyn Db, path: &SystemPath) -> File {
+        let absolute = SystemPath::absolute(path, db.system().current_directory());
+        let absolute = FilePath::System(absolute);
+
         *self
             .inner
             .files_by_path
-            .entry(FilePath::System(path.to_path_buf()))
+            .entry(absolute.clone())
             .or_insert_with(|| {
                 let metadata = db.system().path_metadata(path);
 
                 match metadata {
                     Ok(metadata) if metadata.file_type().is_file() => File::new(
                         db,
-                        FilePath::System(path.to_path_buf()),
+                        absolute,
                         metadata.permissions(),
                         metadata.revision(),
                         FileStatus::Exists,
                         Count::default(),
                     ),
                     _ => File::new(
                         db,
-                        FilePath::System(path.to_path_buf()),
+                        absolute,
                         None,
                         FileRevision::zero(),
                         FileStatus::Deleted,
@@ -89,10 +92,11 @@ impl Files {
     }
 
     /// Tries to look up the file for the given system path, returns `None` if no such file exists yet
-    fn try_system(&self, path: &SystemPath) -> Option<File> {
+    fn try_system(&self, db: &dyn Db, path: &SystemPath) -> Option<File> {
+        let absolute = SystemPath::absolute(path, db.system().current_directory());
         self.inner
             .files_by_path
-            .get(&FilePath::System(path.to_path_buf()))
+            .get(&FilePath::System(absolute))
             .map(|entry| *entry.value())
     }
 
@@ -224,7 +228,7 @@ impl File {
                     _ => (FileStatus::Deleted, FileRevision::zero()),
                 };
 
-                let Some(file) = file.or_else(|| db.files().try_system(path)) else {
+                let Some(file) = file.or_else(|| db.files().try_system(db, path)) else {
                     return;
                 };
 
@@ -260,7 +264,7 @@ mod tests {
     use crate::vendored::tests::VendoredFileSystemBuilder;
 
     #[test]
-    fn file_system_existing_file() -> crate::system::Result<()> {
+    fn system_existing_file() -> crate::system::Result<()> {
         let mut db = TestDb::new();
 
         db.write_file("test.py", "print('Hello world')")?;
@@ -275,14 +279,29 @@ mod tests {
     }
 
     #[test]
-    fn file_system_non_existing_file() {
+    fn system_non_existing_file() {
         let db = TestDb::new();
 
         let test = system_path_to_file(&db, "test.py");
 
         assert_eq!(test, None);
     }
 
+    #[test]
+    fn system_normalize_paths() {
+        let db = TestDb::new();
+
+        assert_eq!(
+            system_path_to_file(&db, "test.py"),
+            system_path_to_file(&db, "/test.py")
+        );
+
+        assert_eq!(
+            system_path_to_file(&db, "/root/.././test.py"),
+            system_path_to_file(&db, "/root/test.py")
+        );
+    }
+
     #[test]
     fn stubbed_vendored_file() {
         let mut db = TestDb::new();

crates/ruff_db/src/system.rs

@@ -51,6 +51,9 @@ pub trait System {
             .map_or(false, |metadata| metadata.file_type.is_file())
     }
 
+    /// Returns the current working directory
+    fn current_directory(&self) -> &SystemPath;
+
     fn as_any(&self) -> &dyn std::any::Any;
 }
 

crates/ruff_db/src/system/memory_fs.rs

@@ -4,7 +4,7 @@ use std::sync::{Arc, RwLock, RwLockWriteGuard};
 use camino::{Utf8Path, Utf8PathBuf};
 use filetime::FileTime;
 
-use crate::system::{FileType, Metadata, Result, SystemPath};
+use crate::system::{FileType, Metadata, Result, SystemPath, SystemPathBuf};
 
 /// File system that stores all content in memory.
 ///
@@ -32,7 +32,7 @@ impl MemoryFileSystem {
 
     /// Creates a new, empty in memory file system with the given current working directory.
     pub fn with_cwd(cwd: impl AsRef<SystemPath>) -> Self {
-        let cwd = Utf8PathBuf::from(cwd.as_ref().as_str());
+        let cwd = cwd.as_ref().to_path_buf();
 
         assert!(
             cwd.starts_with("/"),
@@ -46,11 +46,15 @@ impl MemoryFileSystem {
             }),
         };
 
-        fs.create_directory_all(SystemPath::new(&cwd)).unwrap();
+        fs.create_directory_all(&cwd).unwrap();
 
         fs
     }
 
+    pub fn cwd(&self) -> &SystemPath {
+        &self.inner.cwd
+    }
+
     #[must_use]
     pub fn snapshot(&self) -> Self {
         Self {
@@ -59,9 +63,9 @@ impl MemoryFileSystem {
     }
 
     pub fn metadata(&self, path: impl AsRef<SystemPath>) -> Result<Metadata> {
-        fn metadata(fs: &MemoryFileSystemInner, path: &SystemPath) -> Result<Metadata> {
-            let by_path = fs.by_path.read().unwrap();
-            let normalized = normalize_path(path, &fs.cwd);
+        fn metadata(fs: &MemoryFileSystem, path: &SystemPath) -> Result<Metadata> {
+            let by_path = fs.inner.by_path.read().unwrap();
+            let normalized = fs.normalize_path(path);
 
             let entry = by_path.get(&normalized).ok_or_else(not_found)?;
 
@@ -81,27 +85,27 @@ impl MemoryFileSystem {
             Ok(metadata)
         }
 
-        metadata(&self.inner, path.as_ref())
+        metadata(self, path.as_ref())
     }
 
     pub fn is_file(&self, path: impl AsRef<SystemPath>) -> bool {
         let by_path = self.inner.by_path.read().unwrap();
-        let normalized = normalize_path(path.as_ref(), &self.inner.cwd);
+        let normalized = self.normalize_path(path.as_ref());
 
         matches!(by_path.get(&normalized), Some(Entry::File(_)))
     }
 
     pub fn is_directory(&self, path: impl AsRef<SystemPath>) -> bool {
         let by_path = self.inner.by_path.read().unwrap();
-        let normalized = normalize_path(path.as_ref(), &self.inner.cwd);
+        let normalized = self.normalize_path(path.as_ref());
 
         matches!(by_path.get(&normalized), Some(Entry::Directory(_)))
     }
 
     pub fn read_to_string(&self, path: impl AsRef<SystemPath>) -> Result<String> {
-        fn read_to_string(fs: &MemoryFileSystemInner, path: &SystemPath) -> Result<String> {
-            let by_path = fs.by_path.read().unwrap();
-            let normalized = normalize_path(path, &fs.cwd);
+        fn read_to_string(fs: &MemoryFileSystem, path: &SystemPath) -> Result<String> {
+            let by_path = fs.inner.by_path.read().unwrap();
+            let normalized = fs.normalize_path(path);
 
             let entry = by_path.get(&normalized).ok_or_else(not_found)?;
 
@@ -111,12 +115,12 @@ impl MemoryFileSystem {
             }
         }
 
-        read_to_string(&self.inner, path.as_ref())
+        read_to_string(self, path.as_ref())
     }
 
     pub fn exists(&self, path: &SystemPath) -> bool {
         let by_path = self.inner.by_path.read().unwrap();
-        let normalized = normalize_path(path, &self.inner.cwd);
+        let normalized = self.normalize_path(path);
 
         by_path.contains_key(&normalized)
     }
@@ -146,7 +150,7 @@ impl MemoryFileSystem {
     pub fn write_file(&self, path: impl AsRef<SystemPath>, content: impl ToString) -> Result<()> {
         let mut by_path = self.inner.by_path.write().unwrap();
 
-        let normalized = normalize_path(path.as_ref(), &self.inner.cwd);
+        let normalized = self.normalize_path(path.as_ref());
 
         get_or_create_file(&mut by_path, &normalized)?.content = content.to_string();
 
@@ -156,7 +160,7 @@ impl MemoryFileSystem {
     pub fn remove_file(&self, path: impl AsRef<SystemPath>) -> Result<()> {
         fn remove_file(fs: &MemoryFileSystem, path: &SystemPath) -> Result<()> {
             let mut by_path = fs.inner.by_path.write().unwrap();
-            let normalized = normalize_path(path, &fs.inner.cwd);
+            let normalized = fs.normalize_path(path);
 
             match by_path.entry(normalized) {
                 std::collections::btree_map::Entry::Occupied(entry) => match entry.get() {
@@ -178,7 +182,7 @@ impl MemoryFileSystem {
     /// Creates a new file if the file at `path` doesn't exist.
     pub fn touch(&self, path: impl AsRef<SystemPath>) -> Result<()> {
         let mut by_path = self.inner.by_path.write().unwrap();
-        let normalized = normalize_path(path.as_ref(), &self.inner.cwd);
+        let normalized = self.normalize_path(path.as_ref());
 
         get_or_create_file(&mut by_path, &normalized)?.last_modified = FileTime::now();
 
@@ -188,7 +192,7 @@ impl MemoryFileSystem {
     /// Creates a directory at `path`. All enclosing directories are created if they don't exist.
     pub fn create_directory_all(&self, path: impl AsRef<SystemPath>) -> Result<()> {
         let mut by_path = self.inner.by_path.write().unwrap();
-        let normalized = normalize_path(path.as_ref(), &self.inner.cwd);
+        let normalized = self.normalize_path(path.as_ref());
 
         create_dir_all(&mut by_path, &normalized)
     }
@@ -202,7 +206,7 @@ impl MemoryFileSystem {
     pub fn remove_directory(&self, path: impl AsRef<SystemPath>) -> Result<()> {
         fn remove_directory(fs: &MemoryFileSystem, path: &SystemPath) -> Result<()> {
             let mut by_path = fs.inner.by_path.write().unwrap();
-            let normalized = normalize_path(path, &fs.inner.cwd);
+            let normalized = fs.normalize_path(path);
 
             // Test if the directory is empty
             // Skip the directory path itself
@@ -228,6 +232,11 @@ impl MemoryFileSystem {
 
         remove_directory(self, path.as_ref())
     }
+
+    fn normalize_path(&self, path: impl AsRef<SystemPath>) -> Utf8PathBuf {
+        let normalized = SystemPath::absolute(path, &self.inner.cwd);
+        normalized.into_utf8_path_buf()
+    }
 }
 
 impl Default for MemoryFileSystem {
@@ -246,7 +255,7 @@ impl std::fmt::Debug for MemoryFileSystem {
 
 struct MemoryFileSystemInner {
     by_path: RwLock<BTreeMap<Utf8PathBuf, Entry>>,
-    cwd: Utf8PathBuf,
+    cwd: SystemPathBuf,
 }
 
 #[derive(Debug)]
@@ -292,42 +301,6 @@ fn directory_not_empty() -> std::io::Error {
     std::io::Error::new(std::io::ErrorKind::Other, "directory not empty")
 }
 
-/// Normalizes the path by removing `.` and `..` components and transform the path into an absolute path.
-///
-/// Adapted from https://github.com/rust-lang/cargo/blob/fede83ccf973457de319ba6fa0e36ead454d2e20/src/cargo/util/paths.rs#L61
-fn normalize_path(path: &SystemPath, cwd: &Utf8Path) -> Utf8PathBuf {
-    let path = camino::Utf8Path::new(path.as_str());
-
-    let mut components = path.components().peekable();
-    let mut ret =
-        if let Some(c @ (camino::Utf8Component::Prefix(..) | camino::Utf8Component::RootDir)) =
-            components.peek().cloned()
-        {
-            components.next();
-            Utf8PathBuf::from(c.as_str())
-        } else {
-            cwd.to_path_buf()
-        };
-
-    for component in components {
-        match component {
-            camino::Utf8Component::Prefix(..) => unreachable!(),
-            camino::Utf8Component::RootDir => {
-                ret.push(component);
-            }
-            camino::Utf8Component::CurDir => {}
-            camino::Utf8Component::ParentDir => {
-                ret.pop();
-            }
-            camino::Utf8Component::Normal(c) => {
-                ret.push(c);
-            }
-        }
-    }
-
-    ret
-}
-
 fn create_dir_all(
     paths: &mut RwLockWriteGuard<BTreeMap<Utf8PathBuf, Entry>>,
     normalized: &Utf8Path,