From dce87c21fdf73a58f3821cae5e71b9da234e29ce Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Wed, 21 Aug 2024 17:49:53 +0200 Subject: [PATCH] Eagerly validate typeshed versions (#12786) --- Cargo.lock | 3 +- crates/red_knot/src/main.rs | 106 +++++----- crates/red_knot/tests/file_watching.rs | 197 +++++++++++------- crates/red_knot_python_semantic/Cargo.toml | 1 + crates/red_knot_python_semantic/src/lib.rs | 3 +- .../src/module_resolver/mod.rs | 1 - .../src/module_resolver/path.rs | 148 +++++++------ .../src/module_resolver/resolver.rs | 171 +++++++++++---- .../src/module_resolver/state.rs | 25 --- .../src/module_resolver/testing.rs | 16 +- .../src/module_resolver/typeshed/mod.rs | 2 +- .../src/module_resolver/typeshed/versions.rs | 81 +------ .../red_knot_python_semantic/src/program.rs | 42 +++- .../src/semantic_model.rs | 9 +- .../src/site_packages.rs | 15 +- crates/red_knot_python_semantic/src/types.rs | 9 +- .../src/types/infer.rs | 15 +- crates/red_knot_server/Cargo.toml | 1 - crates/red_knot_server/src/session.rs | 14 +- crates/red_knot_wasm/src/lib.rs | 23 +- crates/red_knot_workspace/Cargo.toml | 1 - crates/red_knot_workspace/src/db.rs | 24 +-- crates/red_knot_workspace/src/db/changes.rs | 42 +++- crates/red_knot_workspace/src/lib.rs | 1 - crates/red_knot_workspace/src/lint.rs | 9 +- crates/red_knot_workspace/src/workspace.rs | 29 ++- .../src/workspace/metadata.rs | 24 ++- .../src/workspace/settings.rs | 89 ++++++++ crates/red_knot_workspace/tests/check.rs | 32 +-- crates/ruff_benchmark/benches/red_knot.rs | 37 ++-- crates/ruff_workspace/src/resolver.rs | 10 +- 31 files changed, 679 insertions(+), 501 deletions(-) delete mode 100644 crates/red_knot_python_semantic/src/module_resolver/state.rs rename crates/{red_knot_workspace => red_knot_python_semantic}/src/site_packages.rs (99%) create mode 100644 crates/red_knot_workspace/src/workspace/settings.rs diff --git a/Cargo.lock b/Cargo.lock index a7eb65c90a7cf..a660c9abbd17c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1935,6 +1935,7 @@ dependencies = [ "smallvec", "static_assertions", "tempfile", + "thiserror", "tracing", "walkdir", "zip", @@ -1950,7 +1951,6 @@ dependencies = [ "libc", "lsp-server", "lsp-types", - "red_knot_python_semantic", "red_knot_workspace", "ruff_db", "ruff_notebook", @@ -1995,7 +1995,6 @@ dependencies = [ "ruff_text_size", "rustc-hash 2.0.0", "salsa", - "thiserror", "tracing", ] diff --git a/crates/red_knot/src/main.rs b/crates/red_knot/src/main.rs index 38db69d866e2b..cd0355233abd8 100644 --- a/crates/red_knot/src/main.rs +++ b/crates/red_knot/src/main.rs @@ -7,12 +7,12 @@ use colored::Colorize; use crossbeam::channel as crossbeam_channel; use salsa::plumbing::ZalsaDatabase; -use red_knot_python_semantic::{ProgramSettings, SearchPathSettings}; +use red_knot_python_semantic::SitePackages; use red_knot_server::run_server; use red_knot_workspace::db::RootDatabase; -use red_knot_workspace::site_packages::VirtualEnvironment; use red_knot_workspace::watch; use red_knot_workspace::watch::WorkspaceWatcher; +use red_knot_workspace::workspace::settings::Configuration; use red_knot_workspace::workspace::WorkspaceMetadata; use ruff_db::system::{OsSystem, System, SystemPath, SystemPathBuf}; use target_version::TargetVersion; @@ -65,15 +65,14 @@ to resolve type information for the project's third-party dependencies.", value_name = "PATH", help = "Additional path to use as a module-resolution source (can be passed multiple times)" )] - extra_search_path: Vec, + extra_search_path: Option>, #[arg( long, help = "Python version to assume when resolving types", - default_value_t = TargetVersion::default(), - value_name="VERSION") - ] - target_version: TargetVersion, + value_name = "VERSION" + )] + target_version: Option, #[clap(flatten)] verbosity: Verbosity, @@ -86,6 +85,36 @@ to resolve type information for the project's third-party dependencies.", watch: bool, } +impl Args { + fn to_configuration(&self, cli_cwd: &SystemPath) -> Configuration { + let mut configuration = Configuration::default(); + + if let Some(target_version) = self.target_version { + configuration.target_version = Some(target_version.into()); + } + + if let Some(venv_path) = &self.venv_path { + configuration.search_paths.site_packages = Some(SitePackages::Derived { + venv_path: SystemPath::absolute(venv_path, cli_cwd), + }); + } + + if let Some(custom_typeshed_dir) = &self.custom_typeshed_dir { + configuration.search_paths.custom_typeshed = + Some(SystemPath::absolute(custom_typeshed_dir, cli_cwd)); + } + + if let Some(extra_search_paths) = &self.extra_search_path { + configuration.search_paths.extra_paths = extra_search_paths + .iter() + .map(|path| Some(SystemPath::absolute(path, cli_cwd))) + .collect(); + } + + configuration + } +} + #[derive(Debug, clap::Subcommand)] pub enum Command { /// Start the language server @@ -115,22 +144,13 @@ pub fn main() -> ExitStatus { } fn run() -> anyhow::Result { - let Args { - command, - current_directory, - custom_typeshed_dir, - extra_search_path: extra_paths, - venv_path, - target_version, - verbosity, - watch, - } = Args::parse_from(std::env::args().collect::>()); - - if matches!(command, Some(Command::Server)) { + let args = Args::parse_from(std::env::args().collect::>()); + + if matches!(args.command, Some(Command::Server)) { return run_server().map(|()| ExitStatus::Success); } - let verbosity = verbosity.level(); + let verbosity = args.verbosity.level(); countme::enable(verbosity.is_trace()); let _guard = setup_tracing(verbosity)?; @@ -146,10 +166,12 @@ fn run() -> anyhow::Result { })? }; - let cwd = current_directory + let cwd = args + .current_directory + .as_ref() .map(|cwd| { if cwd.as_std_path().is_dir() { - Ok(SystemPath::absolute(&cwd, &cli_base_path)) + Ok(SystemPath::absolute(cwd, &cli_base_path)) } else { Err(anyhow!( "Provided current-directory path '{cwd}' is not a directory." @@ -160,33 +182,18 @@ fn run() -> anyhow::Result { .unwrap_or_else(|| cli_base_path.clone()); let system = OsSystem::new(cwd.clone()); - let workspace_metadata = WorkspaceMetadata::from_path(system.current_directory(), &system)?; - - // TODO: Verify the remaining search path settings eagerly. - let site_packages = venv_path - .map(|path| { - VirtualEnvironment::new(path, &OsSystem::new(cli_base_path)) - .and_then(|venv| venv.site_packages_directories(&system)) - }) - .transpose()? - .unwrap_or_default(); - - // TODO: Respect the settings from the workspace metadata. when resolving the program settings. - let program_settings = ProgramSettings { - target_version: target_version.into(), - search_paths: SearchPathSettings { - extra_paths, - src_root: workspace_metadata.root().to_path_buf(), - custom_typeshed: custom_typeshed_dir, - site_packages, - }, - }; + let cli_configuration = args.to_configuration(&cwd); + let workspace_metadata = WorkspaceMetadata::from_path( + system.current_directory(), + &system, + Some(cli_configuration.clone()), + )?; // 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, system)?; - let (main_loop, main_loop_cancellation_token) = MainLoop::new(); + let (main_loop, main_loop_cancellation_token) = MainLoop::new(cli_configuration); // Listen to Ctrl+C and abort the watch mode. let main_loop_cancellation_token = Mutex::new(Some(main_loop_cancellation_token)); @@ -198,7 +205,7 @@ fn run() -> anyhow::Result { } })?; - let exit_status = if watch { + let exit_status = if args.watch { main_loop.watch(&mut db)? } else { main_loop.run(&mut db) @@ -238,10 +245,12 @@ struct MainLoop { /// The file system watcher, if running in watch mode. watcher: Option, + + cli_configuration: Configuration, } impl MainLoop { - fn new() -> (Self, MainLoopCancellationToken) { + fn new(cli_configuration: Configuration) -> (Self, MainLoopCancellationToken) { let (sender, receiver) = crossbeam_channel::bounded(10); ( @@ -249,6 +258,7 @@ impl MainLoop { sender: sender.clone(), receiver, watcher: None, + cli_configuration, }, MainLoopCancellationToken { sender }, ) @@ -331,7 +341,7 @@ impl MainLoop { MainLoopMessage::ApplyChanges(changes) => { revision += 1; // Automatically cancels any pending queries and waits for them to complete. - db.apply_changes(changes); + db.apply_changes(changes, Some(&self.cli_configuration)); if let Some(watcher) = self.watcher.as_mut() { watcher.update(db); } diff --git a/crates/red_knot/tests/file_watching.rs b/crates/red_knot/tests/file_watching.rs index 93d659f1049d6..7e23ac100f702 100644 --- a/crates/red_knot/tests/file_watching.rs +++ b/crates/red_knot/tests/file_watching.rs @@ -5,12 +5,11 @@ use std::time::Duration; use anyhow::{anyhow, Context}; -use red_knot_python_semantic::{ - resolve_module, ModuleName, Program, ProgramSettings, PythonVersion, SearchPathSettings, -}; +use red_knot_python_semantic::{resolve_module, ModuleName, Program, PythonVersion, SitePackages}; use red_knot_workspace::db::RootDatabase; use red_knot_workspace::watch; use red_knot_workspace::watch::{directory_watcher, WorkspaceWatcher}; +use red_knot_workspace::workspace::settings::{Configuration, SearchPathConfiguration}; use red_knot_workspace::workspace::WorkspaceMetadata; use ruff_db::files::{system_path_to_file, File, FileError}; use ruff_db::source::source_text; @@ -25,7 +24,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, + configuration: Configuration, } impl TestCase { @@ -41,10 +40,6 @@ impl TestCase { &self.db } - fn db_mut(&mut self) -> &mut RootDatabase { - &mut self.db - } - fn stop_watch(&mut self) -> Vec { self.try_stop_watch(Duration::from_secs(10)) .expect("Expected watch changes but observed none.") @@ -105,16 +100,20 @@ impl TestCase { Some(all_events) } + fn apply_changes(&mut self, changes: Vec) { + self.db.apply_changes(changes, Some(&self.configuration)); + } + fn update_search_path_settings( &mut self, - f: impl FnOnce(&SearchPathSettings) -> SearchPathSettings, + configuration: SearchPathConfiguration, ) -> anyhow::Result<()> { let program = Program::get(self.db()); - let new_settings = f(&self.search_path_settings); + self.configuration.search_paths = configuration.clone(); + let new_settings = configuration.into_settings(self.db.workspace().root(&self.db)); - program.update_search_paths(&mut self.db, new_settings.clone())?; - self.search_path_settings = new_settings; + program.update_search_paths(&mut self.db, &new_settings)?; if let Some(watcher) = &mut self.watcher { watcher.update(&self.db); @@ -179,17 +178,14 @@ fn setup(setup_files: F) -> anyhow::Result where F: SetupFiles, { - setup_with_search_paths(setup_files, |_root, workspace_path| SearchPathSettings { - extra_paths: vec![], - src_root: workspace_path.to_path_buf(), - custom_typeshed: None, - site_packages: vec![], + setup_with_search_paths(setup_files, |_root, _workspace_path| { + SearchPathConfiguration::default() }) } fn setup_with_search_paths( setup_files: F, - create_search_paths: impl FnOnce(&SystemPath, &SystemPath) -> SearchPathSettings, + create_search_paths: impl FnOnce(&SystemPath, &SystemPath) -> SearchPathConfiguration, ) -> anyhow::Result where F: SetupFiles, @@ -221,25 +217,34 @@ where let system = OsSystem::new(&workspace_path); - let workspace = WorkspaceMetadata::from_path(&workspace_path, &system)?; - let search_path_settings = create_search_paths(&root_path, workspace.root()); + let search_paths = create_search_paths(&root_path, &workspace_path); - for path in search_path_settings + for path in search_paths .extra_paths .iter() - .chain(search_path_settings.site_packages.iter()) - .chain(search_path_settings.custom_typeshed.iter()) + .flatten() + .chain(search_paths.custom_typeshed.iter()) + .chain(search_paths.site_packages.iter().flat_map(|site_packages| { + if let SitePackages::Known(path) = site_packages { + path.as_slice() + } else { + &[] + } + })) { 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_path_settings.clone(), + let configuration = Configuration { + target_version: Some(PythonVersion::PY312), + search_paths, }; - let db = RootDatabase::new(workspace, settings, system)?; + let workspace = + WorkspaceMetadata::from_path(&workspace_path, &system, Some(configuration.clone()))?; + + let db = RootDatabase::new(workspace, system)?; let (sender, receiver) = crossbeam::channel::unbounded(); let watcher = directory_watcher(move |events| sender.send(events).unwrap()) @@ -254,7 +259,7 @@ where watcher: Some(watcher), _temp_dir: temp_dir, root_dir: root_path, - search_path_settings, + configuration, }; // Sometimes the file watcher reports changes for events that happened before the watcher was started. @@ -307,7 +312,7 @@ fn new_file() -> anyhow::Result<()> { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); let foo = case.system_file(&foo_path).expect("foo.py to exist."); @@ -330,7 +335,7 @@ fn new_ignored_file() -> anyhow::Result<()> { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); assert!(case.system_file(&foo_path).is_ok()); assert_eq!(&case.collect_package_files(&bar_path), &[bar_file]); @@ -354,7 +359,7 @@ fn changed_file() -> anyhow::Result<()> { assert!(!changes.is_empty()); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); assert_eq!(source_text(case.db(), foo).as_str(), "print('Version 2')"); assert_eq!(&case.collect_package_files(&foo_path), &[foo]); @@ -377,7 +382,7 @@ fn deleted_file() -> anyhow::Result<()> { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); assert!(!foo.exists(case.db())); assert_eq!(&case.collect_package_files(&foo_path), &[] as &[File]); @@ -409,7 +414,7 @@ fn move_file_to_trash() -> anyhow::Result<()> { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); assert!(!foo.exists(case.db())); assert_eq!(&case.collect_package_files(&foo_path), &[] as &[File]); @@ -441,7 +446,7 @@ fn move_file_to_workspace() -> anyhow::Result<()> { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); let foo_in_workspace = case.system_file(&foo_in_workspace_path)?; @@ -469,7 +474,7 @@ fn rename_file() -> anyhow::Result<()> { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); assert!(!foo.exists(case.db())); @@ -510,7 +515,7 @@ fn directory_moved_to_workspace() -> anyhow::Result<()> { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); let init_file = case .system_file(sub_new_path.join("__init__.py")) @@ -561,7 +566,7 @@ fn directory_moved_to_trash() -> anyhow::Result<()> { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); // `import sub.a` should no longer resolve assert!(resolve_module(case.db().upcast(), ModuleName::new_static("sub.a").unwrap()).is_none()); @@ -615,7 +620,7 @@ fn directory_renamed() -> anyhow::Result<()> { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); // `import sub.a` should no longer resolve assert!(resolve_module(case.db().upcast(), ModuleName::new_static("sub.a").unwrap()).is_none()); @@ -680,7 +685,7 @@ fn directory_deleted() -> anyhow::Result<()> { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); // `import sub.a` should no longer resolve assert!(resolve_module(case.db().upcast(), ModuleName::new_static("sub.a").unwrap()).is_none()); @@ -694,15 +699,13 @@ fn directory_deleted() -> anyhow::Result<()> { #[test] fn search_path() -> anyhow::Result<()> { - let mut case = - setup_with_search_paths([("bar.py", "import sub.a")], |root_path, workspace_path| { - SearchPathSettings { - extra_paths: vec![], - src_root: workspace_path.to_path_buf(), - custom_typeshed: None, - site_packages: vec![root_path.join("site_packages")], - } - })?; + let mut case = setup_with_search_paths( + [("bar.py", "import sub.a")], + |root_path, _workspace_path| SearchPathConfiguration { + site_packages: Some(SitePackages::Known(vec![root_path.join("site_packages")])), + ..SearchPathConfiguration::default() + }, + )?; let site_packages = case.root_path().join("site_packages"); @@ -715,7 +718,7 @@ fn search_path() -> anyhow::Result<()> { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); assert!(resolve_module(case.db().upcast(), ModuleName::new_static("a").unwrap()).is_some()); assert_eq!( @@ -736,9 +739,9 @@ 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 { - site_packages: vec![site_packages.clone()], - ..settings.clone() + case.update_search_path_settings(SearchPathConfiguration { + site_packages: Some(SitePackages::Known(vec![site_packages.clone()])), + ..SearchPathConfiguration::default() }) .expect("Search path settings to be valid"); @@ -746,7 +749,7 @@ fn add_search_path() -> anyhow::Result<()> { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); assert!(resolve_module(case.db().upcast(), ModuleName::new_static("a").unwrap()).is_some()); @@ -755,21 +758,19 @@ fn add_search_path() -> anyhow::Result<()> { #[test] fn remove_search_path() -> anyhow::Result<()> { - let mut case = - setup_with_search_paths([("bar.py", "import sub.a")], |root_path, workspace_path| { - SearchPathSettings { - extra_paths: vec![], - src_root: workspace_path.to_path_buf(), - custom_typeshed: None, - site_packages: vec![root_path.join("site_packages")], - } - })?; + let mut case = setup_with_search_paths( + [("bar.py", "import sub.a")], + |root_path, _workspace_path| SearchPathConfiguration { + site_packages: Some(SitePackages::Known(vec![root_path.join("site_packages")])), + ..SearchPathConfiguration::default() + }, + )?; // Remove site packages from the search path settings. let site_packages = case.root_path().join("site_packages"); - case.update_search_path_settings(|settings| SearchPathSettings { - site_packages: vec![], - ..settings.clone() + case.update_search_path_settings(SearchPathConfiguration { + site_packages: None, + ..SearchPathConfiguration::default() }) .expect("Search path settings to be valid"); @@ -782,6 +783,48 @@ fn remove_search_path() -> anyhow::Result<()> { Ok(()) } +#[test] +fn changed_versions_file() -> anyhow::Result<()> { + let mut case = setup_with_search_paths( + |root_path: &SystemPath, workspace_path: &SystemPath| { + std::fs::write(workspace_path.join("bar.py").as_std_path(), "import sub.a")?; + std::fs::create_dir_all(root_path.join("typeshed/stdlib").as_std_path())?; + std::fs::write(root_path.join("typeshed/stdlib/VERSIONS").as_std_path(), "")?; + std::fs::write( + root_path.join("typeshed/stdlib/os.pyi").as_std_path(), + "# not important", + )?; + + Ok(()) + }, + |root_path, _workspace_path| SearchPathConfiguration { + custom_typeshed: Some(root_path.join("typeshed")), + ..SearchPathConfiguration::default() + }, + )?; + + // Unset the custom typeshed directory. + assert_eq!( + resolve_module(case.db(), ModuleName::new("os").unwrap()), + None + ); + + std::fs::write( + case.root_path() + .join("typeshed/stdlib/VERSIONS") + .as_std_path(), + "os: 3.0-", + )?; + + let changes = case.stop_watch(); + + case.apply_changes(changes); + + assert!(resolve_module(case.db(), ModuleName::new("os").unwrap()).is_some()); + + Ok(()) +} + /// Watch a workspace that contains two files where one file is a hardlink to another. /// /// Setup: @@ -828,7 +871,7 @@ fn hard_links_in_workspace() -> anyhow::Result<()> { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); assert_eq!(source_text(case.db(), foo).as_str(), "print('Version 2')"); @@ -899,7 +942,7 @@ fn hard_links_to_target_outside_workspace() -> anyhow::Result<()> { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); assert_eq!(source_text(case.db(), bar).as_str(), "print('Version 2')"); @@ -938,7 +981,7 @@ mod unix { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); assert_eq!( foo.permissions(case.db()), @@ -1023,7 +1066,7 @@ mod unix { let changes = case.take_watch_changes(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); assert_eq!( source_text(case.db(), baz.file()).as_str(), @@ -1036,7 +1079,7 @@ mod unix { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); assert_eq!( source_text(case.db(), baz.file()).as_str(), @@ -1107,7 +1150,7 @@ mod unix { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); // The file watcher is guaranteed to emit one event for the changed file, but it isn't specified // if the event is emitted for the "original" or linked path because both paths are watched. @@ -1176,11 +1219,11 @@ mod unix { Ok(()) }, - |_root, workspace| SearchPathSettings { - extra_paths: vec![], - src_root: workspace.to_path_buf(), - custom_typeshed: None, - site_packages: vec![workspace.join(".venv/lib/python3.12/site-packages")], + |_root, workspace| SearchPathConfiguration { + site_packages: Some(SitePackages::Known(vec![ + workspace.join(".venv/lib/python3.12/site-packages") + ])), + ..SearchPathConfiguration::default() }, )?; @@ -1215,7 +1258,7 @@ mod unix { let changes = case.stop_watch(); - case.db_mut().apply_changes(changes); + case.apply_changes(changes); assert_eq!( source_text(case.db(), baz_original_file).as_str(), diff --git a/crates/red_knot_python_semantic/Cargo.toml b/crates/red_knot_python_semantic/Cargo.toml index d07978271b3a9..bf6daaa8e588c 100644 --- a/crates/red_knot_python_semantic/Cargo.toml +++ b/crates/red_knot_python_semantic/Cargo.toml @@ -26,6 +26,7 @@ countme = { workspace = true } once_cell = { workspace = true } ordermap = { workspace = true } salsa = { workspace = true } +thiserror = { workspace = true } tracing = { workspace = true } rustc-hash = { workspace = true } hashbrown = { workspace = true } diff --git a/crates/red_knot_python_semantic/src/lib.rs b/crates/red_knot_python_semantic/src/lib.rs index 12ea4dd1b9baf..909c2d8de2838 100644 --- a/crates/red_knot_python_semantic/src/lib.rs +++ b/crates/red_knot_python_semantic/src/lib.rs @@ -5,7 +5,7 @@ use rustc_hash::FxHasher; pub use db::Db; pub use module_name::ModuleName; pub use module_resolver::{resolve_module, system_module_search_paths, vendored_typeshed_stubs}; -pub use program::{Program, ProgramSettings, SearchPathSettings}; +pub use program::{Program, ProgramSettings, SearchPathSettings, SitePackages}; pub use python_version::PythonVersion; pub use semantic_model::{HasTy, SemanticModel}; @@ -19,6 +19,7 @@ mod program; mod python_version; pub mod semantic_index; mod semantic_model; +pub(crate) mod site_packages; pub mod types; type FxOrderSet = ordermap::set::OrderSet>; 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 93a34f7b62c65..31d8d3743d123 100644 --- a/crates/red_knot_python_semantic/src/module_resolver/mod.rs +++ b/crates/red_knot_python_semantic/src/module_resolver/mod.rs @@ -13,7 +13,6 @@ use resolver::SearchPathIterator; mod module; mod path; mod resolver; -mod state; mod typeshed; #[cfg(test)] diff --git a/crates/red_knot_python_semantic/src/module_resolver/path.rs b/crates/red_knot_python_semantic/src/module_resolver/path.rs index 546f4b857c0f5..a49d1dc20d70c 100644 --- a/crates/red_knot_python_semantic/src/module_resolver/path.rs +++ b/crates/red_knot_python_semantic/src/module_resolver/path.rs @@ -9,11 +9,11 @@ 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}; +use super::typeshed::{typeshed_versions, TypeshedVersionsParseError, TypeshedVersionsQueryResult}; use crate::db::Db; use crate::module_name::ModuleName; - -use super::state::ResolverState; -use super::typeshed::{TypeshedVersionsParseError, TypeshedVersionsQueryResult}; +use crate::module_resolver::resolver::ResolverContext; +use crate::site_packages::SitePackagesDiscoveryError; /// A path that points to a Python module. /// @@ -60,7 +60,7 @@ impl ModulePath { } #[must_use] - pub(crate) fn is_directory(&self, resolver: &ResolverState) -> bool { + pub(super) fn is_directory(&self, resolver: &ResolverContext) -> bool { let ModulePath { search_path, relative_path, @@ -74,7 +74,7 @@ impl ModulePath { == Err(FileError::IsADirectory) } SearchPathInner::StandardLibraryCustom(stdlib_root) => { - match query_stdlib_version(Some(stdlib_root), relative_path, resolver) { + match query_stdlib_version(relative_path, resolver) { TypeshedVersionsQueryResult::DoesNotExist => false, TypeshedVersionsQueryResult::Exists | TypeshedVersionsQueryResult::MaybeExists => { @@ -84,7 +84,7 @@ impl ModulePath { } } SearchPathInner::StandardLibraryVendored(stdlib_root) => { - match query_stdlib_version(None, relative_path, resolver) { + match query_stdlib_version(relative_path, resolver) { TypeshedVersionsQueryResult::DoesNotExist => false, TypeshedVersionsQueryResult::Exists | TypeshedVersionsQueryResult::MaybeExists => resolver @@ -96,7 +96,7 @@ impl ModulePath { } #[must_use] - pub(crate) fn is_regular_package(&self, resolver: &ResolverState) -> bool { + pub(super) fn is_regular_package(&self, resolver: &ResolverContext) -> bool { let ModulePath { search_path, relative_path, @@ -113,7 +113,7 @@ impl ModulePath { .is_ok() } SearchPathInner::StandardLibraryCustom(search_path) => { - match query_stdlib_version(Some(search_path), relative_path, resolver) { + match query_stdlib_version(relative_path, resolver) { TypeshedVersionsQueryResult::DoesNotExist => false, TypeshedVersionsQueryResult::Exists | TypeshedVersionsQueryResult::MaybeExists => system_path_to_file( @@ -124,7 +124,7 @@ impl ModulePath { } } SearchPathInner::StandardLibraryVendored(search_path) => { - match query_stdlib_version(None, relative_path, resolver) { + match query_stdlib_version(relative_path, resolver) { TypeshedVersionsQueryResult::DoesNotExist => false, TypeshedVersionsQueryResult::Exists | TypeshedVersionsQueryResult::MaybeExists => resolver @@ -136,7 +136,7 @@ impl ModulePath { } #[must_use] - pub(crate) fn to_file(&self, resolver: &ResolverState) -> Option { + pub(super) fn to_file(&self, resolver: &ResolverContext) -> Option { let db = resolver.db.upcast(); let ModulePath { search_path, @@ -150,7 +150,7 @@ impl ModulePath { 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) { + match query_stdlib_version(relative_path, resolver) { TypeshedVersionsQueryResult::DoesNotExist => None, TypeshedVersionsQueryResult::Exists | TypeshedVersionsQueryResult::MaybeExists => { @@ -159,7 +159,7 @@ impl ModulePath { } } SearchPathInner::StandardLibraryVendored(stdlib_root) => { - match query_stdlib_version(None, relative_path, resolver) { + match query_stdlib_version(relative_path, resolver) { TypeshedVersionsQueryResult::DoesNotExist => None, TypeshedVersionsQueryResult::Exists | TypeshedVersionsQueryResult::MaybeExists => { @@ -273,19 +273,15 @@ fn stdlib_path_to_module_name(relative_path: &Utf8Path) -> Option { #[must_use] fn query_stdlib_version( - custom_stdlib_root: Option<&SystemPath>, relative_path: &Utf8Path, - resolver: &ResolverState, + context: &ResolverContext, ) -> TypeshedVersionsQueryResult { let Some(module_name) = stdlib_path_to_module_name(relative_path) else { return TypeshedVersionsQueryResult::DoesNotExist; }; - let ResolverState { - db, - typeshed_versions, - target_version, - } = resolver; - typeshed_versions.query_module(*db, &module_name, custom_stdlib_root, *target_version) + let ResolverContext { db, target_version } = context; + + typeshed_versions(*db).query_module(&module_name, *target_version) } /// Enumeration describing the various ways in which validation of a search path might fail. @@ -293,7 +289,7 @@ fn query_stdlib_version( /// If validation fails for a search path derived from the user settings, /// a message must be displayed to the user, /// as type checking cannot be done reliably in these circumstances. -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug)] pub(crate) enum SearchPathValidationError { /// The path provided by the user was not a directory NotADirectory(SystemPathBuf), @@ -304,18 +300,20 @@ pub(crate) enum SearchPathValidationError { NoStdlibSubdirectory(SystemPathBuf), /// 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. + /// but `stdlib/VERSIONS` could not be read. /// (This is only relevant for stdlib search paths.) - VersionsIsADirectory(SystemPathBuf), + FailedToReadVersionsFile { + path: SystemPathBuf, + error: std::io::Error, + }, /// 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.) VersionsParseError(TypeshedVersionsParseError), + + /// Failed to discover the site-packages for the configured virtual environment. + SitePackagesDiscovery(SitePackagesDiscoveryError), } impl fmt::Display for SearchPathValidationError { @@ -325,9 +323,16 @@ impl fmt::Display for SearchPathValidationError { Self::NoStdlibSubdirectory(path) => { write!(f, "The directory at {path} has no `stdlib/` subdirectory") } - Self::NoVersionsFile(path) => write!(f, "Expected a file at {path}/stdlib/VERSIONS"), - Self::VersionsIsADirectory(path) => write!(f, "{path}/stdlib/VERSIONS is a directory."), + Self::FailedToReadVersionsFile { path, error } => { + write!( + f, + "Failed to read the custom typeshed versions file '{path}': {error}" + ) + } Self::VersionsParseError(underlying_error) => underlying_error.fmt(f), + SearchPathValidationError::SitePackagesDiscovery(error) => { + write!(f, "Failed to discover the site-packages directory: {error}") + } } } } @@ -342,6 +347,18 @@ impl std::error::Error for SearchPathValidationError { } } +impl From for SearchPathValidationError { + fn from(value: TypeshedVersionsParseError) -> Self { + Self::VersionsParseError(value) + } +} + +impl From for SearchPathValidationError { + fn from(value: SitePackagesDiscoveryError) -> Self { + Self::SitePackagesDiscovery(value) + } +} + type SearchPathResult = Result; #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -384,11 +401,10 @@ pub(crate) struct SearchPath(Arc); impl SearchPath { fn directory_path(system: &dyn System, root: SystemPathBuf) -> SearchPathResult { - let canonicalized = system.canonicalize_path(&root).unwrap_or(root); - if system.is_directory(&canonicalized) { - Ok(canonicalized) + if system.is_directory(&root) { + Ok(root) } else { - Err(SearchPathValidationError::NotADirectory(canonicalized)) + Err(SearchPathValidationError::NotADirectory(root)) } } @@ -407,32 +423,22 @@ impl SearchPath { } /// Create a new standard-library search path pointing to a custom directory on disk - pub(crate) fn custom_stdlib(db: &dyn Db, typeshed: SystemPathBuf) -> SearchPathResult { + pub(crate) fn custom_stdlib(db: &dyn Db, typeshed: &SystemPath) -> SearchPathResult { let system = db.system(); - if !system.is_directory(&typeshed) { + if !system.is_directory(typeshed) { return Err(SearchPathValidationError::NotADirectory( typeshed.to_path_buf(), )); } + let stdlib = Self::directory_path(system, typeshed.join("stdlib")).map_err(|err| match err { - SearchPathValidationError::NotADirectory(path) => { - SearchPathValidationError::NoStdlibSubdirectory(path) + SearchPathValidationError::NotADirectory(_) => { + SearchPathValidationError::NoStdlibSubdirectory(typeshed.to_path_buf()) } err => err, })?; - 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) - } - })?; - super::typeshed::parse_typeshed_versions(db, typeshed_versions) - .as_ref() - .map_err(|validation_error| { - SearchPathValidationError::VersionsParseError(validation_error.clone()) - })?; + Ok(Self(Arc::new(SearchPathInner::StandardLibraryCustom( stdlib, )))) @@ -623,11 +629,11 @@ mod tests { use ruff_db::Db; use crate::db::tests::TestDb; - - use super::*; use crate::module_resolver::testing::{FileSpec, MockedTypeshed, TestCase, TestCaseBuilder}; use crate::python_version::PythonVersion; + use super::*; + impl ModulePath { #[must_use] fn join(&self, component: &str) -> ModulePath { @@ -638,15 +644,6 @@ mod tests { } impl SearchPath { - #[must_use] - pub(crate) fn is_stdlib_search_path(&self) -> bool { - matches!( - &*self.0, - SearchPathInner::StandardLibraryCustom(_) - | SearchPathInner::StandardLibraryVendored(_) - ) - } - fn join(&self, component: &str) -> ModulePath { self.to_module_path().join(component) } @@ -661,7 +658,7 @@ mod tests { .build(); assert_eq!( - SearchPath::custom_stdlib(&db, stdlib.parent().unwrap().to_path_buf()) + SearchPath::custom_stdlib(&db, stdlib.parent().unwrap()) .unwrap() .to_module_path() .with_py_extension(), @@ -669,7 +666,7 @@ mod tests { ); assert_eq!( - &SearchPath::custom_stdlib(&db, stdlib.parent().unwrap().to_path_buf()) + &SearchPath::custom_stdlib(&db, stdlib.parent().unwrap()) .unwrap() .join("foo") .with_pyi_extension(), @@ -780,7 +777,7 @@ mod tests { let TestCase { db, stdlib, .. } = TestCaseBuilder::new() .with_custom_typeshed(MockedTypeshed::default()) .build(); - SearchPath::custom_stdlib(&db, stdlib.parent().unwrap().to_path_buf()) + SearchPath::custom_stdlib(&db, stdlib.parent().unwrap()) .unwrap() .to_module_path() .push("bar.py"); @@ -792,7 +789,7 @@ mod tests { let TestCase { db, stdlib, .. } = TestCaseBuilder::new() .with_custom_typeshed(MockedTypeshed::default()) .build(); - SearchPath::custom_stdlib(&db, stdlib.parent().unwrap().to_path_buf()) + SearchPath::custom_stdlib(&db, stdlib.parent().unwrap()) .unwrap() .to_module_path() .push("bar.rs"); @@ -824,7 +821,7 @@ mod tests { .with_custom_typeshed(MockedTypeshed::default()) .build(); - let root = SearchPath::custom_stdlib(&db, stdlib.parent().unwrap().to_path_buf()).unwrap(); + let root = SearchPath::custom_stdlib(&db, stdlib.parent().unwrap()).unwrap(); // Must have a `.pyi` extension or no extension: let bad_absolute_path = SystemPath::new("foo/stdlib/x.py"); @@ -872,8 +869,7 @@ mod tests { .with_custom_typeshed(typeshed) .with_target_version(target_version) .build(); - let stdlib = - SearchPath::custom_stdlib(&db, stdlib.parent().unwrap().to_path_buf()).unwrap(); + let stdlib = SearchPath::custom_stdlib(&db, stdlib.parent().unwrap()).unwrap(); (db, stdlib) } @@ -898,7 +894,7 @@ mod tests { }; let (db, stdlib_path) = py38_typeshed_test_case(TYPESHED); - let resolver = ResolverState::new(&db, PythonVersion::PY38); + let resolver = ResolverContext::new(&db, PythonVersion::PY38); let asyncio_regular_package = stdlib_path.join("asyncio"); assert!(asyncio_regular_package.is_directory(&resolver)); @@ -926,7 +922,7 @@ mod tests { }; let (db, stdlib_path) = py38_typeshed_test_case(TYPESHED); - let resolver = ResolverState::new(&db, PythonVersion::PY38); + let resolver = ResolverContext::new(&db, PythonVersion::PY38); let xml_namespace_package = stdlib_path.join("xml"); assert!(xml_namespace_package.is_directory(&resolver)); @@ -948,7 +944,7 @@ mod tests { }; let (db, stdlib_path) = py38_typeshed_test_case(TYPESHED); - let resolver = ResolverState::new(&db, PythonVersion::PY38); + let resolver = ResolverContext::new(&db, PythonVersion::PY38); let functools_module = stdlib_path.join("functools.pyi"); assert!(functools_module.to_file(&resolver).is_some()); @@ -964,7 +960,7 @@ mod tests { }; let (db, stdlib_path) = py38_typeshed_test_case(TYPESHED); - let resolver = ResolverState::new(&db, PythonVersion::PY38); + let resolver = ResolverContext::new(&db, PythonVersion::PY38); let collections_regular_package = stdlib_path.join("collections"); assert_eq!(collections_regular_package.to_file(&resolver), None); @@ -980,7 +976,7 @@ mod tests { }; let (db, stdlib_path) = py38_typeshed_test_case(TYPESHED); - let resolver = ResolverState::new(&db, PythonVersion::PY38); + let resolver = ResolverContext::new(&db, PythonVersion::PY38); let importlib_namespace_package = stdlib_path.join("importlib"); assert_eq!(importlib_namespace_package.to_file(&resolver), None); @@ -1001,7 +997,7 @@ mod tests { }; let (db, stdlib_path) = py38_typeshed_test_case(TYPESHED); - let resolver = ResolverState::new(&db, PythonVersion::PY38); + let resolver = ResolverContext::new(&db, PythonVersion::PY38); let non_existent = stdlib_path.join("doesnt_even_exist"); assert_eq!(non_existent.to_file(&resolver), None); @@ -1029,7 +1025,7 @@ mod tests { }; let (db, stdlib_path) = py39_typeshed_test_case(TYPESHED); - let resolver = ResolverState::new(&db, PythonVersion::PY39); + let resolver = ResolverContext::new(&db, PythonVersion::PY39); // Since we've set the target version to Py39, // `collections` should now exist as a directory, according to VERSIONS... @@ -1058,7 +1054,7 @@ mod tests { }; let (db, stdlib_path) = py39_typeshed_test_case(TYPESHED); - let resolver = ResolverState::new(&db, PythonVersion::PY39); + let resolver = ResolverContext::new(&db, PythonVersion::PY39); // The `importlib` directory now also exists let importlib_namespace_package = stdlib_path.join("importlib"); @@ -1082,7 +1078,7 @@ mod tests { }; let (db, stdlib_path) = py39_typeshed_test_case(TYPESHED); - let resolver = ResolverState::new(&db, PythonVersion::PY39); + let resolver = ResolverContext::new(&db, PythonVersion::PY39); // The `xml` package no longer exists on py39: let xml_namespace_package = stdlib_path.join("xml"); 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 293c6776e231c..5648dcd24fb80 100644 --- a/crates/red_knot_python_semantic/src/module_resolver/resolver.rs +++ b/crates/red_knot_python_semantic/src/module_resolver/resolver.rs @@ -1,19 +1,19 @@ +use rustc_hash::{FxBuildHasher, FxHashSet}; use std::borrow::Cow; use std::iter::FusedIterator; - -use rustc_hash::{FxBuildHasher, FxHashSet}; +use std::ops::Deref; 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 ruff_db::system::{DirectoryEntry, System, SystemPath, SystemPathBuf}; +use ruff_db::vendored::{VendoredFileSystem, VendoredPath}; 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::module_resolver::typeshed::{vendored_typeshed_versions, TypeshedVersions}; +use crate::site_packages::VirtualEnvironment; +use crate::{Program, PythonVersion, SearchPathSettings, SitePackages}; /// Resolves a module name to a module. pub fn resolve_module(db: &dyn Db, module_name: ModuleName) -> Option { @@ -122,7 +122,7 @@ pub(crate) fn search_paths(db: &dyn Db) -> SearchPathIterator { Program::get(db).search_paths(db).iter(db) } -#[derive(Debug, PartialEq, Eq, Default)] +#[derive(Debug, PartialEq, Eq)] 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. @@ -135,6 +135,8 @@ pub(crate) struct SearchPaths { /// in terms of module-resolution priority until we've discovered the editable installs /// for the first `site-packages` path site_packages: Vec, + + typeshed_versions: ResolvedTypeshedVersions, } impl SearchPaths { @@ -146,8 +148,14 @@ impl SearchPaths { /// [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, + settings: &SearchPathSettings, ) -> Result { + fn canonicalize(path: &SystemPath, system: &dyn System) -> SystemPathBuf { + system + .canonicalize_path(path) + .unwrap_or_else(|_| path.to_path_buf()) + } + let SearchPathSettings { extra_paths, src_root, @@ -161,45 +169,65 @@ impl SearchPaths { let mut static_paths = vec![]; for path in extra_paths { - tracing::debug!("Adding static extra search-path '{path}'"); + let path = canonicalize(path, system); + files.try_add_root(db.upcast(), &path, FileRootKind::LibrarySearchPath); + tracing::debug!("Adding extra search-path '{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); + static_paths.push(SearchPath::extra(system, path)?); } tracing::debug!("Adding first-party search path '{src_root}'"); - static_paths.push(SearchPath::first_party(system, src_root)?); + static_paths.push(SearchPath::first_party(system, src_root.to_path_buf())?); - static_paths.push(if let Some(custom_typeshed) = custom_typeshed { + let (typeshed_versions, stdlib_path) = if let Some(custom_typeshed) = custom_typeshed { + let custom_typeshed = canonicalize(custom_typeshed, system); tracing::debug!("Adding custom-stdlib search path '{custom_typeshed}'"); - let search_path = SearchPath::custom_stdlib(db, custom_typeshed)?; files.try_add_root( db.upcast(), - search_path.as_system_path().unwrap(), + &custom_typeshed, FileRootKind::LibrarySearchPath, ); - search_path + + let versions_path = custom_typeshed.join("stdlib/VERSIONS"); + + let versions_content = system.read_to_string(&versions_path).map_err(|error| { + SearchPathValidationError::FailedToReadVersionsFile { + path: versions_path, + error, + } + })?; + + let parsed: TypeshedVersions = versions_content.parse()?; + + let search_path = SearchPath::custom_stdlib(db, &custom_typeshed)?; + + (ResolvedTypeshedVersions::Custom(parsed), search_path) } else { - SearchPath::vendored_stdlib() - }); + tracing::debug!("Using vendored stdlib"); + ( + ResolvedTypeshedVersions::Vendored(vendored_typeshed_versions()), + SearchPath::vendored_stdlib(), + ) + }; + + static_paths.push(stdlib_path); + + let site_packages_paths = match site_packages_paths { + SitePackages::Derived { venv_path } => VirtualEnvironment::new(venv_path, system) + .and_then(|venv| venv.site_packages_directories(system))?, + SitePackages::Known(paths) => paths + .iter() + .map(|path| canonicalize(path, system)) + .collect(), + }; let mut site_packages: Vec<_> = Vec::with_capacity(site_packages_paths.len()); for path in site_packages_paths { tracing::debug!("Adding site-packages search 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); + files.try_add_root(db.upcast(), &path, FileRootKind::LibrarySearchPath); + site_packages.push(SearchPath::site_packages(system, path)?); } // TODO vendor typeshed's third-party stubs as well as the stdlib and fallback to them as a final step @@ -224,16 +252,48 @@ impl SearchPaths { Ok(SearchPaths { static_paths, site_packages, + typeshed_versions, }) } - pub(crate) fn iter<'a>(&'a self, db: &'a dyn Db) -> SearchPathIterator<'a> { + pub(super) fn iter<'a>(&'a self, db: &'a dyn Db) -> SearchPathIterator<'a> { SearchPathIterator { db, static_paths: self.static_paths.iter(), dynamic_paths: None, } } + + pub(crate) fn custom_stdlib(&self) -> Option<&SystemPath> { + self.static_paths.iter().find_map(|search_path| { + if search_path.is_standard_library() { + search_path.as_system_path() + } else { + None + } + }) + } + + pub(super) fn typeshed_versions(&self) -> &TypeshedVersions { + &self.typeshed_versions + } +} + +#[derive(Debug, PartialEq, Eq)] +enum ResolvedTypeshedVersions { + Vendored(&'static TypeshedVersions), + Custom(TypeshedVersions), +} + +impl Deref for ResolvedTypeshedVersions { + type Target = TypeshedVersions; + + fn deref(&self) -> &Self::Target { + match self { + ResolvedTypeshedVersions::Vendored(versions) => versions, + ResolvedTypeshedVersions::Custom(versions) => versions, + } + } } /// Collect all dynamic search paths. For each `site-packages` path: @@ -251,6 +311,7 @@ pub(crate) fn dynamic_resolution_paths(db: &dyn Db) -> Vec { let SearchPaths { static_paths, site_packages, + typeshed_versions: _, } = Program::get(db).search_paths(db); let mut dynamic_paths = Vec::new(); @@ -315,12 +376,16 @@ pub(crate) fn dynamic_resolution_paths(db: &dyn Db) -> Vec { let installations = all_pth_files.iter().flat_map(PthFile::items); for installation in installations { + let installation = system + .canonicalize_path(&installation) + .unwrap_or(installation); + if existing_paths.insert(Cow::Owned(installation.clone())) { - match SearchPath::editable(system, installation) { + match SearchPath::editable(system, installation.clone()) { Ok(search_path) => { tracing::debug!( "Adding editable installation to module resolution path {path}", - path = search_path.as_system_path().unwrap() + path = installation ); dynamic_paths.push(search_path); } @@ -482,7 +547,7 @@ struct ModuleNameIngredient<'db> { fn resolve_name(db: &dyn Db, name: &ModuleName) -> Option<(SearchPath, File, ModuleKind)> { let program = Program::get(db); let target_version = program.target_version(db); - let resolver_state = ResolverState::new(db, target_version); + let resolver_state = ResolverContext::new(db, target_version); let is_builtin_module = ruff_python_stdlib::sys::is_builtin_module(target_version.minor, name.as_str()); @@ -545,7 +610,7 @@ fn resolve_name(db: &dyn Db, name: &ModuleName) -> Option<(SearchPath, File, Mod fn resolve_package<'a, 'db, I>( module_search_path: &SearchPath, components: I, - resolver_state: &ResolverState<'db>, + resolver_state: &ResolverContext<'db>, ) -> Result where I: Iterator, @@ -627,6 +692,21 @@ impl PackageKind { } } +pub(super) struct ResolverContext<'db> { + pub(super) db: &'db dyn Db, + pub(super) target_version: PythonVersion, +} + +impl<'db> ResolverContext<'db> { + pub(super) fn new(db: &'db dyn Db, target_version: PythonVersion) -> Self { + Self { db, target_version } + } + + pub(super) fn vendored(&self) -> &VendoredFileSystem { + self.db.vendored() + } +} + #[cfg(test)] mod tests { use ruff_db::files::{system_path_to_file, File, FilePath}; @@ -781,7 +861,7 @@ mod tests { "Search path for {module_name} was unexpectedly {search_path:?}" ); assert!( - search_path.is_stdlib_search_path(), + search_path.is_standard_library(), "Expected a stdlib search path, but got {search_path:?}" ); } @@ -877,7 +957,7 @@ mod tests { "Search path for {module_name} was unexpectedly {search_path:?}" ); assert!( - search_path.is_stdlib_search_path(), + search_path.is_standard_library(), "Expected a stdlib search path, but got {search_path:?}" ); } @@ -1194,13 +1274,13 @@ mod tests { Program::from_settings( &db, - ProgramSettings { + &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], + site_packages: SitePackages::Known(vec![site_packages]), }, }, ) @@ -1699,13 +1779,16 @@ not_a_directory Program::from_settings( &db, - ProgramSettings { + &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], + site_packages: SitePackages::Known(vec![ + venv_site_packages, + system_site_packages, + ]), }, }, ) diff --git a/crates/red_knot_python_semantic/src/module_resolver/state.rs b/crates/red_knot_python_semantic/src/module_resolver/state.rs deleted file mode 100644 index cb56e5c8463fd..0000000000000 --- a/crates/red_knot_python_semantic/src/module_resolver/state.rs +++ /dev/null @@ -1,25 +0,0 @@ -use ruff_db::vendored::VendoredFileSystem; - -use super::typeshed::LazyTypeshedVersions; -use crate::db::Db; -use crate::python_version::PythonVersion; - -pub(crate) struct ResolverState<'db> { - pub(crate) db: &'db dyn Db, - pub(crate) typeshed_versions: LazyTypeshedVersions<'db>, - pub(crate) target_version: PythonVersion, -} - -impl<'db> ResolverState<'db> { - pub(crate) fn new(db: &'db dyn Db, target_version: PythonVersion) -> Self { - Self { - db, - typeshed_versions: LazyTypeshedVersions::new(), - target_version, - } - } - - pub(crate) fn vendored(&self) -> &VendoredFileSystem { - self.db.vendored() - } -} 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 87a05001113c7..0cf486ab6adbe 100644 --- a/crates/red_knot_python_semantic/src/module_resolver/testing.rs +++ b/crates/red_knot_python_semantic/src/module_resolver/testing.rs @@ -4,7 +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; +use crate::{ProgramSettings, SitePackages}; /// A test case for the module resolver. /// @@ -179,6 +179,7 @@ impl TestCaseBuilder { first_party_files, site_packages_files, } = self; + TestCaseBuilder { typeshed_option: typeshed, target_version, @@ -195,6 +196,7 @@ impl TestCaseBuilder { site_packages, target_version, } = self.with_custom_typeshed(MockedTypeshed::default()).build(); + TestCase { db, src, @@ -223,13 +225,13 @@ impl TestCaseBuilder { Program::from_settings( &db, - ProgramSettings { + &ProgramSettings { target_version, search_paths: SearchPathSettings { extra_paths: vec![], src_root: src.clone(), custom_typeshed: Some(typeshed.clone()), - site_packages: vec![site_packages.clone()], + site_packages: SitePackages::Known(vec![site_packages.clone()]), }, }, ) @@ -279,13 +281,11 @@ impl TestCaseBuilder { Program::from_settings( &db, - ProgramSettings { + &ProgramSettings { target_version, search_paths: SearchPathSettings { - extra_paths: vec![], - src_root: src.clone(), - custom_typeshed: None, - site_packages: vec![site_packages.clone()], + site_packages: SitePackages::Known(vec![site_packages.clone()]), + ..SearchPathSettings::new(src.clone()) }, }, ) diff --git a/crates/red_knot_python_semantic/src/module_resolver/typeshed/mod.rs b/crates/red_knot_python_semantic/src/module_resolver/typeshed/mod.rs index 97cac75fa62e0..fe6b08f5766c9 100644 --- a/crates/red_knot_python_semantic/src/module_resolver/typeshed/mod.rs +++ b/crates/red_knot_python_semantic/src/module_resolver/typeshed/mod.rs @@ -1,6 +1,6 @@ pub use self::vendored::vendored_typeshed_stubs; pub(super) use self::versions::{ - parse_typeshed_versions, LazyTypeshedVersions, TypeshedVersionsParseError, + typeshed_versions, vendored_typeshed_versions, TypeshedVersions, TypeshedVersionsParseError, TypeshedVersionsQueryResult, }; diff --git a/crates/red_knot_python_semantic/src/module_resolver/typeshed/versions.rs b/crates/red_knot_python_semantic/src/module_resolver/typeshed/versions.rs index de53f0054809f..f4851858a91d0 100644 --- a/crates/red_knot_python_semantic/src/module_resolver/typeshed/versions.rs +++ b/crates/red_knot_python_semantic/src/module_resolver/typeshed/versions.rs @@ -1,4 +1,3 @@ -use std::cell::OnceCell; use std::collections::BTreeMap; use std::fmt; use std::num::{NonZeroU16, NonZeroUsize}; @@ -6,78 +5,12 @@ use std::ops::{RangeFrom, RangeInclusive}; use std::str::FromStr; use once_cell::sync::Lazy; -use ruff_db::system::SystemPath; use rustc_hash::FxHashMap; -use ruff_db::files::{system_path_to_file, File}; - use super::vendored::vendored_typeshed_stubs; use crate::db::Db; use crate::module_name::ModuleName; -use crate::python_version::PythonVersion; - -#[derive(Debug)] -pub(crate) struct LazyTypeshedVersions<'db>(OnceCell<&'db TypeshedVersions>); - -impl<'db> LazyTypeshedVersions<'db> { - #[must_use] - pub(crate) fn new() -> Self { - Self(OnceCell::new()) - } - - /// Query whether a module exists at runtime in the stdlib on a certain Python version. - /// - /// Simply probing whether a file exists in typeshed is insufficient for this question, - /// as a module in the stdlib may have been added in Python 3.10, but the typeshed stub - /// will still be available (either in a custom typeshed dir or in our vendored copy) - /// even if the user specified Python 3.8 as the target version. - /// - /// For top-level modules and packages, the VERSIONS file can always provide an unambiguous answer - /// as to whether the module exists on the specified target version. However, VERSIONS does not - /// provide comprehensive information on all submodules, meaning that this method sometimes - /// returns [`TypeshedVersionsQueryResult::MaybeExists`]. - /// See [`TypeshedVersionsQueryResult`] for more details. - #[must_use] - pub(crate) fn query_module( - &self, - db: &'db dyn Db, - module: &ModuleName, - stdlib_root: Option<&SystemPath>, - target_version: PythonVersion, - ) -> TypeshedVersionsQueryResult { - let versions = self.0.get_or_init(|| { - let versions_path = if let Some(system_path) = stdlib_root { - system_path.join("VERSIONS") - } else { - return &VENDORED_VERSIONS; - }; - 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}" - ) - }; - // TODO(Alex/Micha): If VERSIONS is invalid, - // this should invalidate not just the specific module resolution we're currently attempting, - // but all type inference that depends on any standard-library types. - // Unwrapping here is not correct... - parse_typeshed_versions(db, versions_file).as_ref().unwrap() - }); - versions.query_module(module, target_version) - } -} - -#[salsa::tracked(return_ref)] -pub(crate) fn parse_typeshed_versions( - db: &dyn Db, - versions_file: File, -) -> Result { - // TODO: Handle IO errors - let file_content = versions_file - .read_to_string(db.upcast()) - .unwrap_or_default(); - file_content.parse() -} +use crate::{Program, PythonVersion}; static VENDORED_VERSIONS: Lazy = Lazy::new(|| { TypeshedVersions::from_str( @@ -88,6 +21,14 @@ static VENDORED_VERSIONS: Lazy = Lazy::new(|| { .unwrap() }); +pub(crate) fn vendored_typeshed_versions() -> &'static TypeshedVersions { + &VENDORED_VERSIONS +} + +pub(crate) fn typeshed_versions(db: &dyn Db) -> &TypeshedVersions { + Program::get(db).search_paths(db).typeshed_versions() +} + #[derive(Debug, PartialEq, Eq, Clone)] pub(crate) struct TypeshedVersionsParseError { line_number: Option, @@ -174,7 +115,7 @@ impl TypeshedVersions { } #[must_use] - fn query_module( + pub(in crate::module_resolver) fn query_module( &self, module: &ModuleName, target_version: PythonVersion, @@ -204,7 +145,7 @@ impl TypeshedVersions { } } -/// Possible answers [`LazyTypeshedVersions::query_module()`] could give to the question: +/// Possible answers [`TypeshedVersions::query_module()`] could give to the question: /// "Does this module exist in the stdlib at runtime on a certain target version?" #[derive(Debug, Copy, PartialEq, Eq, Clone, Hash)] pub(crate) enum TypeshedVersionsQueryResult { diff --git a/crates/red_knot_python_semantic/src/program.rs b/crates/red_knot_python_semantic/src/program.rs index 5362dc6a49238..7671dabb9590c 100644 --- a/crates/red_knot_python_semantic/src/program.rs +++ b/crates/red_knot_python_semantic/src/program.rs @@ -3,7 +3,7 @@ use anyhow::Context; use salsa::Durability; use salsa::Setter; -use ruff_db::system::SystemPathBuf; +use ruff_db::system::{SystemPath, SystemPathBuf}; use crate::module_resolver::SearchPaths; use crate::Db; @@ -12,13 +12,12 @@ use crate::Db; pub struct Program { pub target_version: PythonVersion, - #[default] #[return_ref] pub(crate) search_paths: SearchPaths, } impl Program { - pub fn from_settings(db: &dyn Db, settings: ProgramSettings) -> anyhow::Result { + pub fn from_settings(db: &dyn Db, settings: &ProgramSettings) -> anyhow::Result { let ProgramSettings { target_version, search_paths, @@ -29,16 +28,15 @@ impl Program { let search_paths = SearchPaths::from_settings(db, search_paths) .with_context(|| "Invalid search path settings")?; - Ok(Program::builder(settings.target_version) + Ok(Program::builder(settings.target_version, search_paths) .durability(Durability::HIGH) - .search_paths(search_paths) .new(db)) } pub fn update_search_paths( - &self, + self, db: &mut dyn Db, - search_path_settings: SearchPathSettings, + search_path_settings: &SearchPathSettings, ) -> anyhow::Result<()> { let search_paths = SearchPaths::from_settings(db, search_path_settings)?; @@ -49,16 +47,20 @@ impl Program { Ok(()) } + + pub fn custom_stdlib_search_path(self, db: &dyn Db) -> Option<&SystemPath> { + self.search_paths(db).custom_stdlib() + } } -#[derive(Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq)] pub struct ProgramSettings { pub target_version: PythonVersion, pub search_paths: SearchPathSettings, } /// Configures the search paths for module resolution. -#[derive(Eq, PartialEq, Debug, Clone, Default)] +#[derive(Eq, PartialEq, Debug, Clone)] pub struct SearchPathSettings { /// List of user-provided paths that should take first priority in the module resolution. /// Examples in other type checkers are mypy's MYPYPATH environment variable, @@ -74,5 +76,25 @@ pub struct SearchPathSettings { pub custom_typeshed: Option, /// The path to the user's `site-packages` directory, where third-party packages from ``PyPI`` are installed. - pub site_packages: Vec, + pub site_packages: SitePackages, +} + +impl SearchPathSettings { + pub fn new(src_root: SystemPathBuf) -> Self { + Self { + src_root, + extra_paths: vec![], + custom_typeshed: None, + site_packages: SitePackages::Known(vec![]), + } + } +} + +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum SitePackages { + Derived { + venv_path: SystemPathBuf, + }, + /// Resolved site packages paths + Known(Vec), } diff --git a/crates/red_knot_python_semantic/src/semantic_model.rs b/crates/red_knot_python_semantic/src/semantic_model.rs index da451f60e7704..e7320547821b6 100644 --- a/crates/red_knot_python_semantic/src/semantic_model.rs +++ b/crates/red_knot_python_semantic/src/semantic_model.rs @@ -184,14 +184,9 @@ mod tests { Program::from_settings( &db, - ProgramSettings { + &ProgramSettings { target_version: PythonVersion::default(), - search_paths: SearchPathSettings { - extra_paths: vec![], - src_root: SystemPathBuf::from("/src"), - site_packages: vec![], - custom_typeshed: None, - }, + search_paths: SearchPathSettings::new(SystemPathBuf::from("/src")), }, )?; diff --git a/crates/red_knot_workspace/src/site_packages.rs b/crates/red_knot_python_semantic/src/site_packages.rs similarity index 99% rename from crates/red_knot_workspace/src/site_packages.rs rename to crates/red_knot_python_semantic/src/site_packages.rs index ac78d327fddc3..dc7205d4da514 100644 --- a/crates/red_knot_workspace/src/site_packages.rs +++ b/crates/red_knot_python_semantic/src/site_packages.rs @@ -13,9 +13,10 @@ use std::io; use std::num::NonZeroUsize; use std::ops::Deref; -use red_knot_python_semantic::PythonVersion; use ruff_db::system::{System, SystemPath, SystemPathBuf}; +use crate::PythonVersion; + type SitePackagesDiscoveryResult = Result; /// Abstraction for a Python virtual environment. @@ -24,7 +25,7 @@ type SitePackagesDiscoveryResult = Result; /// The format of this file is not defined anywhere, and exactly which keys are present /// depends on the tool that was used to create the virtual environment. #[derive(Debug)] -pub struct VirtualEnvironment { +pub(crate) struct VirtualEnvironment { venv_path: SysPrefixPath, base_executable_home_path: PythonHomePath, include_system_site_packages: bool, @@ -41,7 +42,7 @@ pub struct VirtualEnvironment { } impl VirtualEnvironment { - pub fn new( + pub(crate) fn new( path: impl AsRef, system: &dyn System, ) -> SitePackagesDiscoveryResult { @@ -157,7 +158,7 @@ impl VirtualEnvironment { /// Return a list of `site-packages` directories that are available from this virtual environment /// /// See the documentation for `site_packages_dir_from_sys_prefix` for more details. - pub fn site_packages_directories( + pub(crate) fn site_packages_directories( &self, system: &dyn System, ) -> SitePackagesDiscoveryResult> { @@ -204,7 +205,7 @@ System site-packages will not be used for module resolution.", } #[derive(Debug, thiserror::Error)] -pub enum SitePackagesDiscoveryError { +pub(crate) enum SitePackagesDiscoveryError { #[error("Invalid --venv-path argument: {0} could not be canonicalized")] VenvDirCanonicalizationError(SystemPathBuf, #[source] io::Error), #[error("Invalid --venv-path argument: {0} does not point to a directory on disk")] @@ -221,7 +222,7 @@ pub enum SitePackagesDiscoveryError { /// The various ways in which parsing a `pyvenv.cfg` file could fail #[derive(Debug)] -pub enum PyvenvCfgParseErrorKind { +pub(crate) enum PyvenvCfgParseErrorKind { TooManyEquals { line_number: NonZeroUsize }, MalformedKeyValuePair { line_number: NonZeroUsize }, NoHomeKey, @@ -370,7 +371,7 @@ fn site_packages_directory_from_sys_prefix( /// /// [`sys.prefix`]: https://docs.python.org/3/library/sys.html#sys.prefix #[derive(Debug, PartialEq, Eq, Clone)] -pub struct SysPrefixPath(SystemPathBuf); +pub(crate) struct SysPrefixPath(SystemPathBuf); impl SysPrefixPath { fn new( diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 50109fd19488c..173c957d1a28e 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -392,14 +392,9 @@ mod tests { Program::from_settings( &db, - ProgramSettings { + &ProgramSettings { target_version: PythonVersion::default(), - search_paths: SearchPathSettings { - extra_paths: Vec::new(), - src_root: SystemPathBuf::from("/src"), - site_packages: vec![], - custom_typeshed: None, - }, + search_paths: SearchPathSettings::new(SystemPathBuf::from("/src")), }, ) .expect("Valid search path settings"); diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index 138bc73372176..9b183727e7920 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -1866,14 +1866,9 @@ mod tests { Program::from_settings( &db, - ProgramSettings { + &ProgramSettings { target_version: PythonVersion::default(), - search_paths: SearchPathSettings { - extra_paths: Vec::new(), - src_root, - site_packages: vec![], - custom_typeshed: None, - }, + search_paths: SearchPathSettings::new(src_root), }, ) .expect("Valid search path settings"); @@ -1893,13 +1888,11 @@ mod tests { Program::from_settings( &db, - ProgramSettings { + &ProgramSettings { target_version: PythonVersion::default(), search_paths: SearchPathSettings { - extra_paths: Vec::new(), - src_root, - site_packages: vec![], custom_typeshed: Some(SystemPathBuf::from(typeshed)), + ..SearchPathSettings::new(src_root) }, }, ) diff --git a/crates/red_knot_server/Cargo.toml b/crates/red_knot_server/Cargo.toml index 71a895632b309..a2bfef6c3c8e6 100644 --- a/crates/red_knot_server/Cargo.toml +++ b/crates/red_knot_server/Cargo.toml @@ -11,7 +11,6 @@ repository = { workspace = true } license = { workspace = true } [dependencies] -red_knot_python_semantic = { workspace = true } red_knot_workspace = { workspace = true } ruff_db = { workspace = true } ruff_notebook = { workspace = true } diff --git a/crates/red_knot_server/src/session.rs b/crates/red_knot_server/src/session.rs index fe2c09a33bc64..89f813588a401 100644 --- a/crates/red_knot_server/src/session.rs +++ b/crates/red_knot_server/src/session.rs @@ -8,7 +8,6 @@ use std::sync::Arc; use anyhow::anyhow; use lsp_types::{ClientCapabilities, Url}; -use red_knot_python_semantic::{ProgramSettings, PythonVersion, SearchPathSettings}; use red_knot_workspace::db::RootDatabase; use red_knot_workspace::workspace::WorkspaceMetadata; use ruff_db::files::{system_path_to_file, File}; @@ -67,19 +66,10 @@ impl Session { .ok_or_else(|| anyhow!("Workspace path is not a valid UTF-8 path: {:?}", path))?; let system = LSPSystem::new(index.clone()); - let metadata = WorkspaceMetadata::from_path(system_path, &system)?; // TODO(dhruvmanila): Get the values from the client settings - let program_settings = ProgramSettings { - target_version: PythonVersion::default(), - search_paths: SearchPathSettings { - extra_paths: vec![], - src_root: system_path.to_path_buf(), - site_packages: vec![], - custom_typeshed: None, - }, - }; + let metadata = WorkspaceMetadata::from_path(system_path, &system, None)?; // TODO(micha): Handle the case where the program settings are incorrect more gracefully. - workspaces.insert(path, RootDatabase::new(metadata, program_settings, system)?); + workspaces.insert(path, RootDatabase::new(metadata, system)?); } Ok(Self { diff --git a/crates/red_knot_wasm/src/lib.rs b/crates/red_knot_wasm/src/lib.rs index b2ab78c4f4093..cb2e531532253 100644 --- a/crates/red_knot_wasm/src/lib.rs +++ b/crates/red_knot_wasm/src/lib.rs @@ -3,8 +3,8 @@ use std::any::Any; use js_sys::Error; use wasm_bindgen::prelude::*; -use red_knot_python_semantic::{ProgramSettings, SearchPathSettings}; use red_knot_workspace::db::RootDatabase; +use red_knot_workspace::workspace::settings::Configuration; use red_knot_workspace::workspace::WorkspaceMetadata; use ruff_db::files::{system_path_to_file, File}; use ruff_db::system::walk_directory::WalkDirectoryBuilder; @@ -41,16 +41,17 @@ impl Workspace { #[wasm_bindgen(constructor)] pub fn new(root: &str, settings: &Settings) -> Result { let system = WasmSystem::new(SystemPath::new(root)); - let workspace = - WorkspaceMetadata::from_path(SystemPath::new(root), &system).map_err(into_error)?; - - let program_settings = ProgramSettings { - target_version: settings.target_version.into(), - search_paths: SearchPathSettings::default(), - }; - - let db = - RootDatabase::new(workspace, program_settings, system.clone()).map_err(into_error)?; + let workspace = WorkspaceMetadata::from_path( + SystemPath::new(root), + &system, + Some(Configuration { + target_version: Some(settings.target_version.into()), + ..Configuration::default() + }), + ) + .map_err(into_error)?; + + let db = RootDatabase::new(workspace, system.clone()).map_err(into_error)?; Ok(Self { db, system }) } diff --git a/crates/red_knot_workspace/Cargo.toml b/crates/red_knot_workspace/Cargo.toml index 8605279b22d0c..4c05b124708db 100644 --- a/crates/red_knot_workspace/Cargo.toml +++ b/crates/red_knot_workspace/Cargo.toml @@ -24,7 +24,6 @@ crossbeam = { workspace = true } notify = { workspace = true } rustc-hash = { workspace = true } salsa = { workspace = true } -thiserror = { workspace = true } tracing = { workspace = true } [dev-dependencies] diff --git a/crates/red_knot_workspace/src/db.rs b/crates/red_knot_workspace/src/db.rs index f172ee0f1a19c..68fd3ca9c6691 100644 --- a/crates/red_knot_workspace/src/db.rs +++ b/crates/red_knot_workspace/src/db.rs @@ -1,15 +1,14 @@ use std::panic::RefUnwindSafe; use std::sync::Arc; -use red_knot_python_semantic::{ - vendored_typeshed_stubs, Db as SemanticDb, Program, ProgramSettings, -}; +use salsa::plumbing::ZalsaDatabase; +use salsa::{Cancelled, Event}; + +use red_knot_python_semantic::{vendored_typeshed_stubs, Db as SemanticDb, Program}; use ruff_db::files::{File, Files}; use ruff_db::system::System; use ruff_db::vendored::VendoredFileSystem; use ruff_db::{Db as SourceDb, Upcast}; -use salsa::plumbing::ZalsaDatabase; -use salsa::{Cancelled, Event}; use crate::workspace::{check_file, Workspace, WorkspaceMetadata}; @@ -27,11 +26,7 @@ pub struct RootDatabase { } impl RootDatabase { - pub fn new( - workspace: WorkspaceMetadata, - settings: ProgramSettings, - system: S, - ) -> anyhow::Result + pub fn new(workspace: WorkspaceMetadata, system: S) -> anyhow::Result where S: System + 'static + Send + Sync + RefUnwindSafe, { @@ -42,11 +37,11 @@ impl RootDatabase { system: Arc::new(system), }; - let workspace = Workspace::from_metadata(&db, workspace); // Initialize the `Program` singleton - Program::from_settings(&db, settings)?; + Program::from_settings(&db, workspace.settings().program())?; + + db.workspace = Some(Workspace::from_metadata(&db, workspace)); - db.workspace = Some(workspace); Ok(db) } @@ -160,9 +155,10 @@ impl Db for RootDatabase {} #[cfg(test)] pub(crate) mod tests { - use salsa::Event; use std::sync::Arc; + use salsa::Event; + use red_knot_python_semantic::{vendored_typeshed_stubs, Db as SemanticDb}; use ruff_db::files::Files; use ruff_db::system::{DbWithTestSystem, System, TestSystem}; diff --git a/crates/red_knot_workspace/src/db/changes.rs b/crates/red_knot_workspace/src/db/changes.rs index 8b50f2548ef24..965b7d9f0a098 100644 --- a/crates/red_knot_workspace/src/db/changes.rs +++ b/crates/red_knot_workspace/src/db/changes.rs @@ -1,22 +1,33 @@ -use rustc_hash::FxHashSet; - +use red_knot_python_semantic::Program; use ruff_db::files::{system_path_to_file, File, Files}; use ruff_db::system::walk_directory::WalkState; use ruff_db::system::SystemPath; use ruff_db::Db; +use rustc_hash::FxHashSet; use crate::db::RootDatabase; use crate::watch; use crate::watch::{CreatedKind, DeletedKind}; +use crate::workspace::settings::Configuration; use crate::workspace::WorkspaceMetadata; impl RootDatabase { - #[tracing::instrument(level = "debug", skip(self, changes))] - pub fn apply_changes(&mut self, changes: Vec) { + #[tracing::instrument(level = "debug", skip(self, changes, base_configuration))] + pub fn apply_changes( + &mut self, + changes: Vec, + base_configuration: Option<&Configuration>, + ) { let workspace = self.workspace(); let workspace_path = workspace.root(self).to_path_buf(); + let program = Program::get(self); + let custom_stdlib_versions_path = program + .custom_stdlib_search_path(self) + .map(|path| path.join("VERSIONS")); let mut workspace_change = false; + // Changes to a custom stdlib path's VERSIONS + let mut custom_stdlib_change = false; // Packages that need reloading let mut changed_packages = FxHashSet::default(); // Paths that were added @@ -54,6 +65,10 @@ impl RootDatabase { continue; } + + if Some(path) == custom_stdlib_versions_path.as_deref() { + custom_stdlib_change = true; + } } match change { @@ -100,7 +115,13 @@ impl RootDatabase { } else { sync_recursively(self, &path); - // TODO: Remove after converting `package.files()` to a salsa query. + if custom_stdlib_versions_path + .as_ref() + .is_some_and(|versions_path| versions_path.starts_with(&path)) + { + custom_stdlib_change = true; + } + if let Some(package) = workspace.package(self, &path) { changed_packages.insert(package); } else { @@ -118,7 +139,11 @@ impl RootDatabase { } if workspace_change { - match WorkspaceMetadata::from_path(&workspace_path, self.system()) { + match WorkspaceMetadata::from_path( + &workspace_path, + self.system(), + base_configuration.cloned(), + ) { Ok(metadata) => { tracing::debug!("Reloading workspace after structural change."); // TODO: Handle changes in the program settings. @@ -130,6 +155,11 @@ impl RootDatabase { } return; + } else if custom_stdlib_change { + let search_paths = workspace.search_path_settings(self).clone(); + if let Err(error) = program.update_search_paths(self, &search_paths) { + tracing::error!("Failed to set the new search paths: {error}"); + } } let mut added_paths = added_paths.into_iter().filter(|path| { diff --git a/crates/red_knot_workspace/src/lib.rs b/crates/red_knot_workspace/src/lib.rs index 45a27012fca5e..f0b3f62a9802f 100644 --- a/crates/red_knot_workspace/src/lib.rs +++ b/crates/red_knot_workspace/src/lib.rs @@ -1,5 +1,4 @@ pub mod db; pub mod lint; -pub mod site_packages; pub mod watch; pub mod workspace; diff --git a/crates/red_knot_workspace/src/lint.rs b/crates/red_knot_workspace/src/lint.rs index 8fee8dd96865b..854e6210c9257 100644 --- a/crates/red_knot_workspace/src/lint.rs +++ b/crates/red_knot_workspace/src/lint.rs @@ -286,14 +286,9 @@ mod tests { Program::from_settings( &db, - ProgramSettings { + &ProgramSettings { target_version: PythonVersion::default(), - search_paths: SearchPathSettings { - extra_paths: Vec::new(), - src_root, - site_packages: vec![], - custom_typeshed: None, - }, + search_paths: SearchPathSettings::new(src_root), }, ) .expect("Valid program settings"); diff --git a/crates/red_knot_workspace/src/workspace.rs b/crates/red_knot_workspace/src/workspace.rs index 2d96efbc56467..22145e9e89eb1 100644 --- a/crates/red_knot_workspace/src/workspace.rs +++ b/crates/red_knot_workspace/src/workspace.rs @@ -5,6 +5,7 @@ use salsa::{Durability, Setter as _}; pub use metadata::{PackageMetadata, WorkspaceMetadata}; use red_knot_python_semantic::types::check_types; +use red_knot_python_semantic::SearchPathSettings; use ruff_db::source::{line_index, source_text, SourceDiagnostic}; use ruff_db::{ files::{system_path_to_file, File}, @@ -21,6 +22,7 @@ use crate::{ mod files; mod metadata; +pub mod settings; /// The project workspace as a Salsa ingredient. /// @@ -81,6 +83,10 @@ pub struct Workspace { /// The (first-party) packages in this workspace. #[return_ref] package_tree: BTreeMap, + + /// The unresolved search path configuration. + #[return_ref] + pub search_path_settings: SearchPathSettings, } /// A first-party package in a workspace. @@ -109,10 +115,14 @@ impl Workspace { packages.insert(package.root.clone(), Package::from_metadata(db, package)); } - Workspace::builder(metadata.root, packages) - .durability(Durability::MEDIUM) - .open_fileset_durability(Durability::LOW) - .new(db) + Workspace::builder( + metadata.root, + packages, + metadata.settings.program.search_paths, + ) + .durability(Durability::MEDIUM) + .open_fileset_durability(Durability::LOW) + .new(db) } pub fn root(self, db: &dyn Db) -> &SystemPath { @@ -143,6 +153,11 @@ impl Workspace { new_packages.insert(path, package); } + if &metadata.settings.program.search_paths != self.search_path_settings(db) { + self.set_search_path_settings(db) + .to(metadata.settings.program.search_paths); + } + self.set_package_tree(db).to(new_packages); } @@ -331,11 +346,7 @@ impl Package { tracing::debug_span!("index_package_files", package = %self.name(db)).entered(); let files = discover_package_files(db, self.root(db)); - tracing::info!( - "Indexed {} files for package '{}'", - files.len(), - self.name(db) - ); + tracing::info!("Found {} files in package '{}'", files.len(), self.name(db)); vacant.set(files) } Index::Indexed(indexed) => indexed, diff --git a/crates/red_knot_workspace/src/workspace/metadata.rs b/crates/red_knot_workspace/src/workspace/metadata.rs index 5c8262cd6db9f..a56f93e19b745 100644 --- a/crates/red_knot_workspace/src/workspace/metadata.rs +++ b/crates/red_knot_workspace/src/workspace/metadata.rs @@ -1,3 +1,4 @@ +use crate::workspace::settings::{Configuration, WorkspaceSettings}; use ruff_db::system::{System, SystemPath, SystemPathBuf}; use ruff_python_ast::name::Name; @@ -7,6 +8,8 @@ pub struct WorkspaceMetadata { /// The (first-party) packages in this workspace. pub(super) packages: Vec, + + pub(super) settings: WorkspaceSettings, } /// A first-party package in a workspace. @@ -21,7 +24,11 @@ pub struct PackageMetadata { impl WorkspaceMetadata { /// Discovers the closest workspace at `path` and returns its metadata. - pub fn from_path(path: &SystemPath, system: &dyn System) -> anyhow::Result { + pub fn from_path( + path: &SystemPath, + system: &dyn System, + base_configuration: Option, + ) -> anyhow::Result { assert!( system.is_directory(path), "Workspace root path must be a directory" @@ -38,9 +45,20 @@ impl WorkspaceMetadata { root: root.clone(), }; + // TODO: Load the configuration from disk. + let mut configuration = Configuration::default(); + + if let Some(base_configuration) = base_configuration { + configuration.extend(base_configuration); + } + + // TODO: Respect the package configurations when resolving settings (e.g. for the target version). + let settings = configuration.into_workspace_settings(&root); + let workspace = WorkspaceMetadata { root, packages: vec![package], + settings, }; Ok(workspace) @@ -53,6 +71,10 @@ impl WorkspaceMetadata { pub fn packages(&self) -> &[PackageMetadata] { &self.packages } + + pub fn settings(&self) -> &WorkspaceSettings { + &self.settings + } } impl PackageMetadata { diff --git a/crates/red_knot_workspace/src/workspace/settings.rs b/crates/red_knot_workspace/src/workspace/settings.rs new file mode 100644 index 0000000000000..38a633b07dcfe --- /dev/null +++ b/crates/red_knot_workspace/src/workspace/settings.rs @@ -0,0 +1,89 @@ +use red_knot_python_semantic::{ProgramSettings, PythonVersion, SearchPathSettings, SitePackages}; +use ruff_db::system::{SystemPath, SystemPathBuf}; + +/// The resolved configurations. +/// +/// The main difference to [`Configuration`] is that default values are filled in. +#[derive(Debug, Clone)] +pub struct WorkspaceSettings { + pub(super) program: ProgramSettings, +} + +impl WorkspaceSettings { + pub fn program(&self) -> &ProgramSettings { + &self.program + } +} + +/// The configuration for the workspace or a package. +#[derive(Debug, Default, Clone)] +pub struct Configuration { + pub target_version: Option, + pub search_paths: SearchPathConfiguration, +} + +impl Configuration { + /// Extends this configuration by using the values from `with` for all values that are absent in `self`. + pub fn extend(&mut self, with: Configuration) { + self.target_version = self.target_version.or(with.target_version); + self.search_paths.extend(with.search_paths); + } + + pub fn into_workspace_settings(self, workspace_root: &SystemPath) -> WorkspaceSettings { + WorkspaceSettings { + program: ProgramSettings { + target_version: self.target_version.unwrap_or_default(), + search_paths: self.search_paths.into_settings(workspace_root), + }, + } + } +} + +#[derive(Debug, Default, Clone, Eq, PartialEq)] +pub struct SearchPathConfiguration { + /// List of user-provided paths that should take first priority in the module resolution. + /// Examples in other type checkers are mypy's MYPYPATH environment variable, + /// or pyright's stubPath configuration setting. + pub extra_paths: Option>, + + /// The root of the workspace, used for finding first-party modules. + pub src_root: Option, + + /// Optional path to a "custom typeshed" directory on disk for us to use for standard-library types. + /// If this is not provided, we will fallback to our vendored typeshed stubs for the stdlib, + /// bundled as a zip file in the binary + pub custom_typeshed: Option, + + /// The path to the user's `site-packages` directory, where third-party packages from ``PyPI`` are installed. + pub site_packages: Option, +} + +impl SearchPathConfiguration { + pub fn into_settings(self, workspace_root: &SystemPath) -> SearchPathSettings { + let site_packages = self.site_packages.unwrap_or(SitePackages::Known(vec![])); + + SearchPathSettings { + extra_paths: self.extra_paths.unwrap_or_default(), + src_root: self + .src_root + .unwrap_or_else(|| workspace_root.to_path_buf()), + custom_typeshed: self.custom_typeshed, + site_packages, + } + } + + pub fn extend(&mut self, with: SearchPathConfiguration) { + if let Some(extra_paths) = with.extra_paths { + self.extra_paths.get_or_insert(extra_paths); + } + if let Some(src_root) = with.src_root { + self.src_root.get_or_insert(src_root); + } + if let Some(custom_typeshed) = with.custom_typeshed { + self.custom_typeshed.get_or_insert(custom_typeshed); + } + if let Some(site_packages) = with.site_packages { + self.site_packages.get_or_insert(site_packages); + } + } +} diff --git a/crates/red_knot_workspace/tests/check.rs b/crates/red_knot_workspace/tests/check.rs index b9619c611e4a4..e2f8c5fd0ba5a 100644 --- a/crates/red_knot_workspace/tests/check.rs +++ b/crates/red_knot_workspace/tests/check.rs @@ -1,6 +1,7 @@ -use red_knot_python_semantic::{ - HasTy, ProgramSettings, PythonVersion, SearchPathSettings, SemanticModel, -}; +use std::fs; +use std::path::PathBuf; + +use red_knot_python_semantic::{HasTy, SemanticModel}; use red_knot_workspace::db::RootDatabase; use red_knot_workspace::workspace::WorkspaceMetadata; use ruff_db::files::{system_path_to_file, File}; @@ -9,23 +10,11 @@ use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf}; use ruff_python_ast::visitor::source_order; use ruff_python_ast::visitor::source_order::SourceOrderVisitor; use ruff_python_ast::{Alias, Expr, Parameter, ParameterWithDefault, Stmt}; -use std::fs; -use std::path::PathBuf; -fn setup_db(workspace_root: SystemPathBuf) -> anyhow::Result { - let system = OsSystem::new(&workspace_root); - let workspace = WorkspaceMetadata::from_path(&workspace_root, &system)?; - let search_paths = SearchPathSettings { - extra_paths: vec![], - src_root: workspace_root, - custom_typeshed: None, - site_packages: vec![], - }; - let settings = ProgramSettings { - target_version: PythonVersion::default(), - search_paths, - }; - RootDatabase::new(workspace, settings, system) +fn setup_db(workspace_root: &SystemPath) -> anyhow::Result { + let system = OsSystem::new(workspace_root); + let workspace = WorkspaceMetadata::from_path(workspace_root, &system, None)?; + RootDatabase::new(workspace, system) } /// Test that all snippets in testcorpus can be checked without panic @@ -33,8 +22,9 @@ fn setup_db(workspace_root: SystemPathBuf) -> anyhow::Result { #[allow(clippy::print_stdout)] fn corpus_no_panic() -> anyhow::Result<()> { let corpus = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("resources/test/corpus"); - let system_corpus = SystemPath::from_std_path(&corpus).expect("corpus path to be UTF8"); - let db = setup_db(system_corpus.to_path_buf())?; + let system_corpus = + SystemPathBuf::from_path_buf(corpus.clone()).expect("corpus path to be UTF8"); + let db = setup_db(&system_corpus)?; for path in fs::read_dir(&corpus).expect("corpus to be a directory") { let path = path.expect("path to not be an error").path(); diff --git a/crates/ruff_benchmark/benches/red_knot.rs b/crates/ruff_benchmark/benches/red_knot.rs index d920793337b07..312b2b2310313 100644 --- a/crates/ruff_benchmark/benches/red_knot.rs +++ b/crates/ruff_benchmark/benches/red_knot.rs @@ -1,8 +1,9 @@ #![allow(clippy::disallowed_names)] -use red_knot_python_semantic::{ProgramSettings, PythonVersion, SearchPathSettings}; +use red_knot_python_semantic::PythonVersion; use red_knot_workspace::db::RootDatabase; use red_knot_workspace::watch::{ChangeEvent, ChangedKind}; +use red_knot_workspace::workspace::settings::Configuration; use red_knot_workspace::workspace::WorkspaceMetadata; use ruff_benchmark::criterion::{criterion_group, criterion_main, BatchSize, Criterion}; use ruff_benchmark::TestFile; @@ -86,18 +87,17 @@ fn setup_case() -> Case { .unwrap(); let src_root = SystemPath::new("/src"); - let metadata = WorkspaceMetadata::from_path(src_root, &system).unwrap(); - let settings = ProgramSettings { - target_version: PythonVersion::PY312, - search_paths: SearchPathSettings { - extra_paths: vec![], - src_root: src_root.to_path_buf(), - site_packages: vec![], - custom_typeshed: None, - }, - }; - - let mut db = RootDatabase::new(metadata, settings, system).unwrap(); + let metadata = WorkspaceMetadata::from_path( + src_root, + &system, + Some(Configuration { + target_version: Some(PythonVersion::PY312), + ..Configuration::default() + }), + ) + .unwrap(); + + let mut db = RootDatabase::new(metadata, system).unwrap(); let parser = system_path_to_file(&db, parser_path).unwrap(); db.workspace().open_file(&mut db, parser); @@ -131,10 +131,13 @@ fn benchmark_incremental(criterion: &mut Criterion) { |case| { let Case { db, .. } = case; - db.apply_changes(vec![ChangeEvent::Changed { - path: case.re_path.to_path_buf(), - kind: ChangedKind::FileContent, - }]); + db.apply_changes( + vec![ChangeEvent::Changed { + path: case.re_path.to_path_buf(), + kind: ChangedKind::FileContent, + }], + None, + ); let result = db.check().unwrap(); diff --git a/crates/ruff_workspace/src/resolver.rs b/crates/ruff_workspace/src/resolver.rs index 2086d4978c105..3fc92348e7ee4 100644 --- a/crates/ruff_workspace/src/resolver.rs +++ b/crates/ruff_workspace/src/resolver.rs @@ -253,7 +253,7 @@ fn is_package_with_cache<'a>( /// Applies a transformation to a [`Configuration`]. /// /// Used to override options with the values provided by the CLI. -pub trait ConfigurationTransformer: Sync { +pub trait ConfigurationTransformer { fn transform(&self, config: Configuration) -> Configuration; } @@ -334,7 +334,7 @@ pub fn resolve_root_settings( pub fn python_files_in_path<'a>( paths: &[PathBuf], pyproject_config: &'a PyprojectConfig, - transformer: &dyn ConfigurationTransformer, + transformer: &(dyn ConfigurationTransformer + Sync), ) -> Result<(Vec>, Resolver<'a>)> { // Normalize every path (e.g., convert from relative to absolute). let mut paths: Vec = paths.iter().map(fs::normalize_path).unique().collect(); @@ -430,12 +430,12 @@ impl<'config> WalkPythonFilesState<'config> { struct PythonFilesVisitorBuilder<'s, 'config> { state: &'s WalkPythonFilesState<'config>, - transformer: &'s dyn ConfigurationTransformer, + transformer: &'s (dyn ConfigurationTransformer + Sync), } impl<'s, 'config> PythonFilesVisitorBuilder<'s, 'config> { fn new( - transformer: &'s dyn ConfigurationTransformer, + transformer: &'s (dyn ConfigurationTransformer + Sync), state: &'s WalkPythonFilesState<'config>, ) -> Self { Self { state, transformer } @@ -446,7 +446,7 @@ struct PythonFilesVisitor<'s, 'config> { local_files: Vec>, local_error: Result<()>, global: &'s WalkPythonFilesState<'config>, - transformer: &'s dyn ConfigurationTransformer, + transformer: &'s (dyn ConfigurationTransformer + Sync), } impl<'config, 's> ignore::ParallelVisitorBuilder<'s> for PythonFilesVisitorBuilder<'s, 'config>