diff --git a/crates/red_knot/src/main.rs b/crates/red_knot/src/main.rs index fbe313cd33940..38db69d866e2b 100644 --- a/crates/red_knot/src/main.rs +++ b/crates/red_knot/src/main.rs @@ -184,7 +184,7 @@ fn run() -> 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(); diff --git a/crates/red_knot/tests/file_watching.rs b/crates/red_knot/tests/file_watching.rs index 1315c8fd6de9b..31789f02313c9 100644 --- a/crates/red_knot/tests/file_watching.rs +++ b/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 { @@ -221,13 +223,13 @@ 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}'"))?; @@ -235,10 +237,10 @@ where 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: ...")?; diff --git a/crates/red_knot_python_semantic/Cargo.toml b/crates/red_knot_python_semantic/Cargo.toml index 06e7e21297cc9..4694c9c3a694f 100644 --- a/crates/red_knot_python_semantic/Cargo.toml +++ b/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 } diff --git a/crates/red_knot_python_semantic/src/module_resolver/mod.rs b/crates/red_knot_python_semantic/src/module_resolver/mod.rs index 000ccb8387fd7..06f13271f0819 100644 --- a/crates/red_knot_python_semantic/src/module_resolver/mod.rs +++ b/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), } } diff --git a/crates/red_knot_python_semantic/src/module_resolver/resolver.rs b/crates/red_knot_python_semantic/src/module_resolver/resolver.rs index 913abd6b1ed4a..ad2b0583c5007 100644 --- a/crates/red_knot_python_semantic/src/module_resolver/resolver.rs +++ b/crates/red_knot_python_semantic/src/module_resolver/resolver.rs @@ -7,12 +7,13 @@ use ruff_db::files::{File, FilePath, FileRootKind}; use ruff_db::system::{DirectoryEntry, SystemPath, SystemPathBuf}; use ruff_db::vendored::VendoredPath; +use crate::db::Db; +use crate::module_name::ModuleName; +use crate::{Program, SearchPathSettings}; + use super::module::{Module, ModuleKind}; use super::path::{ModulePath, SearchPath, SearchPathValidationError}; use super::state::ResolverState; -use crate::db::Db; -use crate::module_name::ModuleName; -use crate::{Program, PythonVersion, SearchPathSettings}; /// Resolves a module name to a module. pub fn resolve_module(db: &dyn Db, module_name: ModuleName) -> Option { @@ -84,9 +85,7 @@ pub(crate) fn file_to_module(db: &dyn Db, file: File) -> Option { FilePath::SystemVirtual(_) => return None, }; - let settings = module_resolution_settings(db); - - let mut search_paths = settings.search_paths(db); + let mut search_paths = search_paths(db); let module_name = loop { let candidate = search_paths.next()?; @@ -119,106 +118,122 @@ pub(crate) fn file_to_module(db: &dyn Db, file: File) -> Option { } } -/// Validate and normalize the raw settings given by the user -/// into settings we can use for module resolution -/// -/// This method also implements the typing spec's [module resolution order]. -/// -/// [module resolution order]: https://typing.readthedocs.io/en/latest/spec/distributing.html#import-resolution-ordering -fn try_resolve_module_resolution_settings( - db: &dyn Db, -) -> Result { - let program = Program::get(db.upcast()); - - let SearchPathSettings { - extra_paths, - src_root, - custom_typeshed, - site_packages, - } = program.search_paths(db.upcast()); - - if !extra_paths.is_empty() { - tracing::info!("Extra search paths: {extra_paths:?}"); - } - - if let Some(custom_typeshed) = custom_typeshed { - tracing::info!("Custom typeshed directory: {custom_typeshed}"); - } +pub(crate) fn search_paths(db: &dyn Db) -> SearchPathIterator { + Program::get(db).search_paths(db).iter(db) +} - let system = db.system(); - let files = db.files(); +#[derive(Debug, PartialEq, Eq, Default)] +pub(crate) struct SearchPaths { + /// Search paths that have been statically determined purely from reading Ruff's configuration settings. + /// These shouldn't ever change unless the config settings themselves change. + static_paths: Vec, - let mut static_search_paths = vec![]; + /// site-packages paths are not included in the above field: + /// if there are multiple site-packages paths, editable installations can appear + /// *between* the site-packages paths on `sys.path` at runtime. + /// That means we can't know where a second or third `site-packages` path should sit + /// in terms of module-resolution priority until we've discovered the editable installs + /// for the first `site-packages` path + site_packages: Vec, +} - for path in extra_paths { - let search_path = SearchPath::extra(system, path.clone())?; - files.try_add_root( - db.upcast(), - search_path.as_system_path().unwrap(), - FileRootKind::LibrarySearchPath, - ); - static_search_paths.push(search_path); - } +impl SearchPaths { + /// Validate and normalize the raw settings given by the user + /// into settings we can use for module resolution + /// + /// This method also implements the typing spec's [module resolution order]. + /// + /// [module resolution order]: https://typing.readthedocs.io/en/latest/spec/distributing.html#import-resolution-ordering + pub(crate) fn from_settings( + db: &dyn Db, + settings: SearchPathSettings, + ) -> Result { + let SearchPathSettings { + extra_paths, + src_root, + custom_typeshed, + site_packages: site_packages_paths, + } = settings; - static_search_paths.push(SearchPath::first_party(system, src_root.clone())?); + let system = db.system(); + let files = db.files(); - static_search_paths.push(if let Some(custom_typeshed) = custom_typeshed.as_ref() { - let search_path = SearchPath::custom_stdlib(db, custom_typeshed.clone())?; - files.try_add_root( - db.upcast(), - search_path.as_system_path().unwrap(), - FileRootKind::LibrarySearchPath, - ); - search_path - } else { - SearchPath::vendored_stdlib() - }); + let mut static_paths = vec![]; - let mut site_packages_paths: Vec<_> = Vec::with_capacity(site_packages.len()); + for path in extra_paths { + tracing::debug!("Adding static extra search-path '{path}'"); - for path in site_packages { - let search_path = SearchPath::site_packages(system, path.to_path_buf())?; - files.try_add_root( - db.upcast(), - search_path.as_system_path().unwrap(), - FileRootKind::LibrarySearchPath, - ); - site_packages_paths.push(search_path); - } + let search_path = SearchPath::extra(system, path)?; + files.try_add_root( + db.upcast(), + search_path.as_system_path().unwrap(), + FileRootKind::LibrarySearchPath, + ); + static_paths.push(search_path); + } - // TODO vendor typeshed's third-party stubs as well as the stdlib and fallback to them as a final step + tracing::debug!("Adding static search path '{src_root}'"); + static_paths.push(SearchPath::first_party(system, src_root)?); - let target_version = program.target_version(db.upcast()); - tracing::info!("Target version: {target_version}"); + static_paths.push(if let Some(custom_typeshed) = custom_typeshed { + tracing::debug!("Adding static custom-sdtlib search-path '{custom_typeshed}'"); - // Filter out module resolution paths that point to the same directory on disk (the same invariant maintained by [`sys.path` at runtime]). - // (Paths may, however, *overlap* -- e.g. you could have both `src/` and `src/foo` - // as module resolution paths simultaneously.) - // - // [`sys.path` at runtime]: https://docs.python.org/3/library/site.html#module-site - // This code doesn't use an `IndexSet` because the key is the system path and not the search root. - let mut seen_paths = - FxHashSet::with_capacity_and_hasher(static_search_paths.len(), FxBuildHasher); - - static_search_paths.retain(|path| { - if let Some(path) = path.as_system_path() { - seen_paths.insert(path.to_path_buf()) + let search_path = SearchPath::custom_stdlib(db, custom_typeshed)?; + files.try_add_root( + db.upcast(), + search_path.as_system_path().unwrap(), + FileRootKind::LibrarySearchPath, + ); + search_path } else { - true + SearchPath::vendored_stdlib() + }); + + let mut site_packages: Vec<_> = Vec::with_capacity(site_packages_paths.len()); + + for path in site_packages_paths { + tracing::debug!("Adding site-package path '{path}'"); + let search_path = SearchPath::site_packages(system, path)?; + files.try_add_root( + db.upcast(), + search_path.as_system_path().unwrap(), + FileRootKind::LibrarySearchPath, + ); + site_packages.push(search_path); } - }); - Ok(ModuleResolutionSettings { - target_version, - static_search_paths, - site_packages_paths, - }) -} + // TODO vendor typeshed's third-party stubs as well as the stdlib and fallback to them as a final step + + // Filter out module resolution paths that point to the same directory on disk (the same invariant maintained by [`sys.path` at runtime]). + // (Paths may, however, *overlap* -- e.g. you could have both `src/` and `src/foo` + // as module resolution paths simultaneously.) + // + // This code doesn't use an `IndexSet` because the key is the system path and not the search root. + // + // [`sys.path` at runtime]: https://docs.python.org/3/library/site.html#module-site + let mut seen_paths = FxHashSet::with_capacity_and_hasher(static_paths.len(), FxBuildHasher); + + static_paths.retain(|path| { + if let Some(path) = path.as_system_path() { + seen_paths.insert(path.to_path_buf()) + } else { + true + } + }); -#[salsa::tracked(return_ref)] -pub(crate) fn module_resolution_settings(db: &dyn Db) -> ModuleResolutionSettings { - // TODO proper error handling if this returns an error: - try_resolve_module_resolution_settings(db).unwrap() + Ok(SearchPaths { + static_paths, + site_packages, + }) + } + + pub(crate) fn iter<'a>(&'a self, db: &'a dyn Db) -> SearchPathIterator<'a> { + SearchPathIterator { + db, + static_paths: self.static_paths.iter(), + dynamic_paths: None, + } + } } /// Collect all dynamic search paths. For each `site-packages` path: @@ -231,19 +246,20 @@ pub(crate) fn module_resolution_settings(db: &dyn Db) -> ModuleResolutionSetting /// module-resolution priority. #[salsa::tracked(return_ref)] pub(crate) fn dynamic_resolution_paths(db: &dyn Db) -> Vec { - let ModuleResolutionSettings { - target_version: _, - static_search_paths, - site_packages_paths, - } = module_resolution_settings(db); + tracing::debug!("Resolving dynamic module resolution paths"); + + let SearchPaths { + static_paths, + site_packages, + } = Program::get(db).search_paths(db); let mut dynamic_paths = Vec::new(); - if site_packages_paths.is_empty() { + if site_packages.is_empty() { return dynamic_paths; } - let mut existing_paths: FxHashSet<_> = static_search_paths + let mut existing_paths: FxHashSet<_> = static_paths .iter() .filter_map(|path| path.as_system_path()) .map(Cow::Borrowed) @@ -252,7 +268,7 @@ pub(crate) fn dynamic_resolution_paths(db: &dyn Db) -> Vec { let files = db.files(); let system = db.system(); - for site_packages_search_path in site_packages_paths { + for site_packages_search_path in site_packages { let site_packages_dir = site_packages_search_path .as_system_path() .expect("Expected site package path to be a system path"); @@ -302,6 +318,10 @@ pub(crate) fn dynamic_resolution_paths(db: &dyn Db) -> Vec { if existing_paths.insert(Cow::Owned(installation.clone())) { match SearchPath::editable(system, installation) { Ok(search_path) => { + tracing::debug!( + "Adding editable installation to module resolution path {path}", + path = search_path.as_system_path().unwrap() + ); dynamic_paths.push(search_path); } @@ -448,38 +468,6 @@ impl<'db> Iterator for PthFileIterator<'db> { } } -/// Validated and normalized module-resolution settings. -#[derive(Clone, Debug, PartialEq, Eq)] -pub(crate) struct ModuleResolutionSettings { - target_version: PythonVersion, - - /// Search paths that have been statically determined purely from reading Ruff's configuration settings. - /// These shouldn't ever change unless the config settings themselves change. - static_search_paths: Vec, - - /// site-packages paths are not included in the above field: - /// if there are multiple site-packages paths, editable installations can appear - /// *between* the site-packages paths on `sys.path` at runtime. - /// That means we can't know where a second or third `site-packages` path should sit - /// in terms of module-resolution priority until we've discovered the editable installs - /// for the first `site-packages` path - site_packages_paths: Vec, -} - -impl ModuleResolutionSettings { - fn target_version(&self) -> PythonVersion { - self.target_version - } - - pub(crate) fn search_paths<'db>(&'db self, db: &'db dyn Db) -> SearchPathIterator<'db> { - SearchPathIterator { - db, - static_paths: self.static_search_paths.iter(), - dynamic_paths: None, - } - } -} - /// A thin wrapper around `ModuleName` to make it a Salsa ingredient. /// /// This is needed because Salsa requires that all query arguments are salsa ingredients. @@ -492,13 +480,13 @@ struct ModuleNameIngredient<'db> { /// Given a module name and a list of search paths in which to lookup modules, /// attempt to resolve the module name fn resolve_name(db: &dyn Db, name: &ModuleName) -> Option<(SearchPath, File, ModuleKind)> { - let resolver_settings = module_resolution_settings(db); - let target_version = resolver_settings.target_version(); + let program = Program::get(db); + let target_version = program.target_version(db); let resolver_state = ResolverState::new(db, target_version); let is_builtin_module = ruff_python_stdlib::sys::is_builtin_module(target_version.minor, name.as_str()); - for search_path in resolver_settings.search_paths(db) { + for search_path in search_paths(db) { // When a builtin module is imported, standard module resolution is bypassed: // the module name always resolves to the stdlib module, // even if there's a module of the same name in the first-party root @@ -652,6 +640,8 @@ mod tests { use crate::module_name::ModuleName; use crate::module_resolver::module::ModuleKind; use crate::module_resolver::testing::{FileSpec, MockedTypeshed, TestCase, TestCaseBuilder}; + use crate::ProgramSettings; + use crate::PythonVersion; use super::*; @@ -1202,14 +1192,19 @@ mod tests { std::fs::write(foo.as_std_path(), "")?; std::os::unix::fs::symlink(foo.as_std_path(), bar.as_std_path())?; - let search_paths = SearchPathSettings { - extra_paths: vec![], - src_root: src.clone(), - custom_typeshed: Some(custom_typeshed.clone()), - site_packages: vec![site_packages], - }; - - Program::new(&db, PythonVersion::PY38, search_paths); + Program::from_settings( + &db, + ProgramSettings { + target_version: PythonVersion::PY38, + search_paths: SearchPathSettings { + extra_paths: vec![], + src_root: src.clone(), + custom_typeshed: Some(custom_typeshed.clone()), + site_packages: vec![site_packages], + }, + }, + ) + .context("Invalid program settings")?; let foo_module = resolve_module(&db, ModuleName::new_static("foo").unwrap()).unwrap(); let bar_module = resolve_module(&db, ModuleName::new_static("bar").unwrap()).unwrap(); @@ -1673,8 +1668,7 @@ not_a_directory .with_site_packages_files(&[("_foo.pth", "/src")]) .build(); - let search_paths: Vec<&SearchPath> = - module_resolution_settings(&db).search_paths(&db).collect(); + let search_paths: Vec<&SearchPath> = search_paths(&db).collect(); assert!(search_paths.contains( &&SearchPath::first_party(db.system(), SystemPathBuf::from("/src")).unwrap() @@ -1703,16 +1697,19 @@ not_a_directory ]) .unwrap(); - Program::new( + Program::from_settings( &db, - PythonVersion::default(), - SearchPathSettings { - extra_paths: vec![], - src_root: SystemPathBuf::from("/src"), - custom_typeshed: None, - site_packages: vec![venv_site_packages, system_site_packages], + ProgramSettings { + target_version: PythonVersion::default(), + search_paths: SearchPathSettings { + extra_paths: vec![], + src_root: SystemPathBuf::from("/src"), + custom_typeshed: None, + site_packages: vec![venv_site_packages, system_site_packages], + }, }, - ); + ) + .expect("Valid program settings"); // The editable installs discovered from the `.pth` file in the first `site-packages` directory // take precedence over the second `site-packages` directory... diff --git a/crates/red_knot_python_semantic/src/module_resolver/testing.rs b/crates/red_knot_python_semantic/src/module_resolver/testing.rs index a754348403f8a..87a05001113c7 100644 --- a/crates/red_knot_python_semantic/src/module_resolver/testing.rs +++ b/crates/red_knot_python_semantic/src/module_resolver/testing.rs @@ -4,6 +4,7 @@ use ruff_db::vendored::VendoredPathBuf; use crate::db::tests::TestDb; use crate::program::{Program, SearchPathSettings}; use crate::python_version::PythonVersion; +use crate::ProgramSettings; /// A test case for the module resolver. /// @@ -220,16 +221,19 @@ impl TestCaseBuilder { let src = Self::write_mock_directory(&mut db, "/src", first_party_files); let typeshed = Self::build_typeshed_mock(&mut db, &typeshed_option); - Program::new( + Program::from_settings( &db, - target_version, - SearchPathSettings { - extra_paths: vec![], - src_root: src.clone(), - custom_typeshed: Some(typeshed.clone()), - site_packages: vec![site_packages.clone()], + ProgramSettings { + target_version, + search_paths: SearchPathSettings { + extra_paths: vec![], + src_root: src.clone(), + custom_typeshed: Some(typeshed.clone()), + site_packages: vec![site_packages.clone()], + }, }, - ); + ) + .expect("Valid program settings"); TestCase { db, @@ -273,16 +277,19 @@ impl TestCaseBuilder { Self::write_mock_directory(&mut db, "/site-packages", site_packages_files); let src = Self::write_mock_directory(&mut db, "/src", first_party_files); - Program::new( + Program::from_settings( &db, - target_version, - SearchPathSettings { - extra_paths: vec![], - src_root: src.clone(), - custom_typeshed: None, - site_packages: vec![site_packages.clone()], + ProgramSettings { + target_version, + search_paths: SearchPathSettings { + extra_paths: vec![], + src_root: src.clone(), + custom_typeshed: None, + site_packages: vec![site_packages.clone()], + }, }, - ); + ) + .expect("Valid search path settings"); TestCase { db, diff --git a/crates/red_knot_python_semantic/src/program.rs b/crates/red_knot_python_semantic/src/program.rs index 7b79caed38a1f..082d6b06dc774 100644 --- a/crates/red_knot_python_semantic/src/program.rs +++ b/crates/red_knot_python_semantic/src/program.rs @@ -1,21 +1,53 @@ use crate::python_version::PythonVersion; -use crate::Db; -use ruff_db::system::SystemPathBuf; +use anyhow::Context; use salsa::Durability; +use salsa::Setter; + +use ruff_db::system::SystemPathBuf; + +use crate::module_resolver::SearchPaths; +use crate::Db; #[salsa::input(singleton)] pub struct Program { pub target_version: PythonVersion, + #[default] #[return_ref] - pub search_paths: SearchPathSettings, + pub(crate) search_paths: SearchPaths, } impl Program { - pub fn from_settings(db: &dyn Db, settings: ProgramSettings) -> Self { - Program::builder(settings.target_version, settings.search_paths) + pub fn from_settings(db: &dyn Db, settings: ProgramSettings) -> anyhow::Result { + let ProgramSettings { + target_version, + search_paths, + } = settings; + + tracing::info!("Target version: {target_version}"); + + let search_paths = SearchPaths::from_settings(db, search_paths) + .with_context(|| "Invalid search path settings")?; + + Ok(Program::builder(settings.target_version) .durability(Durability::HIGH) - .new(db) + .search_paths(search_paths) + .new(db)) + } + + pub fn update_search_paths( + &self, + db: &mut dyn Db, + search_path_settings: SearchPathSettings, + ) -> anyhow::Result<()> { + let search_paths = SearchPaths::from_settings(db, search_path_settings)?; + + if self.search_paths(db) != &search_paths { + tracing::debug!("Update search paths"); + self.set_search_paths(db).to(search_paths); + } + + Ok(()) } } diff --git a/crates/red_knot_python_semantic/src/semantic_model.rs b/crates/red_knot_python_semantic/src/semantic_model.rs index 4b8b24be0b043..ee7b571e223c4 100644 --- a/crates/red_knot_python_semantic/src/semantic_model.rs +++ b/crates/red_knot_python_semantic/src/semantic_model.rs @@ -171,29 +171,32 @@ mod tests { use crate::program::{Program, SearchPathSettings}; use crate::python_version::PythonVersion; use crate::types::Type; - use crate::{HasTy, SemanticModel}; + use crate::{HasTy, ProgramSettings, SemanticModel}; - fn setup_db() -> TestDb { - let db = TestDb::new(); - Program::new( + fn setup_db<'a>(files: impl IntoIterator) -> anyhow::Result { + let mut db = TestDb::new(); + db.write_files(files)?; + + Program::from_settings( &db, - PythonVersion::default(), - SearchPathSettings { - extra_paths: vec![], - src_root: SystemPathBuf::from("/src"), - site_packages: vec![], - custom_typeshed: None, + ProgramSettings { + target_version: PythonVersion::default(), + search_paths: SearchPathSettings { + extra_paths: vec![], + src_root: SystemPathBuf::from("/src"), + site_packages: vec![], + custom_typeshed: None, + }, }, - ); + )?; - db + Ok(db) } #[test] fn function_ty() -> anyhow::Result<()> { - let mut db = setup_db(); + let db = setup_db([("/src/foo.py", "def test(): pass")])?; - db.write_file("/src/foo.py", "def test(): pass")?; let foo = system_path_to_file(&db, "/src/foo.py").unwrap(); let ast = parsed_module(&db, foo); @@ -209,9 +212,8 @@ mod tests { #[test] fn class_ty() -> anyhow::Result<()> { - let mut db = setup_db(); + let db = setup_db([("/src/foo.py", "class Test: pass")])?; - db.write_file("/src/foo.py", "class Test: pass")?; let foo = system_path_to_file(&db, "/src/foo.py").unwrap(); let ast = parsed_module(&db, foo); @@ -227,12 +229,11 @@ mod tests { #[test] fn alias_ty() -> anyhow::Result<()> { - let mut db = setup_db(); - - db.write_files([ + let db = setup_db([ ("/src/foo.py", "class Test: pass"), ("/src/bar.py", "from foo import Test"), ])?; + let bar = system_path_to_file(&db, "/src/bar.py").unwrap(); let ast = parsed_module(&db, bar); diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 40cf6eb873f4f..4e14325673afd 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -1494,6 +1494,7 @@ impl<'db> TypeInferenceBuilder<'db> { #[cfg(test)] mod tests { + use anyhow::Context; use ruff_db::files::{system_path_to_file, File}; use ruff_db::parsed::parsed_module; use ruff_db::system::{DbWithTestSystem, SystemPathBuf}; @@ -1508,40 +1509,58 @@ mod tests { use crate::semantic_index::symbol::FileScopeId; use crate::semantic_index::{global_scope, semantic_index, symbol_table, use_def_map}; use crate::types::{global_symbol_ty_by_name, infer_definition_types, symbol_ty_by_name, Type}; - use crate::{HasTy, SemanticModel}; + use crate::{HasTy, ProgramSettings, SemanticModel}; fn setup_db() -> TestDb { let db = TestDb::new(); - Program::new( + let src_root = SystemPathBuf::from("/src"); + db.memory_file_system() + .create_directory_all(&src_root) + .unwrap(); + + Program::from_settings( &db, - PythonVersion::default(), - SearchPathSettings { - extra_paths: Vec::new(), - src_root: SystemPathBuf::from("/src"), - site_packages: vec![], - custom_typeshed: None, + ProgramSettings { + target_version: PythonVersion::default(), + search_paths: SearchPathSettings { + extra_paths: Vec::new(), + src_root, + site_packages: vec![], + custom_typeshed: None, + }, }, - ); + ) + .expect("Valid search path settings"); db } - fn setup_db_with_custom_typeshed(typeshed: &str) -> TestDb { - let db = TestDb::new(); + fn setup_db_with_custom_typeshed<'a>( + typeshed: &str, + files: impl IntoIterator, + ) -> anyhow::Result { + let mut db = TestDb::new(); + let src_root = SystemPathBuf::from("/src"); + + db.write_files(files) + .context("Failed to write test files")?; - Program::new( + Program::from_settings( &db, - PythonVersion::default(), - SearchPathSettings { - extra_paths: Vec::new(), - src_root: SystemPathBuf::from("/src"), - site_packages: vec![], - custom_typeshed: Some(SystemPathBuf::from(typeshed)), + ProgramSettings { + target_version: PythonVersion::default(), + search_paths: SearchPathSettings { + extra_paths: Vec::new(), + src_root, + site_packages: vec![], + custom_typeshed: Some(SystemPathBuf::from(typeshed)), + }, }, - ); + ) + .context("Failed to create Program")?; - db + Ok(db) } fn assert_public_ty(db: &TestDb, file_name: &str, symbol_name: &str, expected: &str) { @@ -2131,16 +2150,17 @@ mod tests { #[test] fn builtin_symbol_custom_stdlib() -> anyhow::Result<()> { - let mut db = setup_db_with_custom_typeshed("/typeshed"); - - db.write_files([ - ("/src/a.py", "c = copyright"), - ( - "/typeshed/stdlib/builtins.pyi", - "def copyright() -> None: ...", - ), - ("/typeshed/stdlib/VERSIONS", "builtins: 3.8-"), - ])?; + let db = setup_db_with_custom_typeshed( + "/typeshed", + [ + ("/src/a.py", "c = copyright"), + ( + "/typeshed/stdlib/builtins.pyi", + "def copyright() -> None: ...", + ), + ("/typeshed/stdlib/VERSIONS", "builtins: 3.8-"), + ], + )?; assert_public_ty(&db, "/src/a.py", "c", "Literal[copyright]"); @@ -2160,13 +2180,14 @@ mod tests { #[test] fn unknown_builtin_later_defined() -> anyhow::Result<()> { - let mut db = setup_db_with_custom_typeshed("/typeshed"); - - db.write_files([ - ("/src/a.py", "x = foo"), - ("/typeshed/stdlib/builtins.pyi", "foo = bar; bar = 1"), - ("/typeshed/stdlib/VERSIONS", "builtins: 3.8-"), - ])?; + let db = setup_db_with_custom_typeshed( + "/typeshed", + [ + ("/src/a.py", "x = foo"), + ("/typeshed/stdlib/builtins.pyi", "foo = bar; bar = 1"), + ("/typeshed/stdlib/VERSIONS", "builtins: 3.8-"), + ], + )?; assert_public_ty(&db, "/src/a.py", "x", "Unbound"); diff --git a/crates/red_knot_server/src/session.rs b/crates/red_knot_server/src/session.rs index 594a370085375..fe2c09a33bc64 100644 --- a/crates/red_knot_server/src/session.rs +++ b/crates/red_knot_server/src/session.rs @@ -78,7 +78,8 @@ impl Session { custom_typeshed: None, }, }; - workspaces.insert(path, RootDatabase::new(metadata, program_settings, system)); + // TODO(micha): Handle the case where the program settings are incorrect more gracefully. + workspaces.insert(path, RootDatabase::new(metadata, program_settings, system)?); } Ok(Self { diff --git a/crates/red_knot_wasm/src/lib.rs b/crates/red_knot_wasm/src/lib.rs index 87d06b6a21262..4bdfd9c9b2a5d 100644 --- a/crates/red_knot_wasm/src/lib.rs +++ b/crates/red_knot_wasm/src/lib.rs @@ -49,7 +49,8 @@ impl Workspace { search_paths: SearchPathSettings::default(), }; - let db = RootDatabase::new(workspace, program_settings, system.clone()); + let db = + RootDatabase::new(workspace, program_settings, system.clone()).map_err(into_error)?; Ok(Self { db, system }) } diff --git a/crates/red_knot_workspace/src/db.rs b/crates/red_knot_workspace/src/db.rs index a181ec504829c..216885caf3899 100644 --- a/crates/red_knot_workspace/src/db.rs +++ b/crates/red_knot_workspace/src/db.rs @@ -28,7 +28,11 @@ pub struct RootDatabase { } impl RootDatabase { - pub fn new(workspace: WorkspaceMetadata, settings: ProgramSettings, system: S) -> Self + pub fn new( + workspace: WorkspaceMetadata, + settings: ProgramSettings, + system: S, + ) -> anyhow::Result where S: System + 'static + Send + Sync + RefUnwindSafe, { @@ -41,10 +45,10 @@ impl RootDatabase { let workspace = Workspace::from_metadata(&db, workspace); // Initialize the `Program` singleton - Program::from_settings(&db, settings); + Program::from_settings(&db, settings)?; db.workspace = Some(workspace); - db + Ok(db) } pub fn workspace(&self) -> Workspace { diff --git a/crates/red_knot_workspace/src/lint.rs b/crates/red_knot_workspace/src/lint.rs index d1dfedcf094d5..c50cebbbe154e 100644 --- a/crates/red_knot_workspace/src/lint.rs +++ b/crates/red_knot_workspace/src/lint.rs @@ -305,7 +305,7 @@ enum AnyImportRef<'a> { #[cfg(test)] mod tests { - use red_knot_python_semantic::{Program, PythonVersion, SearchPathSettings}; + use red_knot_python_semantic::{Program, ProgramSettings, PythonVersion, SearchPathSettings}; use ruff_db::files::system_path_to_file; use ruff_db::system::{DbWithTestSystem, SystemPathBuf}; @@ -320,16 +320,23 @@ mod tests { fn setup_db_with_root(src_root: SystemPathBuf) -> TestDb { let db = TestDb::new(); - Program::new( + db.memory_file_system() + .create_directory_all(&src_root) + .unwrap(); + + Program::from_settings( &db, - PythonVersion::default(), - SearchPathSettings { - extra_paths: Vec::new(), - src_root, - site_packages: vec![], - custom_typeshed: None, + ProgramSettings { + target_version: PythonVersion::default(), + search_paths: SearchPathSettings { + extra_paths: Vec::new(), + src_root, + site_packages: vec![], + custom_typeshed: None, + }, }, - ); + ) + .expect("Valid program settings"); db } diff --git a/crates/red_knot_workspace/tests/check.rs b/crates/red_knot_workspace/tests/check.rs index dfbc14101b7ce..219f005f2bb80 100644 --- a/crates/red_knot_workspace/tests/check.rs +++ b/crates/red_knot_workspace/tests/check.rs @@ -20,8 +20,7 @@ fn setup_db(workspace_root: SystemPathBuf) -> anyhow::Result { target_version: PythonVersion::default(), search_paths, }; - let db = RootDatabase::new(workspace, settings, system); - Ok(db) + RootDatabase::new(workspace, settings, system) } /// Test that all snippets in testcorpus can be checked without panic diff --git a/crates/ruff_benchmark/benches/red_knot.rs b/crates/ruff_benchmark/benches/red_knot.rs index eb0f3638f85c9..ca275bce8e2d6 100644 --- a/crates/ruff_benchmark/benches/red_knot.rs +++ b/crates/ruff_benchmark/benches/red_knot.rs @@ -52,7 +52,7 @@ fn setup_case() -> Case { }, }; - let mut db = RootDatabase::new(metadata, settings, system); + let mut db = RootDatabase::new(metadata, settings, system).unwrap(); let parser = system_path_to_file(&db, parser_path).unwrap(); db.workspace().open_file(&mut db, parser);