diff --git a/crates/red_knot_server/src/system.rs b/crates/red_knot_server/src/system.rs index 1d834f2b0f27a..729dc02f05e00 100644 --- a/crates/red_knot_server/src/system.rs +++ b/crates/red_knot_server/src/system.rs @@ -144,19 +144,6 @@ impl System for LSPSystem { } } - fn virtual_path_metadata(&self, path: &SystemVirtualPath) -> Result { - // Virtual paths only exists in the LSP system, so we don't need to check the OS system. - let document = self - .system_virtual_path_to_document_ref(path)? - .ok_or_else(|| virtual_path_not_found(path))?; - - Ok(Metadata::new( - document_revision(&document), - None, - FileType::File, - )) - } - fn read_virtual_path_to_string(&self, path: &SystemVirtualPath) -> Result { let document = self .system_virtual_path_to_document_ref(path)? diff --git a/crates/red_knot_wasm/src/lib.rs b/crates/red_knot_wasm/src/lib.rs index cb2e531532253..0379ad3fdda56 100644 --- a/crates/red_knot_wasm/src/lib.rs +++ b/crates/red_knot_wasm/src/lib.rs @@ -234,13 +234,6 @@ impl System for WasmSystem { Notebook::from_source_code(&content) } - fn virtual_path_metadata( - &self, - _path: &SystemVirtualPath, - ) -> ruff_db::system::Result { - Err(not_found()) - } - fn read_virtual_path_to_string( &self, _path: &SystemVirtualPath, diff --git a/crates/ruff_db/src/files.rs b/crates/ruff_db/src/files.rs index 66d29c3bf8604..835ae3ff9c176 100644 --- a/crates/ruff_db/src/files.rs +++ b/crates/ruff_db/src/files.rs @@ -12,7 +12,7 @@ use ruff_notebook::{Notebook, NotebookError}; use crate::file_revision::FileRevision; use crate::files::file_root::FileRoots; use crate::files::private::FileStatus; -use crate::system::{Metadata, SystemPath, SystemPathBuf, SystemVirtualPath, SystemVirtualPathBuf}; +use crate::system::{SystemPath, SystemPathBuf, SystemVirtualPath, SystemVirtualPathBuf}; use crate::vendored::{VendoredPath, VendoredPathBuf}; use crate::{vendored, Db, FxDashMap}; @@ -60,8 +60,8 @@ struct FilesInner { /// so that queries that depend on the existence of a file are re-executed when the file is created. system_by_path: FxDashMap, - /// Lookup table that maps [`SystemVirtualPathBuf`]s to salsa interned [`File`] instances. - system_virtual_by_path: FxDashMap, + /// Lookup table that maps [`SystemVirtualPathBuf`]s to [`VirtualFile`] instances. + system_virtual_by_path: FxDashMap, /// Lookup table that maps vendored files to the salsa [`File`] ingredients. vendored_by_path: FxDashMap, @@ -147,31 +147,31 @@ impl Files { Ok(file) } - /// Looks up a virtual file by its `path`. + /// Create a new virtual file at the given path and store it for future lookups. /// - /// For a non-existing file, creates a new salsa [`File`] ingredient and stores it for future lookups. - /// - /// The operations fails if the system failed to provide a metadata for the path. - pub fn add_virtual_file(&self, db: &dyn Db, path: &SystemVirtualPath) -> Option { - let file = match self.inner.system_virtual_by_path.entry(path.to_path_buf()) { - Entry::Occupied(entry) => *entry.get(), - Entry::Vacant(entry) => { - let metadata = db.system().virtual_path_metadata(path).ok()?; - - tracing::trace!("Adding virtual file '{}'", path); - - let file = File::builder(FilePath::SystemVirtual(path.to_path_buf())) - .revision(metadata.revision()) - .permissions(metadata.permissions()) - .new(db); - - entry.insert(file); - - file - } - }; + /// This will always create a new file, overwriting any existing file at `path` in the internal + /// storage. + pub fn virtual_file(&self, db: &dyn Db, path: &SystemVirtualPath) -> VirtualFile { + tracing::trace!("Adding virtual file {}", path); + let virtual_file = VirtualFile( + File::builder(FilePath::SystemVirtual(path.to_path_buf())) + .status(FileStatus::Exists) + .revision(FileRevision::zero()) + .permissions(None) + .new(db), + ); + self.inner + .system_virtual_by_path + .insert(path.to_path_buf(), virtual_file); + virtual_file + } - Some(file) + /// Tries to look up a virtual file by its path. Returns `None` if no such file exists yet. + pub fn try_virtual_file(&self, path: &SystemVirtualPath) -> Option { + self.inner + .system_virtual_by_path + .get(&path.to_path_buf()) + .map(|entry| *entry.value()) } /// Looks up the closest root for `path`. Returns `None` if `path` isn't enclosed by any source root. @@ -354,6 +354,13 @@ impl File { Self::sync_system_path(db, &absolute, None); } + /// Increments the revision for the virtual file at `path`. + pub fn sync_virtual_path(db: &mut dyn Db, path: &SystemVirtualPath) { + if let Some(virtual_file) = db.files().try_virtual_file(path) { + virtual_file.sync(db); + } + } + /// Syncs the [`File`]'s state with the state of the file on the system. pub fn sync(self, db: &mut dyn Db) { let path = self.path(db).clone(); @@ -366,29 +373,20 @@ impl File { FilePath::Vendored(_) => { // Readonly, can never be out of date. } - FilePath::SystemVirtual(system_virtual) => { - Self::sync_system_virtual_path(db, &system_virtual, self); + FilePath::SystemVirtual(_) => { + VirtualFile(self).sync(db); } } } + /// Private method providing the implementation for [`Self::sync_path`] and [`Self::sync`] for + /// system paths. fn sync_system_path(db: &mut dyn Db, path: &SystemPath, file: Option) { let Some(file) = file.or_else(|| db.files().try_system(db, path)) else { return; }; - let metadata = db.system().path_metadata(path); - Self::sync_impl(db, metadata, file); - } - - fn sync_system_virtual_path(db: &mut dyn Db, path: &SystemVirtualPath, file: File) { - let metadata = db.system().virtual_path_metadata(path); - Self::sync_impl(db, metadata, file); - } - /// Private method providing the implementation for [`Self::sync_system_path`] and - /// [`Self::sync_system_virtual_path`]. - fn sync_impl(db: &mut dyn Db, metadata: crate::system::Result, file: File) { - let (status, revision, permission) = match metadata { + let (status, revision, permission) = match db.system().path_metadata(path) { Ok(metadata) if metadata.file_type().is_file() => ( FileStatus::Exists, metadata.revision(), @@ -422,6 +420,35 @@ impl File { } } +/// A virtual file that doesn't exist on the file system. +/// +/// This is a wrapper around a [`File`] that provides additional methods to interact with a virtual +/// file. +#[derive(Copy, Clone)] +pub struct VirtualFile(File); + +impl VirtualFile { + /// Returns the underlying [`File`]. + pub fn file(&self) -> File { + self.0 + } + + /// Increments the revision of the underlying [`File`]. + fn sync(&self, db: &mut dyn Db) { + let file = self.0; + tracing::debug!("Updating the revision of '{}'", file.path(db)); + let current_revision = file.revision(db); + file.set_revision(db) + .to(FileRevision::new(current_revision.as_u128() + 1)); + } + + /// Closes the virtual file. + pub fn close(&self, db: &mut dyn Db) { + tracing::debug!("Closing virtual file '{}'", self.0.path(db)); + self.0.set_status(db).to(FileStatus::NotFound); + } +} + // The types in here need to be public because they're salsa ingredients but we // don't want them to be publicly accessible. That's why we put them into a private module. mod private { diff --git a/crates/ruff_db/src/parsed.rs b/crates/ruff_db/src/parsed.rs index 610372b59c57f..c47e71fdfff86 100644 --- a/crates/ruff_db/src/parsed.rs +++ b/crates/ruff_db/src/parsed.rs @@ -121,9 +121,9 @@ mod tests { db.write_virtual_file(path, "x = 10"); - let file = db.files().add_virtual_file(&db, path).unwrap(); + let virtual_file = db.files().virtual_file(&db, path); - let parsed = parsed_module(&db, file); + let parsed = parsed_module(&db, virtual_file.file()); assert!(parsed.is_valid()); @@ -137,9 +137,9 @@ mod tests { db.write_virtual_file(path, "%timeit a = b"); - let file = db.files().add_virtual_file(&db, path).unwrap(); + let virtual_file = db.files().virtual_file(&db, path); - let parsed = parsed_module(&db, file); + let parsed = parsed_module(&db, virtual_file.file()); assert!(parsed.is_valid()); diff --git a/crates/ruff_db/src/system.rs b/crates/ruff_db/src/system.rs index ab0ab222bd395..35a916fc47d4c 100644 --- a/crates/ruff_db/src/system.rs +++ b/crates/ruff_db/src/system.rs @@ -63,9 +63,6 @@ pub trait System: Debug { /// representation fall-back to deserializing the notebook from a string. fn read_to_notebook(&self, path: &SystemPath) -> std::result::Result; - /// Reads the metadata of the virtual file at `path`. - fn virtual_path_metadata(&self, path: &SystemVirtualPath) -> Result; - /// Reads the content of the virtual file at `path` into a [`String`]. fn read_virtual_path_to_string(&self, path: &SystemVirtualPath) -> Result; diff --git a/crates/ruff_db/src/system/memory_fs.rs b/crates/ruff_db/src/system/memory_fs.rs index c53b5d9be1311..44e7fe8a5fa59 100644 --- a/crates/ruff_db/src/system/memory_fs.rs +++ b/crates/ruff_db/src/system/memory_fs.rs @@ -136,22 +136,6 @@ impl MemoryFileSystem { ruff_notebook::Notebook::from_source_code(&content) } - pub(crate) fn virtual_path_metadata( - &self, - path: impl AsRef, - ) -> Result { - let virtual_files = self.inner.virtual_files.read().unwrap(); - let file = virtual_files - .get(&path.as_ref().to_path_buf()) - .ok_or_else(not_found)?; - - Ok(Metadata { - revision: file.last_modified.into(), - permissions: Some(MemoryFileSystem::PERMISSION), - file_type: FileType::File, - }) - } - pub(crate) fn read_virtual_path_to_string( &self, path: impl AsRef, diff --git a/crates/ruff_db/src/system/os.rs b/crates/ruff_db/src/system/os.rs index a5362ec9fccce..d4ff8bd3926df 100644 --- a/crates/ruff_db/src/system/os.rs +++ b/crates/ruff_db/src/system/os.rs @@ -77,10 +77,6 @@ impl System for OsSystem { Notebook::from_path(path.as_std_path()) } - fn virtual_path_metadata(&self, _path: &SystemVirtualPath) -> Result { - Err(not_found()) - } - fn read_virtual_path_to_string(&self, _path: &SystemVirtualPath) -> Result { Err(not_found()) } diff --git a/crates/ruff_db/src/system/test.rs b/crates/ruff_db/src/system/test.rs index 1ef18d8bf3c43..2b8ecdfc29b64 100644 --- a/crates/ruff_db/src/system/test.rs +++ b/crates/ruff_db/src/system/test.rs @@ -69,13 +69,6 @@ impl System for TestSystem { } } - fn virtual_path_metadata(&self, path: &SystemVirtualPath) -> Result { - match &self.inner { - TestSystemInner::Stub(fs) => fs.virtual_path_metadata(path), - TestSystemInner::System(system) => system.virtual_path_metadata(path), - } - } - fn read_virtual_path_to_string(&self, path: &SystemVirtualPath) -> Result { match &self.inner { TestSystemInner::Stub(fs) => fs.read_virtual_path_to_string(path),