diff --git a/Cargo.lock b/Cargo.lock index af6cb8c8da..b4fe23b69f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2724,7 +2724,7 @@ checksum = "e86697c916019a8588c99b5fac3cead74ec0b4b819707a682fd4d23fa0ce1ba1" [[package]] name = "salsa" version = "0.18.0" -source = "git+https://github.com/salsa-rs/salsa.git?rev=cd339fc1c9a6ea0ffb1d09bd3bffb5633f776ef3#cd339fc1c9a6ea0ffb1d09bd3bffb5633f776ef3" +source = "git+https://github.com/MichaReiser/salsa.git?rev=0cae5c52a3240172ef0be5c9d19e63448c53397c#0cae5c52a3240172ef0be5c9d19e63448c53397c" dependencies = [ "arc-swap", "boomphf", @@ -2744,12 +2744,12 @@ dependencies = [ [[package]] name = "salsa-macro-rules" version = "0.1.0" -source = "git+https://github.com/salsa-rs/salsa.git?rev=cd339fc1c9a6ea0ffb1d09bd3bffb5633f776ef3#cd339fc1c9a6ea0ffb1d09bd3bffb5633f776ef3" +source = "git+https://github.com/MichaReiser/salsa.git?rev=0cae5c52a3240172ef0be5c9d19e63448c53397c#0cae5c52a3240172ef0be5c9d19e63448c53397c" [[package]] name = "salsa-macros" version = "0.18.0" -source = "git+https://github.com/salsa-rs/salsa.git?rev=cd339fc1c9a6ea0ffb1d09bd3bffb5633f776ef3#cd339fc1c9a6ea0ffb1d09bd3bffb5633f776ef3" +source = "git+https://github.com/MichaReiser/salsa.git?rev=0cae5c52a3240172ef0be5c9d19e63448c53397c#0cae5c52a3240172ef0be5c9d19e63448c53397c" dependencies = [ "heck", "proc-macro2", diff --git a/Cargo.toml b/Cargo.toml index 05646a919d..77e326a4c6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -107,7 +107,7 @@ rand = { version = "0.8.5" } rayon = { version = "1.10.0" } regex = { version = "1.10.2" } rustc-hash = { version = "2.0.0" } -salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "cd339fc1c9a6ea0ffb1d09bd3bffb5633f776ef3" } +salsa = { git = "https://github.com/MichaReiser/salsa.git", rev = "0cae5c52a3240172ef0be5c9d19e63448c53397c" } schemars = { version = "0.8.16" } seahash = { version = "4.1.0" } serde = { version = "1.0.197", features = ["derive"] } diff --git a/crates/red_knot/src/db/changes.rs b/crates/red_knot/src/db/changes.rs index df527f68fd..d97cc4f034 100644 --- a/crates/red_knot/src/db/changes.rs +++ b/crates/red_knot/src/db/changes.rs @@ -173,7 +173,7 @@ impl RootDatabase { let package = workspace.package(self, &path); let file = system_path_to_file(self, &path); - if let (Some(package), Some(file)) = (package, file) { + if let (Some(package), Ok(file)) = (package, file) { package.add_file(self, file); } } diff --git a/crates/red_knot/src/workspace.rs b/crates/red_knot/src/workspace.rs index 5ab8e89f80..d262ef39b0 100644 --- a/crates/red_knot/src/workspace.rs +++ b/crates/red_knot/src/workspace.rs @@ -1,4 +1,4 @@ -use salsa::Setter as _; +use salsa::{Durability, Setter as _}; use std::{collections::BTreeMap, sync::Arc}; use rustc_hash::{FxBuildHasher, FxHashSet}; @@ -105,7 +105,9 @@ impl Workspace { packages.insert(package.root.clone(), Package::from_metadata(db, package)); } - Workspace::new(db, metadata.root, None, packages) + Workspace::builder(metadata.root, None, packages) + .durability(Durability::MEDIUM) + .new(db) } pub fn root(self, db: &dyn Db) -> &SystemPath { @@ -136,7 +138,9 @@ impl Workspace { new_packages.insert(path, package); } - self.set_package_tree(db).to(new_packages); + self.set_package_tree(db) + .with_durability(Durability::MEDIUM) + .to(new_packages); } #[tracing::instrument(level = "debug", skip_all)] @@ -305,20 +309,28 @@ impl Package { } fn from_metadata(db: &dyn Db, metadata: PackageMetadata) -> Self { - Self::new(db, metadata.name, metadata.root, PackageFiles::default()) + Self::builder(metadata.name, metadata.root, PackageFiles::default()) + .durability(Durability::MEDIUM) + .new(db) } fn update(self, db: &mut dyn Db, metadata: PackageMetadata) { let root = self.root(db); assert_eq!(root, metadata.root()); - self.set_name(db).to(metadata.name); + if self.name(db) != metadata.name() { + self.set_name(db) + .with_durability(Durability::MEDIUM) + .to(metadata.name); + } } #[tracing::instrument(level = "debug", skip(db))] pub fn reload_files(self, db: &mut dyn Db) { - // Force a re-index of the files in the next revision. - self.set_file_set(db).to(PackageFiles::lazy()); + if !self.file_set(db).is_lazy() { + // Force a re-index of the files in the next revision. + self.set_file_set(db).to(PackageFiles::lazy()); + } } } @@ -364,7 +376,7 @@ fn discover_package_files(db: &dyn Db, path: &SystemPath) -> FxHashSet { for path in paths { // If this returns `None`, then the file was deleted between the `walk_directory` call and now. // We can ignore this. - if let Some(file) = system_path_to_file(db.upcast(), &path) { + if let Ok(file) = system_path_to_file(db.upcast(), &path) { files.insert(file); } } diff --git a/crates/red_knot/src/workspace/files.rs b/crates/red_knot/src/workspace/files.rs index ab5c0fb868..ae391fdcd2 100644 --- a/crates/red_knot/src/workspace/files.rs +++ b/crates/red_knot/src/workspace/files.rs @@ -47,6 +47,10 @@ impl PackageFiles { } } + pub fn is_lazy(&self) -> bool { + matches!(*self.state.lock().unwrap(), State::Lazy) + } + /// Returns a mutable view on the index that allows cheap in-place mutations. /// /// The changes are automatically written back to the database once the view is dropped. diff --git a/crates/red_knot/tests/file_watching.rs b/crates/red_knot/tests/file_watching.rs index 915a4182ad..9af46cf73c 100644 --- a/crates/red_knot/tests/file_watching.rs +++ b/crates/red_knot/tests/file_watching.rs @@ -10,7 +10,7 @@ use red_knot::watch; use red_knot::watch::{directory_watcher, WorkspaceWatcher}; use red_knot::workspace::WorkspaceMetadata; use red_knot_module_resolver::{resolve_module, ModuleName}; -use ruff_db::files::{system_path_to_file, File}; +use ruff_db::files::{system_path_to_file, File, FileError}; use ruff_db::program::{Program, ProgramSettings, SearchPathSettings, TargetVersion}; use ruff_db::source::source_text; use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf}; @@ -82,7 +82,7 @@ impl TestCase { collected } - fn system_file(&self, path: impl AsRef) -> Option { + fn system_file(&self, path: impl AsRef) -> Result { system_path_to_file(self.db(), path.as_ref()) } } @@ -190,7 +190,7 @@ fn new_file() -> anyhow::Result<()> { let bar_file = case.system_file(&bar_path).unwrap(); let foo_path = case.workspace_path("foo.py"); - assert_eq!(case.system_file(&foo_path), None); + assert_eq!(case.system_file(&foo_path), Err(FileError::NotFound)); assert_eq!(&case.collect_package_files(&bar_path), &[bar_file]); std::fs::write(foo_path.as_std_path(), "print('Hello')")?; @@ -213,7 +213,7 @@ fn new_ignored_file() -> anyhow::Result<()> { let bar_file = case.system_file(&bar_path).unwrap(); let foo_path = case.workspace_path("foo.py"); - assert_eq!(case.system_file(&foo_path), None); + assert_eq!(case.system_file(&foo_path), Err(FileError::NotFound)); assert_eq!(&case.collect_package_files(&bar_path), &[bar_file]); std::fs::write(foo_path.as_std_path(), "print('Hello')")?; @@ -222,7 +222,7 @@ fn new_ignored_file() -> anyhow::Result<()> { case.db_mut().apply_changes(changes); - assert!(case.system_file(&foo_path).is_some()); + assert!(case.system_file(&foo_path).is_ok()); assert_eq!(&case.collect_package_files(&bar_path), &[bar_file]); Ok(()) @@ -234,9 +234,7 @@ fn changed_file() -> anyhow::Result<()> { let mut case = setup([("foo.py", foo_source)])?; let foo_path = case.workspace_path("foo.py"); - let foo = case - .system_file(&foo_path) - .ok_or_else(|| anyhow!("Foo not found"))?; + let foo = case.system_file(&foo_path)?; assert_eq!(source_text(case.db(), foo).as_str(), foo_source); assert_eq!(&case.collect_package_files(&foo_path), &[foo]); @@ -260,9 +258,7 @@ fn changed_metadata() -> anyhow::Result<()> { let mut case = setup([("foo.py", "")])?; let foo_path = case.workspace_path("foo.py"); - let foo = case - .system_file(&foo_path) - .ok_or_else(|| anyhow!("Foo not found"))?; + let foo = case.system_file(&foo_path)?; assert_eq!( foo.permissions(case.db()), Some( @@ -302,9 +298,7 @@ fn deleted_file() -> anyhow::Result<()> { let mut case = setup([("foo.py", foo_source)])?; let foo_path = case.workspace_path("foo.py"); - let foo = case - .system_file(&foo_path) - .ok_or_else(|| anyhow!("Foo not found"))?; + let foo = case.system_file(&foo_path)?; assert!(foo.exists(case.db())); assert_eq!(&case.collect_package_files(&foo_path), &[foo]); @@ -333,9 +327,7 @@ fn move_file_to_trash() -> anyhow::Result<()> { let trash_path = case.root_path().join(".trash"); std::fs::create_dir_all(trash_path.as_std_path())?; - let foo = case - .system_file(&foo_path) - .ok_or_else(|| anyhow!("Foo not found"))?; + let foo = case.system_file(&foo_path)?; assert!(foo.exists(case.db())); assert_eq!(&case.collect_package_files(&foo_path), &[foo]); @@ -367,7 +359,7 @@ fn move_file_to_workspace() -> anyhow::Result<()> { let foo_in_workspace_path = case.workspace_path("foo.py"); - assert!(case.system_file(&foo_path).is_some()); + assert!(case.system_file(&foo_path).is_ok()); assert_eq!(&case.collect_package_files(&bar_path), &[bar]); assert!(case .db() @@ -381,9 +373,7 @@ fn move_file_to_workspace() -> anyhow::Result<()> { case.db_mut().apply_changes(changes); - let foo_in_workspace = case - .system_file(&foo_in_workspace_path) - .ok_or_else(|| anyhow!("Foo not found"))?; + let foo_in_workspace = case.system_file(&foo_in_workspace_path)?; assert!(foo_in_workspace.exists(case.db())); assert_eq!( @@ -401,9 +391,7 @@ fn rename_file() -> anyhow::Result<()> { let foo_path = case.workspace_path("foo.py"); let bar_path = case.workspace_path("bar.py"); - let foo = case - .system_file(&foo_path) - .ok_or_else(|| anyhow!("Foo not found"))?; + let foo = case.system_file(&foo_path)?; assert_eq!(case.collect_package_files(&foo_path), [foo]); @@ -415,9 +403,7 @@ fn rename_file() -> anyhow::Result<()> { assert!(!foo.exists(case.db())); - let bar = case - .system_file(&bar_path) - .ok_or_else(|| anyhow!("Bar not found"))?; + let bar = case.system_file(&bar_path)?; assert!(bar.exists(case.db())); assert_eq!(case.collect_package_files(&foo_path), [bar]); diff --git a/crates/red_knot_module_resolver/src/path.rs b/crates/red_knot_module_resolver/src/path.rs index e9fdcd493d..eea4948b48 100644 --- a/crates/red_knot_module_resolver/src/path.rs +++ b/crates/red_knot_module_resolver/src/path.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use camino::{Utf8Path, Utf8PathBuf}; -use ruff_db::files::{system_path_to_file, vendored_path_to_file, File}; +use ruff_db::files::{system_path_to_file, vendored_path_to_file, File, FileError}; use ruff_db::system::{System, SystemPath, SystemPathBuf}; use ruff_db::vendored::{VendoredPath, VendoredPathBuf}; @@ -68,16 +68,18 @@ impl ModulePath { SearchPathInner::Extra(search_path) | SearchPathInner::FirstParty(search_path) | SearchPathInner::SitePackages(search_path) - | SearchPathInner::Editable(search_path) => resolver - .system() - .is_directory(&search_path.join(relative_path)), + | SearchPathInner::Editable(search_path) => { + system_path_to_file(resolver.db.upcast(), search_path.join(relative_path)) + == Err(FileError::IsADirectory) + } SearchPathInner::StandardLibraryCustom(stdlib_root) => { match query_stdlib_version(Some(stdlib_root), relative_path, resolver) { TypeshedVersionsQueryResult::DoesNotExist => false, TypeshedVersionsQueryResult::Exists - | TypeshedVersionsQueryResult::MaybeExists => resolver - .system() - .is_directory(&stdlib_root.join(relative_path)), + | TypeshedVersionsQueryResult::MaybeExists => { + system_path_to_file(resolver.db.upcast(), stdlib_root.join(relative_path)) + == Err(FileError::IsADirectory) + } } } SearchPathInner::StandardLibraryVendored(stdlib_root) => { @@ -105,10 +107,9 @@ impl ModulePath { | SearchPathInner::SitePackages(search_path) | SearchPathInner::Editable(search_path) => { let absolute_path = search_path.join(relative_path); - system_path_to_file(resolver.db.upcast(), absolute_path.join("__init__.py")) - .is_some() + system_path_to_file(resolver.db.upcast(), absolute_path.join("__init__.py")).is_ok() || system_path_to_file(resolver.db.upcast(), absolute_path.join("__init__.py")) - .is_some() + .is_ok() } SearchPathInner::StandardLibraryCustom(search_path) => { match query_stdlib_version(Some(search_path), relative_path, resolver) { @@ -118,7 +119,7 @@ impl ModulePath { resolver.db.upcast(), search_path.join(relative_path).join("__init__.pyi"), ) - .is_some(), + .is_ok(), } } SearchPathInner::StandardLibraryVendored(search_path) => { @@ -145,14 +146,14 @@ impl ModulePath { | SearchPathInner::FirstParty(search_path) | SearchPathInner::SitePackages(search_path) | SearchPathInner::Editable(search_path) => { - system_path_to_file(db, search_path.join(relative_path)) + system_path_to_file(db, search_path.join(relative_path)).ok() } SearchPathInner::StandardLibraryCustom(stdlib_root) => { match query_stdlib_version(Some(stdlib_root), relative_path, resolver) { TypeshedVersionsQueryResult::DoesNotExist => None, TypeshedVersionsQueryResult::Exists | TypeshedVersionsQueryResult::MaybeExists => { - system_path_to_file(db, stdlib_root.join(relative_path)) + system_path_to_file(db, stdlib_root.join(relative_path)).ok() } } } @@ -161,7 +162,7 @@ impl ModulePath { TypeshedVersionsQueryResult::DoesNotExist => None, TypeshedVersionsQueryResult::Exists | TypeshedVersionsQueryResult::MaybeExists => { - vendored_path_to_file(db, stdlib_root.join(relative_path)) + vendored_path_to_file(db, stdlib_root.join(relative_path)).ok() } } } @@ -301,11 +302,15 @@ pub(crate) enum SearchPathValidationError { /// (This is only relevant for stdlib search paths.) NoStdlibSubdirectory(SystemPathBuf), - /// The path provided by the user is a directory, + /// The typeshed path provided by the user is a directory, /// but no `stdlib/VERSIONS` file exists. /// (This is only relevant for stdlib search paths.) NoVersionsFile(SystemPathBuf), + /// `stdlib/VERSIONS` is a directory. + /// (This is only relevant for stdlib search paths.) + VersionsIsADirectory(SystemPathBuf), + /// The path provided by the user is a directory, /// and a `stdlib/VERSIONS` file exists, but it fails to parse. /// (This is only relevant for stdlib search paths.) @@ -320,6 +325,7 @@ impl fmt::Display for SearchPathValidationError { write!(f, "The directory at {path} has no `stdlib/` subdirectory") } Self::NoVersionsFile(path) => write!(f, "Expected a file at {path}/stldib/VERSIONS"), + Self::VersionsIsADirectory(path) => write!(f, "{path}/stldib/VERSIONS is a directory."), Self::VersionsParseError(underlying_error) => underlying_error.fmt(f), } } @@ -408,10 +414,13 @@ impl SearchPath { typeshed.to_path_buf(), )); } - let Some(typeshed_versions) = system_path_to_file(db.upcast(), stdlib.join("VERSIONS")) - else { - return Err(SearchPathValidationError::NoVersionsFile(typeshed)); - }; + let typeshed_versions = + system_path_to_file(db.upcast(), stdlib.join("VERSIONS")).map_err(|err| match err { + FileError::NotFound => SearchPathValidationError::NoVersionsFile(typeshed), + FileError::IsADirectory => { + SearchPathValidationError::VersionsIsADirectory(typeshed) + } + })?; crate::typeshed::parse_typeshed_versions(db, typeshed_versions) .as_ref() .map_err(|validation_error| { diff --git a/crates/red_knot_module_resolver/src/resolver.rs b/crates/red_knot_module_resolver/src/resolver.rs index ff8165b22c..8587cbe819 100644 --- a/crates/red_knot_module_resolver/src/resolver.rs +++ b/crates/red_knot_module_resolver/src/resolver.rs @@ -489,6 +489,7 @@ fn resolve_name(db: &dyn Db, name: &ModuleName) -> Option<(SearchPath, File, Mod if is_builtin_module && !search_path.is_standard_library() { continue; } + let mut components = name.components(); let module_name = components.next_back()?; @@ -1282,6 +1283,7 @@ mod tests { db.memory_file_system() .remove_directory(foo_init_path.parent().unwrap())?; File::sync_path(&mut db, &foo_init_path); + File::sync_path(&mut db, foo_init_path.parent().unwrap()); let foo_module = resolve_module(&db, foo_module_name).expect("Foo module to resolve"); assert_eq!(&src.join("foo.py"), foo_module.file().path(&db)); @@ -1312,7 +1314,7 @@ mod tests { let functools_module = resolve_module(&db, functools_module_name.clone()).unwrap(); assert_eq!(functools_module.search_path(), &stdlib); assert_eq!( - Some(functools_module.file()), + Ok(functools_module.file()), system_path_to_file(&db, &stdlib_functools_path) ); @@ -1332,7 +1334,7 @@ mod tests { ); assert_eq!(functools_module.search_path(), &stdlib); assert_eq!( - Some(functools_module.file()), + Ok(functools_module.file()), system_path_to_file(&db, &stdlib_functools_path) ); } @@ -1358,7 +1360,7 @@ mod tests { let functools_module = resolve_module(&db, functools_module_name.clone()).unwrap(); assert_eq!(functools_module.search_path(), &stdlib); assert_eq!( - Some(functools_module.file()), + Ok(functools_module.file()), system_path_to_file(&db, stdlib.join("functools.pyi")) ); @@ -1369,7 +1371,7 @@ mod tests { let functools_module = resolve_module(&db, functools_module_name.clone()).unwrap(); assert_eq!(functools_module.search_path(), &src); assert_eq!( - Some(functools_module.file()), + Ok(functools_module.file()), system_path_to_file(&db, &src_functools_path) ); } @@ -1400,7 +1402,7 @@ mod tests { let functools_module = resolve_module(&db, functools_module_name.clone()).unwrap(); assert_eq!(functools_module.search_path(), &src); assert_eq!( - Some(functools_module.file()), + Ok(functools_module.file()), system_path_to_file(&db, &src_functools_path) ); @@ -1413,7 +1415,7 @@ mod tests { let functools_module = resolve_module(&db, functools_module_name.clone()).unwrap(); assert_eq!(functools_module.search_path(), &stdlib); assert_eq!( - Some(functools_module.file()), + Ok(functools_module.file()), system_path_to_file(&db, stdlib.join("functools.pyi")) ); } diff --git a/crates/red_knot_module_resolver/src/state.rs b/crates/red_knot_module_resolver/src/state.rs index 048504f60c..ec32c3e791 100644 --- a/crates/red_knot_module_resolver/src/state.rs +++ b/crates/red_knot_module_resolver/src/state.rs @@ -1,5 +1,4 @@ use ruff_db::program::TargetVersion; -use ruff_db::system::System; use ruff_db::vendored::VendoredFileSystem; use crate::db::Db; @@ -20,10 +19,6 @@ impl<'db> ResolverState<'db> { } } - pub(crate) fn system(&self) -> &dyn System { - self.db.system() - } - pub(crate) fn vendored(&self) -> &VendoredFileSystem { self.db.vendored() } diff --git a/crates/red_knot_module_resolver/src/typeshed/versions.rs b/crates/red_knot_module_resolver/src/typeshed/versions.rs index d0aef6e0bd..e5aae22c5f 100644 --- a/crates/red_knot_module_resolver/src/typeshed/versions.rs +++ b/crates/red_knot_module_resolver/src/typeshed/versions.rs @@ -52,7 +52,7 @@ impl<'db> LazyTypeshedVersions<'db> { } else { return &VENDORED_VERSIONS; }; - let Some(versions_file) = system_path_to_file(db.upcast(), &versions_path) else { + let Ok(versions_file) = system_path_to_file(db.upcast(), &versions_path) else { todo!( "Still need to figure out how to handle VERSIONS files being deleted \ from custom typeshed directories! Expected a file to exist at {versions_path}" diff --git a/crates/ruff_db/src/files.rs b/crates/ruff_db/src/files.rs index 88c768c7eb..2ad371542b 100644 --- a/crates/ruff_db/src/files.rs +++ b/crates/ruff_db/src/files.rs @@ -1,8 +1,9 @@ +use std::fmt::Formatter; use std::sync::Arc; use countme::Count; use dashmap::mapref::entry::Entry; -use salsa::Setter; +use salsa::{Durability, Setter}; pub use file_root::{FileRoot, FileRootKind}; pub use path::FilePath; @@ -13,28 +14,35 @@ use crate::files::file_root::FileRoots; use crate::files::private::FileStatus; use crate::system::{Metadata, SystemPath, SystemPathBuf, SystemVirtualPath, SystemVirtualPathBuf}; use crate::vendored::{VendoredPath, VendoredPathBuf}; -use crate::{Db, FxDashMap}; +use crate::{vendored, Db, FxDashMap}; mod file_root; mod path; /// Interns a file system path and returns a salsa `File` ingredient. /// -/// Returns `None` if the path doesn't exist, isn't accessible, or if the path points to a directory. +/// Returns `Err` if the path doesn't exist, isn't accessible, or if the path points to a directory. #[inline] -pub fn system_path_to_file(db: &dyn Db, path: impl AsRef) -> Option { +pub fn system_path_to_file(db: &dyn Db, path: impl AsRef) -> Result { let file = db.files().system(db, path.as_ref()); // It's important that `vfs.file_system` creates a `VfsFile` even for files that don't exist or don't // exist anymore so that Salsa can track that the caller of this function depends on the existence of // that file. This function filters out files that don't exist, but Salsa will know that it must // re-run the calling query whenever the `file`'s status changes (because of the `.status` call here). - file.exists(db).then_some(file) + match file.status(db) { + FileStatus::Exists => Ok(file), + FileStatus::IsADirectory => Err(FileError::IsADirectory), + FileStatus::NotFound => Err(FileError::NotFound), + } } /// Interns a vendored file path. Returns `Some` if the vendored file for `path` exists and `None` otherwise. #[inline] -pub fn vendored_path_to_file(db: &dyn Db, path: impl AsRef) -> Option { +pub fn vendored_path_to_file( + db: &dyn Db, + path: impl AsRef, +) -> Result { db.files().vendored(db, path.as_ref()) } @@ -68,7 +76,7 @@ impl Files { /// For a non-existing file, creates a new salsa [`File`] ingredient and stores it for future lookups. /// /// The operation always succeeds even if the path doesn't exist on disk, isn't accessible or if the path points to a directory. - /// In these cases, a file with status [`FileStatus::Deleted`] is returned. + /// In these cases, a file with status [`FileStatus::NotFound`] is returned. #[tracing::instrument(level = "trace", skip(self, db))] fn system(&self, db: &dyn Db, path: &SystemPath) -> File { let absolute = SystemPath::absolute(path, db.system().current_directory()); @@ -78,27 +86,32 @@ impl Files { .system_by_path .entry(absolute.clone()) .or_insert_with(|| { - // TODO: Set correct durability according to source root. let metadata = db.system().path_metadata(path); + let durability = self + .root(db, path) + .map_or(Durability::default(), |root| root.durability(db)); - match metadata { - Ok(metadata) if metadata.file_type().is_file() => File::new( - db, - FilePath::System(absolute), + let (permissions, revision, status) = match metadata { + Ok(metadata) if metadata.file_type().is_file() => ( metadata.permissions(), metadata.revision(), FileStatus::Exists, - Count::default(), ), - _ => File::new( - db, - FilePath::System(absolute), - None, - FileRevision::zero(), - FileStatus::Deleted, - Count::default(), - ), - } + Ok(metadata) if metadata.file_type().is_directory() => { + (None, FileRevision::zero(), FileStatus::IsADirectory) + } + _ => (None, FileRevision::zero(), FileStatus::NotFound), + }; + + File::builder( + FilePath::System(absolute), + permissions, + revision, + status, + Count::default(), + ) + .durability(durability) + .new(db) }) } @@ -114,20 +127,27 @@ impl Files { /// Looks up a vendored file by its path. Returns `Some` if a vendored file for the given path /// exists and `None` otherwise. #[tracing::instrument(level = "trace", skip(self, db))] - fn vendored(&self, db: &dyn Db, path: &VendoredPath) -> Option { + fn vendored(&self, db: &dyn Db, path: &VendoredPath) -> Result { let file = match self.inner.vendored_by_path.entry(path.to_path_buf()) { Entry::Occupied(entry) => *entry.get(), Entry::Vacant(entry) => { - let metadata = db.vendored().metadata(path).ok()?; + let metadata = match db.vendored().metadata(path) { + Ok(metadata) => match metadata.kind() { + vendored::FileType::File => metadata, + vendored::FileType::Directory => return Err(FileError::IsADirectory), + }, + Err(_) => return Err(FileError::NotFound), + }; - let file = File::new( - db, + let file = File::builder( FilePath::Vendored(path.to_path_buf()), Some(0o444), metadata.revision(), FileStatus::Exists, Count::default(), - ); + ) + .durability(Durability::HIGH) + .new(db); entry.insert(file); @@ -135,7 +155,7 @@ impl Files { } }; - Some(file) + Ok(file) } /// Looks up a virtual file by its `path`. @@ -210,8 +230,7 @@ impl Files { let inner = Arc::clone(&db.files().inner); for entry in inner.system_by_path.iter_mut() { if entry.key().starts_with(&path) { - let file = entry.value(); - file.sync(db); + File::sync_system_path(db, entry.key(), Some(*entry.value())); } } @@ -219,7 +238,9 @@ impl Files { for root in roots.all() { if root.path(db).starts_with(&path) { - root.set_revision(db).to(FileRevision::now()); + root.set_revision(db) + .with_durability(Durability::HIGH) + .to(FileRevision::now()); } } } @@ -236,14 +257,15 @@ impl Files { pub fn sync_all(db: &mut dyn Db) { let inner = Arc::clone(&db.files().inner); for entry in inner.system_by_path.iter_mut() { - let file = entry.value(); - file.sync(db); + File::sync_system_path(db, entry.key(), Some(*entry.value())); } let roots = inner.roots.read().unwrap(); for root in roots.all() { - root.set_revision(db).to(FileRevision::now()); + root.set_revision(db) + .with_durability(Durability::HIGH) + .to(FileRevision::now()); } } } @@ -335,6 +357,7 @@ impl File { #[tracing::instrument(level = "debug", skip(db))] pub fn sync_path(db: &mut dyn Db, path: &SystemPath) { let absolute = SystemPath::absolute(path, db.system().current_directory()); + Files::touch_root(db, &absolute); Self::sync_system_path(db, &absolute, None); } @@ -345,6 +368,7 @@ impl File { match path { FilePath::System(system) => { + Files::touch_root(db, &system); Self::sync_system_path(db, &system, Some(self)); } FilePath::Vendored(_) => { @@ -357,34 +381,56 @@ impl File { } fn sync_system_path(db: &mut dyn Db, path: &SystemPath, file: Option) { - Files::touch_root(db, path); 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); + let durability = db.files().root(db, path).map(|root| root.durability(db)); + Self::sync_impl(db, metadata, file, durability); } 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); + Self::sync_impl(db, metadata, file, None); } /// 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) { + fn sync_impl( + db: &mut dyn Db, + metadata: crate::system::Result, + file: File, + durability: Option, + ) { let (status, revision, permission) = match metadata { Ok(metadata) if metadata.file_type().is_file() => ( FileStatus::Exists, metadata.revision(), metadata.permissions(), ), - _ => (FileStatus::Deleted, FileRevision::zero(), None), + Ok(metadata) if metadata.file_type().is_directory() => { + (FileStatus::IsADirectory, FileRevision::zero(), None) + } + _ => (FileStatus::NotFound, FileRevision::zero(), None), }; - file.set_status(db).to(status); - file.set_revision(db).to(revision); - file.set_permissions(db).to(permission); + let durability = durability.unwrap_or_default(); + + if file.status(db) != status { + file.set_status(db).with_durability(durability).to(status); + } + + if file.revision(db) != revision { + file.set_revision(db) + .with_durability(durability) + .to(revision); + } + + if file.permissions(db) != permission { + file.set_permissions(db) + .with_durability(durability) + .to(permission); + } } /// Returns `true` if the file exists. @@ -401,15 +447,35 @@ mod private { /// The file exists. Exists, - /// The file was deleted, didn't exist to begin with or the path isn't a file. - Deleted, + /// The path isn't a file and instead points to a directory. + IsADirectory, + + /// The path doesn't exist, isn't accessible, or no longer exists. + NotFound, } } +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum FileError { + IsADirectory, + NotFound, +} + +impl std::fmt::Display for FileError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + FileError::IsADirectory => f.write_str("Is a directory"), + FileError::NotFound => f.write_str("Not found"), + } + } +} + +impl std::error::Error for FileError {} + #[cfg(test)] mod tests { use crate::file_revision::FileRevision; - use crate::files::{system_path_to_file, vendored_path_to_file}; + use crate::files::{system_path_to_file, vendored_path_to_file, FileError}; use crate::system::DbWithTestSystem; use crate::tests::TestDb; use crate::vendored::tests::VendoredFileSystemBuilder; @@ -435,7 +501,7 @@ mod tests { let test = system_path_to_file(&db, "test.py"); - assert_eq!(test, None); + assert_eq!(test, Err(FileError::NotFound)); } #[test] @@ -477,6 +543,9 @@ mod tests { fn stubbed_vendored_file_non_existing() { let db = TestDb::new(); - assert_eq!(vendored_path_to_file(&db, "test.py"), None); + assert_eq!( + vendored_path_to_file(&db, "test.py"), + Err(FileError::NotFound) + ); } } diff --git a/crates/ruff_db/src/files/file_root.rs b/crates/ruff_db/src/files/file_root.rs index 3eb64609b6..6375655edd 100644 --- a/crates/ruff_db/src/files/file_root.rs +++ b/crates/ruff_db/src/files/file_root.rs @@ -1,6 +1,7 @@ use std::fmt::Formatter; use path_slash::PathExt; +use salsa::Durability; use crate::file_revision::FileRevision; use crate::system::{SystemPath, SystemPathBuf}; @@ -83,7 +84,9 @@ impl FileRoots { let mut route = normalized_path.replace('{', "{{").replace('}', "}}"); // Insert a new source root - let root = FileRoot::new(db, path, kind, FileRevision::now()); + let root = FileRoot::builder(path, kind, FileRevision::now()) + .durability(Durability::HIGH) + .new(db); // Insert a path that matches the root itself self.by_path.insert(route.clone(), root).unwrap(); diff --git a/crates/ruff_db/src/files/path.rs b/crates/ruff_db/src/files/path.rs index a1c3530ab0..816eaf461a 100644 --- a/crates/ruff_db/src/files/path.rs +++ b/crates/ruff_db/src/files/path.rs @@ -95,8 +95,8 @@ impl FilePath { #[inline] pub fn to_file(&self, db: &dyn Db) -> Option { match self { - FilePath::System(path) => system_path_to_file(db, path), - FilePath::Vendored(path) => vendored_path_to_file(db, path), + FilePath::System(path) => system_path_to_file(db, path).ok(), + FilePath::Vendored(path) => vendored_path_to_file(db, path).ok(), FilePath::SystemVirtual(_) => None, } } diff --git a/crates/ruff_db/src/program.rs b/crates/ruff_db/src/program.rs index 83716ebeae..c5cdc30de6 100644 --- a/crates/ruff_db/src/program.rs +++ b/crates/ruff_db/src/program.rs @@ -1,4 +1,5 @@ use crate::{system::SystemPathBuf, Db}; +use salsa::Durability; #[salsa::input(singleton)] pub struct Program { @@ -10,7 +11,9 @@ pub struct Program { impl Program { pub fn from_settings(db: &dyn Db, settings: ProgramSettings) -> Self { - Program::new(db, settings.target_version, settings.search_paths) + Program::builder(settings.target_version, settings.search_paths) + .durability(Durability::HIGH) + .new(db) } } diff --git a/crates/ruff_db/src/system/memory_fs.rs b/crates/ruff_db/src/system/memory_fs.rs index be4406691b..3754a5b9c2 100644 --- a/crates/ruff_db/src/system/memory_fs.rs +++ b/crates/ruff_db/src/system/memory_fs.rs @@ -207,7 +207,9 @@ impl MemoryFileSystem { let normalized = self.normalize_path(path.as_ref()); - get_or_create_file(&mut by_path, &normalized)?.content = content.to_string(); + let file = get_or_create_file(&mut by_path, &normalized)?; + file.content = content.to_string(); + file.last_modified = FileTime::now(); Ok(()) } diff --git a/crates/ruff_db/src/system/test.rs b/crates/ruff_db/src/system/test.rs index 3a25954224..c5f6dd4060 100644 --- a/crates/ruff_db/src/system/test.rs +++ b/crates/ruff_db/src/system/test.rs @@ -1,3 +1,7 @@ +use std::any::Any; +use std::panic::RefUnwindSafe; +use std::sync::Arc; + use ruff_notebook::{Notebook, NotebookError}; use ruff_python_trivia::textwrap; @@ -6,9 +10,6 @@ use crate::system::{ DirectoryEntry, MemoryFileSystem, Metadata, Result, System, SystemPath, SystemVirtualPath, }; use crate::Db; -use std::any::Any; -use std::panic::RefUnwindSafe; -use std::sync::Arc; use super::walk_directory::WalkDirectoryBuilder;