[red-knot] Eagerly validate search paths before constructing Programs

crates/red_knot/src/main.rs

@@ -14,7 +14,7 @@ use red_knot_workspace::db::RootDatabase;
 use red_knot_workspace::watch;
 use red_knot_workspace::watch::WorkspaceWatcher;
 use red_knot_workspace::workspace::WorkspaceMetadata;
-use ruff_db::program::{ProgramSettings, SearchPathSettings};
+use ruff_db::program::{RawProgramSettings, RawSearchPathSettings};
 use ruff_db::system::{OsSystem, System, SystemPathBuf};
 use target_version::TargetVersion;
 use verbosity::{Verbosity, VerbosityLevel};
@@ -120,9 +120,9 @@ pub fn main() -> anyhow::Result<()> {
         WorkspaceMetadata::from_path(system.current_directory(), &system).unwrap();
 
     // TODO: Respect the settings from the workspace metadata. when resolving the program settings.
-    let program_settings = ProgramSettings {
+    let program_settings = RawProgramSettings {
         target_version: target_version.into(),
-        search_paths: SearchPathSettings {
+        search_paths: RawSearchPathSettings {
             extra_paths,
             src_root: workspace_metadata.root().to_path_buf(),
             custom_typeshed: custom_typeshed_dir,
@@ -132,7 +132,7 @@ pub fn main() -> anyhow::Result<()> {
 
     // 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(verbosity);
 

crates/red_knot/tests/file_watching.rs

@@ -6,13 +6,15 @@ use std::time::Duration;
 use anyhow::{anyhow, Context};
 use salsa::Setter;
 
-use red_knot_module_resolver::{resolve_module, ModuleName};
+use red_knot_module_resolver::{
+    resolve_module, try_resolve_module_resolution_settings, ModuleName,
+};
 use red_knot_workspace::db::RootDatabase;
 use red_knot_workspace::watch;
 use red_knot_workspace::watch::{directory_watcher, WorkspaceWatcher};
 use red_knot_workspace::workspace::WorkspaceMetadata;
 use ruff_db::files::{system_path_to_file, File, FileError};
-use ruff_db::program::{Program, ProgramSettings, SearchPathSettings, TargetVersion};
+use ruff_db::program::{Program, RawProgramSettings, RawSearchPathSettings, TargetVersion};
 use ruff_db::source::source_text;
 use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf};
 use ruff_db::Upcast;
@@ -24,6 +26,7 @@ struct TestCase {
     /// The temporary directory that contains the test files.
     /// We need to hold on to it in the test case or the temp files get deleted.
     _temp_dir: tempfile::TempDir,
+    initial_settings: RawProgramSettings,
     root_dir: SystemPathBuf,
 }
 
@@ -106,14 +109,17 @@ impl TestCase {
 
     fn update_search_path_settings(
         &mut self,
-        f: impl FnOnce(&SearchPathSettings) -> SearchPathSettings,
+        f: impl FnOnce(&RawSearchPathSettings) -> RawSearchPathSettings,
     ) {
         let program = Program::get(self.db());
-        let search_path_settings = program.search_paths(self.db());
+        let initial_path_settings = &self.initial_settings.search_paths;
 
-        let new_settings = f(search_path_settings);
+        let new_settings =
+            try_resolve_module_resolution_settings(self.db(), f(initial_path_settings)).unwrap();
 
-        program.set_search_paths(&mut self.db).to(new_settings);
+        program
+            .set_search_path_settings(&mut self.db)
+            .to(new_settings);
 
         if let Some(watcher) = &mut self.watcher {
             watcher.update(&self.db);
@@ -177,7 +183,7 @@ fn setup<F>(setup_files: F) -> anyhow::Result<TestCase>
 where
     F: SetupFiles,
 {
-    setup_with_search_paths(setup_files, |_root, workspace_path| SearchPathSettings {
+    setup_with_search_paths(setup_files, |_root, workspace_path| RawSearchPathSettings {
         extra_paths: vec![],
         src_root: workspace_path.to_path_buf(),
         custom_typeshed: None,
@@ -187,7 +193,7 @@ where
 
 fn setup_with_search_paths<F>(
     setup_files: F,
-    create_search_paths: impl FnOnce(&SystemPath, &SystemPath) -> SearchPathSettings,
+    create_search_paths: impl FnOnce(&SystemPath, &SystemPath) -> RawSearchPathSettings,
 ) -> anyhow::Result<TestCase>
 where
     F: SetupFiles,
@@ -232,12 +238,12 @@ where
             .with_context(|| format!("Failed to create search path '{path}'"))?;
     }
 
-    let settings = ProgramSettings {
+    let settings = RawProgramSettings {
         target_version: TargetVersion::default(),
         search_paths,
     };
 
-    let db = RootDatabase::new(workspace, settings, system);
+    let db = RootDatabase::new(workspace, settings.clone(), system)?;
 
     let (sender, receiver) = crossbeam::channel::unbounded();
     let watcher = directory_watcher(move |events| sender.send(events).unwrap())
@@ -252,6 +258,7 @@ where
         watcher: Some(watcher),
         _temp_dir: temp_dir,
         root_dir: root_path,
+        initial_settings: settings,
     };
 
     // Sometimes the file watcher reports changes for events that happened before the watcher was started.
@@ -693,7 +700,7 @@ fn directory_deleted() -> anyhow::Result<()> {
 fn search_path() -> anyhow::Result<()> {
     let mut case =
         setup_with_search_paths([("bar.py", "import sub.a")], |root_path, workspace_path| {
-            SearchPathSettings {
+            RawSearchPathSettings {
                 extra_paths: vec![],
                 src_root: workspace_path.to_path_buf(),
                 custom_typeshed: None,
@@ -733,7 +740,7 @@ fn add_search_path() -> anyhow::Result<()> {
     assert!(resolve_module(case.db().upcast(), ModuleName::new_static("a").unwrap()).is_none());
 
     // Register site-packages as a search path.
-    case.update_search_path_settings(|settings| SearchPathSettings {
+    case.update_search_path_settings(|settings| RawSearchPathSettings {
         site_packages: vec![site_packages.clone()],
         ..settings.clone()
     });
@@ -753,7 +760,7 @@ fn add_search_path() -> anyhow::Result<()> {
 fn remove_search_path() -> anyhow::Result<()> {
     let mut case =
         setup_with_search_paths([("bar.py", "import sub.a")], |root_path, workspace_path| {
-            SearchPathSettings {
+            RawSearchPathSettings {
                 extra_paths: vec![],
                 src_root: workspace_path.to_path_buf(),
                 custom_typeshed: None,
@@ -763,7 +770,7 @@ fn remove_search_path() -> anyhow::Result<()> {
 
     // Remove site packages from the search path settings.
     let site_packages = case.root_path().join("site_packages");
-    case.update_search_path_settings(|settings| SearchPathSettings {
+    case.update_search_path_settings(|settings| RawSearchPathSettings {
         site_packages: vec![],
         ..settings.clone()
     });
@@ -1171,7 +1178,7 @@ mod unix {
 
                 Ok(())
             },
-            |_root, workspace| SearchPathSettings {
+            |_root, workspace| RawSearchPathSettings {
                 extra_paths: vec![],
                 src_root: workspace.to_path_buf(),
                 custom_typeshed: None,

crates/red_knot_module_resolver/src/lib.rs

@@ -3,48 +3,49 @@ use std::iter::FusedIterator;
 pub use db::Db;
 pub use module::{Module, ModuleKind};
 pub use module_name::ModuleName;
+pub use path::SearchPathValidationError;
 pub use resolver::resolve_module;
-use ruff_db::system::SystemPath;
+pub use settings_resolution::{program_from_raw_settings, try_resolve_module_resolution_settings};
 pub use typeshed::{
     vendored_typeshed_stubs, TypeshedVersionsParseError, TypeshedVersionsParseErrorKind,
 };
 
-use crate::resolver::{module_resolution_settings, SearchPathIterator};
-
 mod db;
 mod module;
 mod module_name;
 mod path;
 mod resolver;
+mod settings_resolution;
 mod state;
 mod typeshed;
 
 #[cfg(test)]
 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),
+/// Returns an iterator over all search paths
+pub fn module_search_paths(db: &dyn Db) -> ModuleSearchPathsIter {
+    ModuleSearchPathsIter {
+        inner: resolver::module_search_paths(db),
     }
 }
 
-pub struct SystemModuleSearchPathsIter<'db> {
-    inner: SearchPathIterator<'db>,
+// Unlike the internal `SearchPathIterator` struct,
+// which yields instances of the private
+// `red_knot_module_resolver::path::SearchPath` type,
+// this public iterator yields instances of the public
+// `ruff_db::program::SearchPath` type
+pub struct ModuleSearchPathsIter<'db> {
+    inner: resolver::SearchPathIterator<'db>,
 }
 
-impl<'db> Iterator for SystemModuleSearchPathsIter<'db> {
-    type Item = &'db SystemPath;
+impl<'db> Iterator for ModuleSearchPathsIter<'db> {
+    type Item = ruff_db::program::SearchPath;
 
     fn next(&mut self) -> Option<Self::Item> {
-        loop {
-            let next = self.inner.next()?;
-
-            if let Some(system_path) = next.as_system_path() {
-                return Some(system_path);
-            }
-        }
+        self.inner
+            .next()
+            .map(|path| ruff_db::program::SearchPath::from(&*path))
     }
 }
 
-impl FusedIterator for SystemModuleSearchPathsIter<'_> {}
+impl FusedIterator for ModuleSearchPathsIter<'_> {}

crates/red_knot_module_resolver/src/path.rs

@@ -6,6 +6,7 @@ use std::sync::Arc;
 use camino::{Utf8Path, Utf8PathBuf};
 
 use ruff_db::files::{system_path_to_file, vendored_path_to_file, File, FileError};
+use ruff_db::program::SearchPathInner;
 use ruff_db::system::{System, SystemPath, SystemPathBuf};
 use ruff_db::vendored::{VendoredPath, VendoredPathBuf};
 
@@ -293,7 +294,7 @@ fn query_stdlib_version(
 /// a message must be displayed to the user,
 /// as type checking cannot be done reliably in these circumstances.
 #[derive(Debug, PartialEq, Eq)]
-pub(crate) enum SearchPathValidationError {
+pub enum SearchPathValidationError {
     /// The path provided by the user was not a directory
     NotADirectory(SystemPathBuf),
 
@@ -343,16 +344,6 @@ impl std::error::Error for SearchPathValidationError {
 
 type SearchPathResult<T> = Result<T, SearchPathValidationError>;
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
-enum SearchPathInner {
-    Extra(SystemPathBuf),
-    FirstParty(SystemPathBuf),
-    StandardLibraryCustom(SystemPathBuf),
-    StandardLibraryVendored(VendoredPathBuf),
-    SitePackages(SystemPathBuf),
-    Editable(SystemPathBuf),
-}
-
 /// Unification of the various kinds of search paths
 /// that can be used to locate Python modules.
 ///
@@ -611,6 +602,30 @@ impl PartialEq<SearchPath> for VendoredPathBuf {
     }
 }
 
+impl From<ruff_db::program::SearchPath> for SearchPath {
+    fn from(value: ruff_db::program::SearchPath) -> Self {
+        Self(value.0)
+    }
+}
+
+impl From<&ruff_db::program::SearchPath> for SearchPath {
+    fn from(value: &ruff_db::program::SearchPath) -> Self {
+        Self(Arc::clone(&value.0))
+    }
+}
+
+impl From<SearchPath> for ruff_db::program::SearchPath {
+    fn from(value: SearchPath) -> Self {
+        ruff_db::program::SearchPath(value.0)
+    }
+}
+
+impl From<&SearchPath> for ruff_db::program::SearchPath {
+    fn from(value: &SearchPath) -> Self {
+        ruff_db::program::SearchPath(Arc::clone(&value.0))
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use ruff_db::program::TargetVersion;