From 132d10fb6fb30db17ebf894284e97cd2cc831e10 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Thu, 6 Nov 2025 08:27:49 -0600 Subject: [PATCH] [ty] Discover site-packages from the environment that ty is installed in (#21286) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Closes https://github.com/astral-sh/ty/issues/989 There are various situations where users expect the Python packages installed in the same environment as ty itself to be considered during type checking. A minimal example would look like: ``` uv venv my-env uv pip install my-env ty httpx echo "import httpx" > foo.py ./my-env/bin/ty check foo.py ``` or ``` uv tool install ty --with httpx echo "import httpx" > foo.py ty check foo.py ``` While these are a bit contrived, there are real-world situations where a user would expect a similar behavior to work. Notably, all of the other type checkers consider their own environment when determining search paths (though I'll admit that I have not verified when they choose not to do this). One common situation where users are encountering this today is with `uvx --with-requirements script.py ty check script.py` — which is currently our "best" recommendation for type checking a PEP 723 script, but it doesn't work. Of the options discussed in https://github.com/astral-sh/ty/issues/989#issuecomment-3307417985, I've chosen (2) as our criteria for including ty's environment in the search paths. - If no virtual environment is discovered, we will always include ty's environment. - If a `.venv` is discovered in the working directory, we will _prepend_ ty's environment to the search paths. The dependencies in ty's environment (e.g., from `uvx --with`) will take precedence. - If a virtual environment is active, e.g., `VIRTUAL_ENV` (i.e., including conda prefixes) is set, we will not include ty's environment. The reason we need to special case the `.venv` case is that we both 1. Recommend `uvx ty` today as a way to check your project 2. Want to enable `uvx --with <...> ty` And I don't want (2) to break when you _happen_ to be in a project (i.e., if we only included ty's environment when _no_ environment is found) and don't want to remove support for (1). I think long-term, I want to make `uvx ` layer the environment on _top_ of the project environment (in uv), which would obviate the need for this change when you're using uv. However, that change is breaking and I think users will expect this behavior in contexts where they're not using uv, so I think we should handle it in ty regardless. I've opted not to include the environment if it's non-virtual (i.e., a system environment) for now. It seems better to start by being more restrictive. I left a comment in the code. ## Test Plan I did some manual testing with the initial commit, then subsequently added some unit tests. ``` ❯ echo "import httpx" > example.py ❯ uvx --with httpx ty check example.py Installed 8 packages in 19ms error[unresolved-import]: Cannot resolve imported module `httpx` --> foo/example.py:1:8 | 1 | import httpx | ^^^^^ | info: Searched in the following paths during module resolution: info: 1. /Users/zb/workspace/ty/python (first-party code) info: 2. /Users/zb/workspace/ty (first-party code) info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty) info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment info: rule `unresolved-import` is enabled by default Found 1 diagnostic ❯ uvx --from . --with httpx ty check example.py All checks passed! ``` ``` ❯ uv init --script foo.py Initialized script at `foo.py` ❯ uv add --script foo.py httpx warning: The Python request from `.python-version` resolved to Python 3.13.8, which is incompatible with the script's Python requirement: `>=3.14` Updated `foo.py` ❯ echo "import httpx" >> foo.py ❯ uvx --with-requirements foo.py ty check foo.py error[unresolved-import]: Cannot resolve imported module `httpx` --> foo.py:15:8 | 13 | if __name__ == "__main__": 14 | main() 15 | import httpx | ^^^^^ | info: Searched in the following paths during module resolution: info: 1. /Users/zb/workspace/ty/python (first-party code) info: 2. /Users/zb/workspace/ty (first-party code) info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty) info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment info: rule `unresolved-import` is enabled by default Found 1 diagnostic ❯ uvx --from . --with-requirements foo.py ty check foo.py All checks passed! ``` Notice we do not include ty's environment if `VIRTUAL_ENV` is set ``` ❯ VIRTUAL_ENV=.venv uvx --with httpx ty check foo/example.py error[unresolved-import]: Cannot resolve imported module `httpx` --> foo/example.py:1:8 | 1 | import httpx | ^^^^^ | info: Searched in the following paths during module resolution: info: 1. /Users/zb/workspace/ty/python (first-party code) info: 2. /Users/zb/workspace/ty (first-party code) info: 3. vendored://stdlib (stdlib typeshed stubs vendored by ty) info: 4. /Users/zb/workspace/ty/.venv/lib/python3.13/site-packages (site-packages) info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment info: rule `unresolved-import` is enabled by default Found 1 diagnostic ``` --- crates/ty/tests/cli/main.rs | 43 ++- crates/ty/tests/cli/python_environment.rs | 274 +++++++++++++++++- crates/ty_project/src/metadata/options.rs | 56 +++- .../ty_python_semantic/src/site_packages.rs | 62 +++- 4 files changed, 417 insertions(+), 18 deletions(-) diff --git a/crates/ty/tests/cli/main.rs b/crates/ty/tests/cli/main.rs index e911e300c0..446ba86611 100644 --- a/crates/ty/tests/cli/main.rs +++ b/crates/ty/tests/cli/main.rs @@ -5,6 +5,7 @@ mod python_environment; mod rule_selection; use anyhow::Context as _; +use insta::Settings; use insta::internals::SettingsBindDropGuard; use insta_cmd::{assert_cmd_snapshot, get_cargo_bin}; use std::{ @@ -760,8 +761,10 @@ fn can_handle_large_binop_expressions() -> anyhow::Result<()> { pub(crate) struct CliTest { _temp_dir: TempDir, - _settings_scope: SettingsBindDropGuard, + settings: Settings, + settings_scope: Option, project_dir: PathBuf, + ty_binary_path: PathBuf, } impl CliTest { @@ -794,7 +797,9 @@ impl CliTest { Ok(Self { project_dir, _temp_dir: temp_dir, - _settings_scope: settings_scope, + settings, + settings_scope: Some(settings_scope), + ty_binary_path: get_cargo_bin("ty"), }) } @@ -823,6 +828,30 @@ impl CliTest { Ok(()) } + /// Return [`Self`] with the ty binary copied to the specified path instead. + pub(crate) fn with_ty_at(mut self, dest_path: impl AsRef) -> anyhow::Result { + let dest_path = dest_path.as_ref(); + let dest_path = self.project_dir.join(dest_path); + + Self::ensure_parent_directory(&dest_path)?; + std::fs::copy(&self.ty_binary_path, &dest_path) + .with_context(|| format!("Failed to copy ty binary to `{}`", dest_path.display()))?; + + self.ty_binary_path = dest_path; + Ok(self) + } + + /// Add a filter to the settings and rebind them. + pub(crate) fn with_filter(mut self, pattern: &str, replacement: &str) -> Self { + self.settings.add_filter(pattern, replacement); + // Drop the old scope before binding a new one, otherwise the old scope is dropped _after_ + // binding and assigning the new one, restoring the settings to their state before the old + // scope was bound. + drop(self.settings_scope.take()); + self.settings_scope = Some(self.settings.bind_to_scope()); + self + } + fn ensure_parent_directory(path: &Path) -> anyhow::Result<()> { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent) @@ -868,7 +897,7 @@ impl CliTest { } pub(crate) fn command(&self) -> Command { - let mut command = Command::new(get_cargo_bin("ty")); + let mut command = Command::new(&self.ty_binary_path); command.current_dir(&self.project_dir).arg("check"); // Unset all environment variables because they can affect test behavior. @@ -881,3 +910,11 @@ impl CliTest { fn tempdir_filter(path: &Path) -> String { format!(r"{}\\?/?", regex::escape(path.to_str().unwrap())) } + +fn site_packages_filter(python_version: &str) -> String { + if cfg!(windows) { + "Lib/site-packages".to_string() + } else { + format!("lib/python{}/site-packages", regex::escape(python_version)) + } +} diff --git a/crates/ty/tests/cli/python_environment.rs b/crates/ty/tests/cli/python_environment.rs index 04fa8be88f..de6d99aa9a 100644 --- a/crates/ty/tests/cli/python_environment.rs +++ b/crates/ty/tests/cli/python_environment.rs @@ -1,7 +1,7 @@ use insta_cmd::assert_cmd_snapshot; use ruff_python_ast::PythonVersion; -use crate::CliTest; +use crate::{CliTest, site_packages_filter}; /// Specifying an option on the CLI should take precedence over the same setting in the /// project's configuration. Here, this is tested for the Python version. @@ -1654,6 +1654,278 @@ home = ./ Ok(()) } +/// ty should include site packages from its own environment when no other environment is found. +#[test] +fn ty_environment_is_only_environment() -> anyhow::Result<()> { + let ty_venv_site_packages = if cfg!(windows) { + "ty-venv/Lib/site-packages" + } else { + "ty-venv/lib/python3.13/site-packages" + }; + + let ty_executable_path = if cfg!(windows) { + "ty-venv/Scripts/ty.exe" + } else { + "ty-venv/bin/ty" + }; + + let ty_package_path = format!("{ty_venv_site_packages}/ty_package/__init__.py"); + + let case = CliTest::with_files([ + (ty_package_path.as_str(), "class TyEnvClass: ..."), + ( + "ty-venv/pyvenv.cfg", + r" + home = ./ + version = 3.13 + ", + ), + ( + "test.py", + r" + from ty_package import TyEnvClass + ", + ), + ])?; + + let case = case.with_ty_at(ty_executable_path)?; + assert_cmd_snapshot!(case.command(), @r###" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + "###); + + Ok(()) +} + +/// ty should include site packages from both its own environment and a local `.venv`. The packages +/// from ty's environment should take precedence. +#[test] +fn ty_environment_and_discovered_venv() -> anyhow::Result<()> { + let ty_venv_site_packages = if cfg!(windows) { + "ty-venv/Lib/site-packages" + } else { + "ty-venv/lib/python3.13/site-packages" + }; + + let ty_executable_path = if cfg!(windows) { + "ty-venv/Scripts/ty.exe" + } else { + "ty-venv/bin/ty" + }; + + let local_venv_site_packages = if cfg!(windows) { + ".venv/Lib/site-packages" + } else { + ".venv/lib/python3.13/site-packages" + }; + + let ty_unique_package = format!("{ty_venv_site_packages}/ty_package/__init__.py"); + let local_unique_package = format!("{local_venv_site_packages}/local_package/__init__.py"); + let ty_conflicting_package = format!("{ty_venv_site_packages}/shared_package/__init__.py"); + let local_conflicting_package = + format!("{local_venv_site_packages}/shared_package/__init__.py"); + + let case = CliTest::with_files([ + (ty_unique_package.as_str(), "class TyEnvClass: ..."), + (local_unique_package.as_str(), "class LocalClass: ..."), + (ty_conflicting_package.as_str(), "class FromTyEnv: ..."), + ( + local_conflicting_package.as_str(), + "class FromLocalVenv: ...", + ), + ( + "ty-venv/pyvenv.cfg", + r" + home = ./ + version = 3.13 + ", + ), + ( + ".venv/pyvenv.cfg", + r" + home = ./ + version = 3.13 + ", + ), + ( + "test.py", + r" + # Should resolve from ty's environment + from ty_package import TyEnvClass + # Should resolve from local .venv + from local_package import LocalClass + # Should resolve from ty's environment (takes precedence) + from shared_package import FromTyEnv + # Should NOT resolve (shadowed by ty's environment version) + from shared_package import FromLocalVenv + ", + ), + ])? + .with_ty_at(ty_executable_path)?; + + assert_cmd_snapshot!(case.command(), @r###" + success: false + exit_code: 1 + ----- stdout ----- + error[unresolved-import]: Module `shared_package` has no member `FromLocalVenv` + --> test.py:9:28 + | + 7 | from shared_package import FromTyEnv + 8 | # Should NOT resolve (shadowed by ty's environment version) + 9 | from shared_package import FromLocalVenv + | ^^^^^^^^^^^^^ + | + info: rule `unresolved-import` is enabled by default + + Found 1 diagnostic + + ----- stderr ----- + "###); + + Ok(()) +} + +/// When `VIRTUAL_ENV` is set, ty should *not* discover its own environment's site-packages. +#[test] +fn ty_environment_and_active_environment() -> anyhow::Result<()> { + let ty_venv_site_packages = if cfg!(windows) { + "ty-venv/Lib/site-packages" + } else { + "ty-venv/lib/python3.13/site-packages" + }; + + let ty_executable_path = if cfg!(windows) { + "ty-venv/Scripts/ty.exe" + } else { + "ty-venv/bin/ty" + }; + + let active_venv_site_packages = if cfg!(windows) { + "active-venv/Lib/site-packages" + } else { + "active-venv/lib/python3.13/site-packages" + }; + + let ty_package_path = format!("{ty_venv_site_packages}/ty_package/__init__.py"); + let active_package_path = format!("{active_venv_site_packages}/active_package/__init__.py"); + + let case = CliTest::with_files([ + (ty_package_path.as_str(), "class TyEnvClass: ..."), + ( + "ty-venv/pyvenv.cfg", + r" + home = ./ + version = 3.13 + ", + ), + (active_package_path.as_str(), "class ActiveClass: ..."), + ( + "active-venv/pyvenv.cfg", + r" + home = ./ + version = 3.13 + ", + ), + ( + "test.py", + r" + from ty_package import TyEnvClass + from active_package import ActiveClass + ", + ), + ])? + .with_ty_at(ty_executable_path)? + .with_filter(&site_packages_filter("3.13"), ""); + + assert_cmd_snapshot!( + case.command() + .env("VIRTUAL_ENV", case.root().join("active-venv")), + @r" + success: false + exit_code: 1 + ----- stdout ----- + error[unresolved-import]: Cannot resolve imported module `ty_package` + --> test.py:2:6 + | + 2 | from ty_package import TyEnvClass + | ^^^^^^^^^^ + 3 | from active_package import ActiveClass + | + info: Searched in the following paths during module resolution: + info: 1. / (first-party code) + info: 2. vendored://stdlib (stdlib typeshed stubs vendored by ty) + info: 3. /active-venv/ (site-packages) + info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment + info: rule `unresolved-import` is enabled by default + + Found 1 diagnostic + + ----- stderr ----- + " + ); + + Ok(()) +} + +/// When ty is installed in a system environment rather than a virtual environment, it should +/// not include the environment's site-packages in its search path. +#[test] +fn ty_environment_is_system_not_virtual() -> anyhow::Result<()> { + let ty_system_site_packages = if cfg!(windows) { + "system-python/Lib/site-packages" + } else { + "system-python/lib/python3.13/site-packages" + }; + + let ty_executable_path = if cfg!(windows) { + "system-python/Scripts/ty.exe" + } else { + "system-python/bin/ty" + }; + + let ty_package_path = format!("{ty_system_site_packages}/system_package/__init__.py"); + + let case = CliTest::with_files([ + // Package in system Python installation (should NOT be discovered) + (ty_package_path.as_str(), "class SystemClass: ..."), + // Note: NO pyvenv.cfg - this is a system installation, not a venv + ( + "test.py", + r" + from system_package import SystemClass + ", + ), + ])? + .with_ty_at(ty_executable_path)?; + + assert_cmd_snapshot!(case.command(), @r###" + success: false + exit_code: 1 + ----- stdout ----- + error[unresolved-import]: Cannot resolve imported module `system_package` + --> test.py:2:6 + | + 2 | from system_package import SystemClass + | ^^^^^^^^^^^^^^ + | + info: Searched in the following paths during module resolution: + info: 1. / (first-party code) + info: 2. vendored://stdlib (stdlib typeshed stubs vendored by ty) + info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment + info: rule `unresolved-import` is enabled by default + + Found 1 diagnostic + + ----- stderr ----- + "###); + + Ok(()) +} + #[test] fn src_root_deprecation_warning() -> anyhow::Result<()> { let case = CliTest::with_files([ diff --git a/crates/ty_project/src/metadata/options.rs b/crates/ty_project/src/metadata/options.rs index 1e498f6bf8..d83270be90 100644 --- a/crates/ty_project/src/metadata/options.rs +++ b/crates/ty_project/src/metadata/options.rs @@ -164,14 +164,24 @@ impl Options { .context("Failed to discover local Python environment")? }; - let site_packages_paths = if let Some(python_environment) = python_environment.as_ref() { + let self_site_packages = self_environment_search_paths( python_environment - .site_packages_paths(system) - .context("Failed to discover the site-packages directory")? + .as_ref() + .map(ty_python_semantic::PythonEnvironment::origin) + .cloned(), + system, + ) + .unwrap_or_default(); + + let site_packages_paths = if let Some(python_environment) = python_environment.as_ref() { + self_site_packages.concatenate( + python_environment + .site_packages_paths(system) + .context("Failed to discover the site-packages directory")?, + ) } else { tracing::debug!("No virtual environment found"); - - SitePackagesPaths::default() + self_site_packages }; let real_stdlib_path = python_environment.as_ref().and_then(|python_environment| { @@ -461,6 +471,42 @@ impl Options { } } +/// Return the site-packages from the environment ty is installed in, as derived from ty's +/// executable. +/// +/// If there's an existing environment with an origin that does not allow including site-packages +/// from ty's environment, discovery of ty's environment is skipped and [`None`] is returned. +/// +/// Since ty may be executed from an arbitrary non-Python location, errors during discovery of ty's +/// environment are not raised, instead [`None`] is returned. +fn self_environment_search_paths( + existing_origin: Option, + system: &dyn System, +) -> Option { + if existing_origin.is_some_and(|origin| !origin.allows_concatenation_with_self_environment()) { + return None; + } + + let Ok(exe_path) = std::env::current_exe() else { + return None; + }; + let ty_path = SystemPath::from_std_path(exe_path.as_path())?; + + let environment = PythonEnvironment::new(ty_path, SysPrefixPathOrigin::SelfEnvironment, system) + .inspect_err(|err| tracing::debug!("Failed to discover ty's environment: {err}")) + .ok()?; + + let search_paths = environment + .site_packages_paths(system) + .inspect_err(|err| { + tracing::debug!("Failed to discover site-packages in ty's environment: {err}"); + }) + .ok(); + + tracing::debug!("Using site-packages from ty's environment"); + search_paths +} + #[derive( Debug, Default, diff --git a/crates/ty_python_semantic/src/site_packages.rs b/crates/ty_python_semantic/src/site_packages.rs index c162dfc70a..8418db6124 100644 --- a/crates/ty_python_semantic/src/site_packages.rs +++ b/crates/ty_python_semantic/src/site_packages.rs @@ -62,6 +62,15 @@ impl SitePackagesPaths { self.0.extend(other.0); } + /// Concatenate two instances of [`SitePackagesPaths`]. + #[must_use] + pub fn concatenate(mut self, other: Self) -> Self { + for path in other { + self.0.insert(path); + } + self + } + /// Tries to detect the version from the layout of the `site-packages` directory. pub fn python_version_from_layout(&self) -> Option { if cfg!(windows) { @@ -252,6 +261,13 @@ impl PythonEnvironment { Self::System(env) => env.real_stdlib_directory(system), } } + + pub fn origin(&self) -> &SysPrefixPathOrigin { + match self { + Self::Virtual(env) => &env.root_path.origin, + Self::System(env) => &env.root_path.origin, + } + } } /// Enumeration of the subdirectories of `sys.prefix` that could contain a @@ -1393,15 +1409,15 @@ impl SysPrefixPath { ) -> SitePackagesDiscoveryResult { let sys_prefix = if !origin.must_point_directly_to_sys_prefix() && system.is_file(unvalidated_path) - && unvalidated_path - .file_name() - .is_some_and(|name| name.starts_with("python")) - { - // It looks like they passed us a path to a Python executable, e.g. `.venv/bin/python3`. - // Try to figure out the `sys.prefix` value from the Python executable. + && unvalidated_path.file_name().is_some_and(|name| { + name.starts_with("python") + || name.eq_ignore_ascii_case(&format!("ty{}", std::env::consts::EXE_SUFFIX)) + }) { + // It looks like they passed us a path to an executable, e.g. `.venv/bin/python3`. Try + // to figure out the `sys.prefix` value from the Python executable. let sys_prefix = if cfg!(windows) { - // On Windows, the relative path to the Python executable from `sys.prefix` - // is different depending on whether it's a virtual environment or a system installation. + // On Windows, the relative path to the executable from `sys.prefix` is different + // depending on whether it's a virtual environment or a system installation. // System installations have their executable at `/python.exe`, // whereas virtual environments have their executable at `/Scripts/python.exe`. unvalidated_path.parent().and_then(|parent| { @@ -1586,6 +1602,8 @@ pub enum SysPrefixPathOrigin { /// A `.venv` directory was found in the current working directory, /// and the `sys.prefix` path is the path to that virtual environment. LocalVenv, + /// The `sys.prefix` path came from the environment ty is installed in. + SelfEnvironment, } impl SysPrefixPathOrigin { @@ -1599,6 +1617,13 @@ impl SysPrefixPathOrigin { | Self::Editor | Self::DerivedFromPyvenvCfg | Self::CondaPrefixVar => false, + // It's not strictly true that the self environment must be virtual, e.g., ty could be + // installed in a system Python environment and users may expect us to respect + // dependencies installed alongside it. However, we're intentionally excluding support + // for this to start. Note a change here has downstream implications, i.e., we probably + // don't want the packages in a system environment to take precedence over those in a + // virtual environment and would need to reverse the ordering in that case. + Self::SelfEnvironment => true, } } @@ -1608,13 +1633,31 @@ impl SysPrefixPathOrigin { /// the `sys.prefix` directory, e.g. the `--python` CLI flag. pub(crate) const fn must_point_directly_to_sys_prefix(&self) -> bool { match self { - Self::PythonCliFlag | Self::ConfigFileSetting(..) | Self::Editor => false, + Self::PythonCliFlag + | Self::ConfigFileSetting(..) + | Self::Editor + | Self::SelfEnvironment => false, Self::VirtualEnvVar | Self::CondaPrefixVar | Self::DerivedFromPyvenvCfg | Self::LocalVenv => true, } } + + /// Whether paths with this origin should allow combination with paths with a + /// [`SysPrefixPathOrigin::SelfEnvironment`] origin. + pub const fn allows_concatenation_with_self_environment(&self) -> bool { + match self { + Self::SelfEnvironment + | Self::CondaPrefixVar + | Self::VirtualEnvVar + | Self::Editor + | Self::DerivedFromPyvenvCfg + | Self::ConfigFileSetting(..) + | Self::PythonCliFlag => false, + Self::LocalVenv => true, + } + } } impl std::fmt::Display for SysPrefixPathOrigin { @@ -1627,6 +1670,7 @@ impl std::fmt::Display for SysPrefixPathOrigin { Self::DerivedFromPyvenvCfg => f.write_str("derived `sys.prefix` path"), Self::LocalVenv => f.write_str("local virtual environment"), Self::Editor => f.write_str("selected interpreter in your editor"), + Self::SelfEnvironment => f.write_str("ty environment"), } } }