Skip to content

Commit 132d10f

Browse files
authored
[ty] Discover site-packages from the environment that ty is installed in (#21286)
<!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## Summary <!-- What's the purpose of the change? What does it do, and why? --> Closes astral-sh/ty#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 astral-sh/ty#989 (comment), 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 <cmd>` 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 ```
1 parent f189aad commit 132d10f

File tree

4 files changed

+417
-18
lines changed

4 files changed

+417
-18
lines changed

crates/ty/tests/cli/main.rs

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ mod python_environment;
55
mod rule_selection;
66

77
use anyhow::Context as _;
8+
use insta::Settings;
89
use insta::internals::SettingsBindDropGuard;
910
use insta_cmd::{assert_cmd_snapshot, get_cargo_bin};
1011
use std::{
@@ -760,8 +761,10 @@ fn can_handle_large_binop_expressions() -> anyhow::Result<()> {
760761

761762
pub(crate) struct CliTest {
762763
_temp_dir: TempDir,
763-
_settings_scope: SettingsBindDropGuard,
764+
settings: Settings,
765+
settings_scope: Option<SettingsBindDropGuard>,
764766
project_dir: PathBuf,
767+
ty_binary_path: PathBuf,
765768
}
766769

767770
impl CliTest {
@@ -794,7 +797,9 @@ impl CliTest {
794797
Ok(Self {
795798
project_dir,
796799
_temp_dir: temp_dir,
797-
_settings_scope: settings_scope,
800+
settings,
801+
settings_scope: Some(settings_scope),
802+
ty_binary_path: get_cargo_bin("ty"),
798803
})
799804
}
800805

@@ -823,6 +828,30 @@ impl CliTest {
823828
Ok(())
824829
}
825830

831+
/// Return [`Self`] with the ty binary copied to the specified path instead.
832+
pub(crate) fn with_ty_at(mut self, dest_path: impl AsRef<Path>) -> anyhow::Result<Self> {
833+
let dest_path = dest_path.as_ref();
834+
let dest_path = self.project_dir.join(dest_path);
835+
836+
Self::ensure_parent_directory(&dest_path)?;
837+
std::fs::copy(&self.ty_binary_path, &dest_path)
838+
.with_context(|| format!("Failed to copy ty binary to `{}`", dest_path.display()))?;
839+
840+
self.ty_binary_path = dest_path;
841+
Ok(self)
842+
}
843+
844+
/// Add a filter to the settings and rebind them.
845+
pub(crate) fn with_filter(mut self, pattern: &str, replacement: &str) -> Self {
846+
self.settings.add_filter(pattern, replacement);
847+
// Drop the old scope before binding a new one, otherwise the old scope is dropped _after_
848+
// binding and assigning the new one, restoring the settings to their state before the old
849+
// scope was bound.
850+
drop(self.settings_scope.take());
851+
self.settings_scope = Some(self.settings.bind_to_scope());
852+
self
853+
}
854+
826855
fn ensure_parent_directory(path: &Path) -> anyhow::Result<()> {
827856
if let Some(parent) = path.parent() {
828857
std::fs::create_dir_all(parent)
@@ -868,7 +897,7 @@ impl CliTest {
868897
}
869898

870899
pub(crate) fn command(&self) -> Command {
871-
let mut command = Command::new(get_cargo_bin("ty"));
900+
let mut command = Command::new(&self.ty_binary_path);
872901
command.current_dir(&self.project_dir).arg("check");
873902

874903
// Unset all environment variables because they can affect test behavior.
@@ -881,3 +910,11 @@ impl CliTest {
881910
fn tempdir_filter(path: &Path) -> String {
882911
format!(r"{}\\?/?", regex::escape(path.to_str().unwrap()))
883912
}
913+
914+
fn site_packages_filter(python_version: &str) -> String {
915+
if cfg!(windows) {
916+
"Lib/site-packages".to_string()
917+
} else {
918+
format!("lib/python{}/site-packages", regex::escape(python_version))
919+
}
920+
}

crates/ty/tests/cli/python_environment.rs

Lines changed: 273 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use insta_cmd::assert_cmd_snapshot;
22
use ruff_python_ast::PythonVersion;
33

4-
use crate::CliTest;
4+
use crate::{CliTest, site_packages_filter};
55

66
/// Specifying an option on the CLI should take precedence over the same setting in the
77
/// project's configuration. Here, this is tested for the Python version.
@@ -1654,6 +1654,278 @@ home = ./
16541654
Ok(())
16551655
}
16561656

1657+
/// ty should include site packages from its own environment when no other environment is found.
1658+
#[test]
1659+
fn ty_environment_is_only_environment() -> anyhow::Result<()> {
1660+
let ty_venv_site_packages = if cfg!(windows) {
1661+
"ty-venv/Lib/site-packages"
1662+
} else {
1663+
"ty-venv/lib/python3.13/site-packages"
1664+
};
1665+
1666+
let ty_executable_path = if cfg!(windows) {
1667+
"ty-venv/Scripts/ty.exe"
1668+
} else {
1669+
"ty-venv/bin/ty"
1670+
};
1671+
1672+
let ty_package_path = format!("{ty_venv_site_packages}/ty_package/__init__.py");
1673+
1674+
let case = CliTest::with_files([
1675+
(ty_package_path.as_str(), "class TyEnvClass: ..."),
1676+
(
1677+
"ty-venv/pyvenv.cfg",
1678+
r"
1679+
home = ./
1680+
version = 3.13
1681+
",
1682+
),
1683+
(
1684+
"test.py",
1685+
r"
1686+
from ty_package import TyEnvClass
1687+
",
1688+
),
1689+
])?;
1690+
1691+
let case = case.with_ty_at(ty_executable_path)?;
1692+
assert_cmd_snapshot!(case.command(), @r###"
1693+
success: true
1694+
exit_code: 0
1695+
----- stdout -----
1696+
All checks passed!
1697+
1698+
----- stderr -----
1699+
"###);
1700+
1701+
Ok(())
1702+
}
1703+
1704+
/// ty should include site packages from both its own environment and a local `.venv`. The packages
1705+
/// from ty's environment should take precedence.
1706+
#[test]
1707+
fn ty_environment_and_discovered_venv() -> anyhow::Result<()> {
1708+
let ty_venv_site_packages = if cfg!(windows) {
1709+
"ty-venv/Lib/site-packages"
1710+
} else {
1711+
"ty-venv/lib/python3.13/site-packages"
1712+
};
1713+
1714+
let ty_executable_path = if cfg!(windows) {
1715+
"ty-venv/Scripts/ty.exe"
1716+
} else {
1717+
"ty-venv/bin/ty"
1718+
};
1719+
1720+
let local_venv_site_packages = if cfg!(windows) {
1721+
".venv/Lib/site-packages"
1722+
} else {
1723+
".venv/lib/python3.13/site-packages"
1724+
};
1725+
1726+
let ty_unique_package = format!("{ty_venv_site_packages}/ty_package/__init__.py");
1727+
let local_unique_package = format!("{local_venv_site_packages}/local_package/__init__.py");
1728+
let ty_conflicting_package = format!("{ty_venv_site_packages}/shared_package/__init__.py");
1729+
let local_conflicting_package =
1730+
format!("{local_venv_site_packages}/shared_package/__init__.py");
1731+
1732+
let case = CliTest::with_files([
1733+
(ty_unique_package.as_str(), "class TyEnvClass: ..."),
1734+
(local_unique_package.as_str(), "class LocalClass: ..."),
1735+
(ty_conflicting_package.as_str(), "class FromTyEnv: ..."),
1736+
(
1737+
local_conflicting_package.as_str(),
1738+
"class FromLocalVenv: ...",
1739+
),
1740+
(
1741+
"ty-venv/pyvenv.cfg",
1742+
r"
1743+
home = ./
1744+
version = 3.13
1745+
",
1746+
),
1747+
(
1748+
".venv/pyvenv.cfg",
1749+
r"
1750+
home = ./
1751+
version = 3.13
1752+
",
1753+
),
1754+
(
1755+
"test.py",
1756+
r"
1757+
# Should resolve from ty's environment
1758+
from ty_package import TyEnvClass
1759+
# Should resolve from local .venv
1760+
from local_package import LocalClass
1761+
# Should resolve from ty's environment (takes precedence)
1762+
from shared_package import FromTyEnv
1763+
# Should NOT resolve (shadowed by ty's environment version)
1764+
from shared_package import FromLocalVenv
1765+
",
1766+
),
1767+
])?
1768+
.with_ty_at(ty_executable_path)?;
1769+
1770+
assert_cmd_snapshot!(case.command(), @r###"
1771+
success: false
1772+
exit_code: 1
1773+
----- stdout -----
1774+
error[unresolved-import]: Module `shared_package` has no member `FromLocalVenv`
1775+
--> test.py:9:28
1776+
|
1777+
7 | from shared_package import FromTyEnv
1778+
8 | # Should NOT resolve (shadowed by ty's environment version)
1779+
9 | from shared_package import FromLocalVenv
1780+
| ^^^^^^^^^^^^^
1781+
|
1782+
info: rule `unresolved-import` is enabled by default
1783+
1784+
Found 1 diagnostic
1785+
1786+
----- stderr -----
1787+
"###);
1788+
1789+
Ok(())
1790+
}
1791+
1792+
/// When `VIRTUAL_ENV` is set, ty should *not* discover its own environment's site-packages.
1793+
#[test]
1794+
fn ty_environment_and_active_environment() -> anyhow::Result<()> {
1795+
let ty_venv_site_packages = if cfg!(windows) {
1796+
"ty-venv/Lib/site-packages"
1797+
} else {
1798+
"ty-venv/lib/python3.13/site-packages"
1799+
};
1800+
1801+
let ty_executable_path = if cfg!(windows) {
1802+
"ty-venv/Scripts/ty.exe"
1803+
} else {
1804+
"ty-venv/bin/ty"
1805+
};
1806+
1807+
let active_venv_site_packages = if cfg!(windows) {
1808+
"active-venv/Lib/site-packages"
1809+
} else {
1810+
"active-venv/lib/python3.13/site-packages"
1811+
};
1812+
1813+
let ty_package_path = format!("{ty_venv_site_packages}/ty_package/__init__.py");
1814+
let active_package_path = format!("{active_venv_site_packages}/active_package/__init__.py");
1815+
1816+
let case = CliTest::with_files([
1817+
(ty_package_path.as_str(), "class TyEnvClass: ..."),
1818+
(
1819+
"ty-venv/pyvenv.cfg",
1820+
r"
1821+
home = ./
1822+
version = 3.13
1823+
",
1824+
),
1825+
(active_package_path.as_str(), "class ActiveClass: ..."),
1826+
(
1827+
"active-venv/pyvenv.cfg",
1828+
r"
1829+
home = ./
1830+
version = 3.13
1831+
",
1832+
),
1833+
(
1834+
"test.py",
1835+
r"
1836+
from ty_package import TyEnvClass
1837+
from active_package import ActiveClass
1838+
",
1839+
),
1840+
])?
1841+
.with_ty_at(ty_executable_path)?
1842+
.with_filter(&site_packages_filter("3.13"), "<site-packages>");
1843+
1844+
assert_cmd_snapshot!(
1845+
case.command()
1846+
.env("VIRTUAL_ENV", case.root().join("active-venv")),
1847+
@r"
1848+
success: false
1849+
exit_code: 1
1850+
----- stdout -----
1851+
error[unresolved-import]: Cannot resolve imported module `ty_package`
1852+
--> test.py:2:6
1853+
|
1854+
2 | from ty_package import TyEnvClass
1855+
| ^^^^^^^^^^
1856+
3 | from active_package import ActiveClass
1857+
|
1858+
info: Searched in the following paths during module resolution:
1859+
info: 1. <temp_dir>/ (first-party code)
1860+
info: 2. vendored://stdlib (stdlib typeshed stubs vendored by ty)
1861+
info: 3. <temp_dir>/active-venv/<site-packages> (site-packages)
1862+
info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment
1863+
info: rule `unresolved-import` is enabled by default
1864+
1865+
Found 1 diagnostic
1866+
1867+
----- stderr -----
1868+
"
1869+
);
1870+
1871+
Ok(())
1872+
}
1873+
1874+
/// When ty is installed in a system environment rather than a virtual environment, it should
1875+
/// not include the environment's site-packages in its search path.
1876+
#[test]
1877+
fn ty_environment_is_system_not_virtual() -> anyhow::Result<()> {
1878+
let ty_system_site_packages = if cfg!(windows) {
1879+
"system-python/Lib/site-packages"
1880+
} else {
1881+
"system-python/lib/python3.13/site-packages"
1882+
};
1883+
1884+
let ty_executable_path = if cfg!(windows) {
1885+
"system-python/Scripts/ty.exe"
1886+
} else {
1887+
"system-python/bin/ty"
1888+
};
1889+
1890+
let ty_package_path = format!("{ty_system_site_packages}/system_package/__init__.py");
1891+
1892+
let case = CliTest::with_files([
1893+
// Package in system Python installation (should NOT be discovered)
1894+
(ty_package_path.as_str(), "class SystemClass: ..."),
1895+
// Note: NO pyvenv.cfg - this is a system installation, not a venv
1896+
(
1897+
"test.py",
1898+
r"
1899+
from system_package import SystemClass
1900+
",
1901+
),
1902+
])?
1903+
.with_ty_at(ty_executable_path)?;
1904+
1905+
assert_cmd_snapshot!(case.command(), @r###"
1906+
success: false
1907+
exit_code: 1
1908+
----- stdout -----
1909+
error[unresolved-import]: Cannot resolve imported module `system_package`
1910+
--> test.py:2:6
1911+
|
1912+
2 | from system_package import SystemClass
1913+
| ^^^^^^^^^^^^^^
1914+
|
1915+
info: Searched in the following paths during module resolution:
1916+
info: 1. <temp_dir>/ (first-party code)
1917+
info: 2. vendored://stdlib (stdlib typeshed stubs vendored by ty)
1918+
info: make sure your Python environment is properly configured: https://docs.astral.sh/ty/modules/#python-environment
1919+
info: rule `unresolved-import` is enabled by default
1920+
1921+
Found 1 diagnostic
1922+
1923+
----- stderr -----
1924+
"###);
1925+
1926+
Ok(())
1927+
}
1928+
16571929
#[test]
16581930
fn src_root_deprecation_warning() -> anyhow::Result<()> {
16591931
let case = CliTest::with_files([

0 commit comments

Comments
 (0)