Set Durability to 'HIGH' for most inputs and third-party libraries (#…

…12566)

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"] }

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);
                 }
             }

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<File> {
     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);
         }
     }

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.

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<SystemPath>) -> Option<File> {
+    fn system_file(&self, path: impl AsRef<SystemPath>) -> Result<File, FileError> {
         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]);

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| {