Eagerly validate search paths (#12783)

Co-authored-by: Alex Waygood <[email protected]>

Author: MichaReiser

crates/red_knot/src/main.rs

@@ -184,7 +184,7 @@ fn run() -> anyhow::Result<ExitStatus> {
 
     // TODO: Use the `program_settings` to compute the key for the database's persistent
     //   cache and load the cache if it exists.
-    let mut db = RootDatabase::new(workspace_metadata, program_settings, system);
+    let mut db = RootDatabase::new(workspace_metadata, program_settings, system)?;
 
     let (main_loop, main_loop_cancellation_token) = MainLoop::new();
 

crates/red_knot/tests/file_watching.rs

@@ -4,7 +4,6 @@ use std::io::Write;
 use std::time::Duration;
 
 use anyhow::{anyhow, Context};
-use salsa::Setter;
 
 use red_knot_python_semantic::{
     resolve_module, ModuleName, Program, ProgramSettings, PythonVersion, SearchPathSettings,
@@ -26,6 +25,7 @@ struct TestCase {
     /// We need to hold on to it in the test case or the temp files get deleted.
     _temp_dir: tempfile::TempDir,
     root_dir: SystemPathBuf,
+    search_path_settings: SearchPathSettings,
 }
 
 impl TestCase {
@@ -108,18 +108,20 @@ impl TestCase {
     fn update_search_path_settings(
         &mut self,
         f: impl FnOnce(&SearchPathSettings) -> SearchPathSettings,
-    ) {
+    ) -> anyhow::Result<()> {
         let program = Program::get(self.db());
-        let search_path_settings = program.search_paths(self.db());
 
-        let new_settings = f(search_path_settings);
+        let new_settings = f(&self.search_path_settings);
 
-        program.set_search_paths(&mut self.db).to(new_settings);
+        program.update_search_paths(&mut self.db, new_settings.clone())?;
+        self.search_path_settings = new_settings;
 
         if let Some(watcher) = &mut self.watcher {
             watcher.update(&self.db);
             assert!(!watcher.has_errored_paths());
         }
+
+        Ok(())
     }
 
     fn collect_package_files(&self, path: &SystemPath) -> Vec<File> {
@@ -221,24 +223,24 @@ where
     let system = OsSystem::new(&workspace_path);
 
     let workspace = WorkspaceMetadata::from_path(&workspace_path, &system)?;
-    let search_paths = create_search_paths(&root_path, workspace.root());
+    let search_path_settings = create_search_paths(&root_path, workspace.root());
 
-    for path in search_paths
+    for path in search_path_settings
         .extra_paths
         .iter()
-        .chain(search_paths.site_packages.iter())
-        .chain(search_paths.custom_typeshed.iter())
+        .chain(search_path_settings.site_packages.iter())
+        .chain(search_path_settings.custom_typeshed.iter())
     {
         std::fs::create_dir_all(path.as_std_path())
             .with_context(|| format!("Failed to create search path '{path}'"))?;
     }
 
     let settings = ProgramSettings {
         target_version: PythonVersion::default(),
-        search_paths,
+        search_paths: search_path_settings.clone(),
     };
 
-    let db = RootDatabase::new(workspace, settings, system);
+    let db = RootDatabase::new(workspace, settings, system)?;
 
     let (sender, receiver) = crossbeam::channel::unbounded();
     let watcher = directory_watcher(move |events| sender.send(events).unwrap())
@@ -253,6 +255,7 @@ where
         watcher: Some(watcher),
         _temp_dir: temp_dir,
         root_dir: root_path,
+        search_path_settings,
     };
 
     // Sometimes the file watcher reports changes for events that happened before the watcher was started.
@@ -737,7 +740,8 @@ fn add_search_path() -> anyhow::Result<()> {
     case.update_search_path_settings(|settings| SearchPathSettings {
         site_packages: vec![site_packages.clone()],
         ..settings.clone()
-    });
+    })
+    .expect("Search path settings to be valid");
 
     std::fs::write(site_packages.join("a.py").as_std_path(), "class A: ...")?;
 
@@ -767,7 +771,8 @@ fn remove_search_path() -> anyhow::Result<()> {
     case.update_search_path_settings(|settings| SearchPathSettings {
         site_packages: vec![],
         ..settings.clone()
-    });
+    })
+    .expect("Search path settings to be valid");
 
     std::fs::write(site_packages.join("a.py").as_std_path(), "class A: ...")?;
 

crates/red_knot_python_semantic/Cargo.toml

@@ -17,6 +17,7 @@ ruff_python_ast = { workspace = true }
 ruff_python_stdlib = { workspace = true }
 ruff_text_size = { workspace = true }
 
+anyhow = { workspace = true }
 bitflags = { workspace = true }
 camino = { workspace = true }
 compact_str = { workspace = true }
@@ -34,7 +35,7 @@ walkdir = { workspace = true }
 zip = { workspace = true, features = ["zstd", "deflate"] }
 
 [dev-dependencies]
-ruff_db = { workspace = true, features = ["os", "testing"]}
+ruff_db = { workspace = true, features = ["os", "testing"] }
 ruff_python_parser = { workspace = true }
 
 anyhow = { workspace = true }

crates/red_knot_python_semantic/src/module_resolver/mod.rs

@@ -2,11 +2,13 @@ use std::iter::FusedIterator;
 
 pub(crate) use module::Module;
 pub use resolver::resolve_module;
+pub(crate) use resolver::SearchPaths;
 use ruff_db::system::SystemPath;
 pub use typeshed::vendored_typeshed_stubs;
 
+use crate::module_resolver::resolver::search_paths;
 use crate::Db;
-use resolver::{module_resolution_settings, SearchPathIterator};
+use resolver::SearchPathIterator;
 
 mod module;
 mod path;
@@ -20,7 +22,7 @@ mod testing;
 /// Returns an iterator over all search paths pointing to a system path
 pub fn system_module_search_paths(db: &dyn Db) -> SystemModuleSearchPathsIter {
     SystemModuleSearchPathsIter {
-        inner: module_resolution_settings(db).search_paths(db),
+        inner: search_paths(db),
     }
 }