diff --git a/crates/ruff/tests/analyze_graph.rs b/crates/ruff/tests/analyze_graph.rs index e59a01c294..3c5ba4498b 100644 --- a/crates/ruff/tests/analyze_graph.rs +++ b/crates/ruff/tests/analyze_graph.rs @@ -566,7 +566,7 @@ fn venv() -> Result<()> { ----- stderr ----- ruff failed Cause: Invalid search path settings - Cause: Failed to discover the site-packages directory: Invalid `--python` argument: `none` could not be canonicalized + Cause: Failed to discover the site-packages directory: Invalid `--python` argument: `none` does not point to a Python executable or a directory on disk "); }); diff --git a/crates/ruff_graph/src/db.rs b/crates/ruff_graph/src/db.rs index 2e84254f36..89eb974cc8 100644 --- a/crates/ruff_graph/src/db.rs +++ b/crates/ruff_graph/src/db.rs @@ -10,7 +10,7 @@ use ruff_python_ast::PythonVersion; use ty_python_semantic::lint::{LintRegistry, RuleSelection}; use ty_python_semantic::{ Db, Program, ProgramSettings, PythonPath, PythonPlatform, PythonVersionSource, - PythonVersionWithSource, SearchPathSettings, default_lint_registry, + PythonVersionWithSource, SearchPathSettings, SysPrefixPathOrigin, default_lint_registry, }; static EMPTY_VENDORED: std::sync::LazyLock = std::sync::LazyLock::new(|| { @@ -37,7 +37,8 @@ impl ModuleDb { ) -> Result { let mut search_paths = SearchPathSettings::new(src_roots); if let Some(venv_path) = venv_path { - search_paths.python_path = PythonPath::from_cli_flag(venv_path); + search_paths.python_path = + PythonPath::sys_prefix(venv_path, SysPrefixPathOrigin::PythonCliFlag); } let db = Self::default(); diff --git a/crates/ty/tests/cli.rs b/crates/ty/tests/cli.rs index 201781ff9d..3bf7d972dc 100644 --- a/crates/ty/tests/cli.rs +++ b/crates/ty/tests/cli.rs @@ -918,6 +918,156 @@ fn cli_unknown_rules() -> anyhow::Result<()> { Ok(()) } +#[test] +fn python_cli_argument_virtual_environment() -> anyhow::Result<()> { + let path_to_executable = if cfg!(windows) { + "my-venv/Scripts/python.exe" + } else { + "my-venv/bin/python" + }; + + let other_venv_path = "my-venv/foo/some_other_file.txt"; + + let case = TestCase::with_files([ + ("test.py", ""), + ( + if cfg!(windows) { + "my-venv/Lib/site-packages/foo.py" + } else { + "my-venv/lib/python3.13/site-packages/foo.py" + }, + "", + ), + (path_to_executable, ""), + (other_venv_path, ""), + ])?; + + // Passing a path to the installation works + assert_cmd_snapshot!(case.command().arg("--python").arg("my-venv"), @r" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. + "); + + // And so does passing a path to the executable inside the installation + assert_cmd_snapshot!(case.command().arg("--python").arg(path_to_executable), @r" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. + "); + + // But random other paths inside the installation are rejected + assert_cmd_snapshot!(case.command().arg("--python").arg(other_venv_path), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. + ty failed + Cause: Invalid search path settings + Cause: Failed to discover the site-packages directory: Invalid `--python` argument: `/my-venv/foo/some_other_file.txt` does not point to a Python executable or a directory on disk + "); + + // And so are paths that do not exist on disk + assert_cmd_snapshot!(case.command().arg("--python").arg("not-a-directory-or-executable"), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. + ty failed + Cause: Invalid search path settings + Cause: Failed to discover the site-packages directory: Invalid `--python` argument: `/not-a-directory-or-executable` does not point to a Python executable or a directory on disk + "); + + Ok(()) +} + +#[test] +fn python_cli_argument_system_installation() -> anyhow::Result<()> { + let path_to_executable = if cfg!(windows) { + "Python3.11/python.exe" + } else { + "Python3.11/bin/python" + }; + + let case = TestCase::with_files([ + ("test.py", ""), + ( + if cfg!(windows) { + "Python3.11/Lib/site-packages/foo.py" + } else { + "Python3.11/lib/python3.11/site-packages/foo.py" + }, + "", + ), + (path_to_executable, ""), + ])?; + + // Passing a path to the installation works + assert_cmd_snapshot!(case.command().arg("--python").arg("Python3.11"), @r" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. + "); + + // And so does passing a path to the executable inside the installation + assert_cmd_snapshot!(case.command().arg("--python").arg(path_to_executable), @r" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. + "); + + Ok(()) +} + +#[test] +fn config_file_broken_python_setting() -> anyhow::Result<()> { + let case = TestCase::with_files([ + ( + "pyproject.toml", + r#" + [tool.ty.environment] + python = "not-a-directory-or-executable" + "#, + ), + ("test.py", ""), + ])?; + + // TODO: this error message should say "invalid `python` configuration setting" rather than "invalid `--python` argument" + assert_cmd_snapshot!(case.command(), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. + ty failed + Cause: Invalid search path settings + Cause: Failed to discover the site-packages directory: Invalid `--python` argument: `/not-a-directory-or-executable` does not point to a Python executable or a directory on disk + "); + + Ok(()) +} + #[test] fn exit_code_only_warnings() -> anyhow::Result<()> { let case = TestCase::with_file("test.py", r"print(x) # [unresolved-reference]")?; diff --git a/crates/ty_project/src/metadata/options.rs b/crates/ty_project/src/metadata/options.rs index a9a52a4b07..4571655ce3 100644 --- a/crates/ty_project/src/metadata/options.rs +++ b/crates/ty_project/src/metadata/options.rs @@ -12,7 +12,7 @@ use thiserror::Error; use ty_python_semantic::lint::{GetLintError, Level, LintSource, RuleSelection}; use ty_python_semantic::{ ProgramSettings, PythonPath, PythonPlatform, PythonVersionFileSource, PythonVersionSource, - PythonVersionWithSource, SearchPathSettings, + PythonVersionWithSource, SearchPathSettings, SysPrefixPathOrigin, }; use super::settings::{Settings, TerminalSettings}; @@ -182,19 +182,27 @@ impl Options { custom_typeshed: typeshed.map(|path| path.absolute(project_root, system)), python_path: python .map(|python_path| { - PythonPath::from_cli_flag(python_path.absolute(project_root, system)) + PythonPath::sys_prefix( + python_path.absolute(project_root, system), + SysPrefixPathOrigin::PythonCliFlag, + ) }) .or_else(|| { - std::env::var("VIRTUAL_ENV") - .ok() - .map(PythonPath::from_virtual_env_var) + std::env::var("VIRTUAL_ENV").ok().map(|virtual_env| { + PythonPath::sys_prefix(virtual_env, SysPrefixPathOrigin::VirtualEnvVar) + }) }) .or_else(|| { - std::env::var("CONDA_PREFIX") - .ok() - .map(PythonPath::from_conda_prefix_var) + std::env::var("CONDA_PREFIX").ok().map(|path| { + PythonPath::sys_prefix(path, SysPrefixPathOrigin::CondaPrefixVar) + }) }) - .unwrap_or_else(|| PythonPath::Discover(project_root.to_path_buf())), + .unwrap_or_else(|| { + PythonPath::sys_prefix( + project_root.to_path_buf(), + SysPrefixPathOrigin::LocalVenv, + ) + }), } } diff --git a/crates/ty_python_semantic/src/module_resolver/resolver.rs b/crates/ty_python_semantic/src/module_resolver/resolver.rs index 74cc931f89..d5088828f9 100644 --- a/crates/ty_python_semantic/src/module_resolver/resolver.rs +++ b/crates/ty_python_semantic/src/module_resolver/resolver.rs @@ -139,15 +139,6 @@ pub(crate) fn search_paths(db: &dyn Db) -> SearchPathIterator { Program::get(db).search_paths(db).iter(db) } -/// Searches for a `.venv` directory in `project_root` that contains a `pyvenv.cfg` file. -fn discover_venv_in(system: &dyn System, project_root: &SystemPath) -> Option { - let virtual_env_directory = project_root.join(".venv"); - - system - .is_file(&virtual_env_directory.join("pyvenv.cfg")) - .then_some(virtual_env_directory) -} - #[derive(Debug, PartialEq, Eq)] pub struct SearchPaths { /// Search paths that have been statically determined purely from reading Ruff's configuration settings. @@ -243,68 +234,34 @@ impl SearchPaths { static_paths.push(stdlib_path); let (site_packages_paths, python_version) = match python_path { - PythonPath::SysPrefix(sys_prefix, origin) => { - tracing::debug!( - "Discovering site-packages paths from sys-prefix `{sys_prefix}` ({origin}')" - ); - // TODO: We may want to warn here if the venv's python version is older - // than the one resolved in the program settings because it indicates - // that the `target-version` is incorrectly configured or that the - // venv is out of date. - PythonEnvironment::new(sys_prefix, *origin, system)?.into_settings(system)? - } + PythonPath::IntoSysPrefix(path, origin) => { + if *origin == SysPrefixPathOrigin::LocalVenv { + tracing::debug!("Discovering virtual environment in `{path}`"); + let virtual_env_directory = path.join(".venv"); - PythonPath::Resolve(target, origin) => { - tracing::debug!("Resolving {origin}: {target}"); - - let root = system - // If given a file, assume it's a Python executable, e.g., `.venv/bin/python3`, - // and search for a virtual environment in the root directory. Ideally, we'd - // invoke the target to determine `sys.prefix` here, but that's more complicated - // and may be deferred to uv. - .is_file(target) - .then(|| target.as_path()) - .take_if(|target| { - // Avoid using the target if it doesn't look like a Python executable, e.g., - // to deny cases like `.venv/bin/foo` - target - .file_name() - .is_some_and(|name| name.starts_with("python")) - }) - .and_then(SystemPath::parent) - .and_then(SystemPath::parent) - // If not a file, use the path as given and allow let `PythonEnvironment::new` - // handle the error. - .unwrap_or(target); - - PythonEnvironment::new(root, *origin, system)?.into_settings(system)? - } - - PythonPath::Discover(root) => { - tracing::debug!("Discovering virtual environment in `{root}`"); - discover_venv_in(db.system(), root) - .and_then(|virtual_env_path| { - tracing::debug!("Found `.venv` folder at `{}`", virtual_env_path); - - PythonEnvironment::new( - virtual_env_path.clone(), - SysPrefixPathOrigin::LocalVenv, - system, - ) - .and_then(|env| env.into_settings(system)) - .inspect_err(|err| { + PythonEnvironment::new( + &virtual_env_directory, + SysPrefixPathOrigin::LocalVenv, + system, + ) + .and_then(|venv| venv.into_settings(system)) + .inspect_err(|err| { + if system.is_directory(&virtual_env_directory) { tracing::debug!( "Ignoring automatically detected virtual environment at `{}`: {}", - virtual_env_path, + &virtual_env_directory, err ); - }) - .ok() + } }) - .unwrap_or_else(|| { + .unwrap_or_else(|_| { tracing::debug!("No virtual environment found"); (SitePackagesPaths::default(), None) }) + } else { + tracing::debug!("Resolving {origin}: {path}"); + PythonEnvironment::new(path, *origin, system)?.into_settings(system)? + } } PythonPath::KnownSitePackages(paths) => ( diff --git a/crates/ty_python_semantic/src/program.rs b/crates/ty_python_semantic/src/program.rs index 6057b19419..b968c57bd0 100644 --- a/crates/ty_python_semantic/src/program.rs +++ b/crates/ty_python_semantic/src/program.rs @@ -262,8 +262,10 @@ impl SearchPathSettings { #[derive(Debug, Clone, Eq, PartialEq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub enum PythonPath { - /// A path that represents the value of [`sys.prefix`] at runtime in Python - /// for a given Python executable. + /// A path that either represents the value of [`sys.prefix`] at runtime in Python + /// for a given Python executable, or which represents a path relative to `sys.prefix` + /// that we will attempt later to resolve into `sys.prefix`. Exactly which this variant + /// represents depends on the [`SysPrefixPathOrigin`] element in the tuple. /// /// For the case of a virtual environment, where a /// Python binary is at `/.venv/bin/python`, `sys.prefix` is the path to @@ -275,13 +277,7 @@ pub enum PythonPath { /// `/opt/homebrew/lib/python3.X/site-packages`. /// /// [`sys.prefix`]: https://docs.python.org/3/library/sys.html#sys.prefix - SysPrefix(SystemPathBuf, SysPrefixPathOrigin), - - /// Resolve a path to an executable (or environment directory) into a usable environment. - Resolve(SystemPathBuf, SysPrefixPathOrigin), - - /// Tries to discover a virtual environment in the given path. - Discover(SystemPathBuf), + IntoSysPrefix(SystemPathBuf, SysPrefixPathOrigin), /// Resolved site packages paths. /// @@ -291,16 +287,8 @@ pub enum PythonPath { } impl PythonPath { - pub fn from_virtual_env_var(path: impl Into) -> Self { - Self::SysPrefix(path.into(), SysPrefixPathOrigin::VirtualEnvVar) - } - - pub fn from_conda_prefix_var(path: impl Into) -> Self { - Self::Resolve(path.into(), SysPrefixPathOrigin::CondaPrefixVar) - } - - pub fn from_cli_flag(path: SystemPathBuf) -> Self { - Self::Resolve(path, SysPrefixPathOrigin::PythonCliFlag) + pub fn sys_prefix(path: impl Into, origin: SysPrefixPathOrigin) -> Self { + Self::IntoSysPrefix(path.into(), origin) } } diff --git a/crates/ty_python_semantic/src/site_packages.rs b/crates/ty_python_semantic/src/site_packages.rs index 8548430492..677ab75880 100644 --- a/crates/ty_python_semantic/src/site_packages.rs +++ b/crates/ty_python_semantic/src/site_packages.rs @@ -536,10 +536,18 @@ pub(crate) enum SitePackagesDiscoveryError { #[error("Invalid {1}: `{0}` could not be canonicalized")] CanonicalizationError(SystemPathBuf, SysPrefixPathOrigin, #[source] io::Error), - /// `site-packages` discovery failed because the [`SysPrefixPathOrigin`] indicated that - /// the provided path should point to `sys.prefix` directly, but the path wasn't a directory. - #[error("Invalid {1}: `{0}` does not point to a directory on disk")] - SysPrefixNotADirectory(SystemPathBuf, SysPrefixPathOrigin), + /// `site-packages` discovery failed because the provided path doesn't appear to point to + /// a Python executable or a `sys.prefix` directory. + #[error( + "Invalid {1}: `{0}` does not point to a {thing}", + + thing = if .1.must_point_directly_to_sys_prefix() { + "directory on disk" + } else { + "Python executable or a directory on disk" + } + )] + PathNotExecutableOrDirectory(SystemPathBuf, SysPrefixPathOrigin), /// `site-packages` discovery failed because the [`SysPrefixPathOrigin`] indicated that /// the provided path should point to the `sys.prefix` of a virtual environment, @@ -738,24 +746,79 @@ impl SysPrefixPath { let canonicalized = system .canonicalize_path(unvalidated_path) .map_err(|io_err| { - SitePackagesDiscoveryError::CanonicalizationError( - unvalidated_path.to_path_buf(), - origin, - io_err, - ) + let unvalidated_path = unvalidated_path.to_path_buf(); + if io_err.kind() == io::ErrorKind::NotFound { + SitePackagesDiscoveryError::PathNotExecutableOrDirectory( + unvalidated_path, + origin, + ) + } else { + SitePackagesDiscoveryError::CanonicalizationError( + unvalidated_path, + origin, + io_err, + ) + } })?; - system - .is_directory(&canonicalized) - .then_some(Self { - inner: canonicalized, - origin, - }) - .ok_or_else(|| { - SitePackagesDiscoveryError::SysPrefixNotADirectory( + + if origin.must_point_directly_to_sys_prefix() { + return system + .is_directory(&canonicalized) + .then_some(Self { + inner: canonicalized, + origin, + }) + .ok_or_else(|| { + SitePackagesDiscoveryError::PathNotExecutableOrDirectory( + unvalidated_path.to_path_buf(), + origin, + ) + }); + } + + let sys_prefix = if system.is_file(&canonicalized) + && canonicalized + .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. + 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. + // System installations have their executable at `/python.exe`, + // whereas virtual environments have their executable at `/Scripts/python.exe`. + canonicalized.parent().and_then(|parent| { + if parent.file_name() == Some("Scripts") { + parent.parent() + } else { + Some(parent) + } + }) + } else { + // On Unix, `sys.prefix` is always the grandparent directory of the Python executable, + // regardless of whether it's a virtual environment or a system installation. + canonicalized.ancestors().nth(2) + }; + sys_prefix.map(SystemPath::to_path_buf).ok_or_else(|| { + SitePackagesDiscoveryError::PathNotExecutableOrDirectory( unvalidated_path.to_path_buf(), origin, ) - }) + })? + } else if system.is_directory(&canonicalized) { + canonicalized + } else { + return Err(SitePackagesDiscoveryError::PathNotExecutableOrDirectory( + unvalidated_path.to_path_buf(), + origin, + )); + }; + + Ok(Self { + inner: sys_prefix, + origin, + }) } fn from_executable_home_path(path: &PythonHomePath) -> Option { @@ -812,12 +875,26 @@ pub enum SysPrefixPathOrigin { impl SysPrefixPathOrigin { /// Whether the given `sys.prefix` path must be a virtual environment (rather than a system /// Python environment). - pub(crate) fn must_be_virtual_env(self) -> bool { + pub(crate) const fn must_be_virtual_env(self) -> bool { match self { Self::LocalVenv | Self::VirtualEnvVar => true, Self::PythonCliFlag | Self::DerivedFromPyvenvCfg | Self::CondaPrefixVar => false, } } + + /// Whether paths with this origin always point directly to the `sys.prefix` directory. + /// + /// Some variants can point either directly to `sys.prefix` or to a Python executable inside + /// 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 => false, + Self::VirtualEnvVar + | Self::CondaPrefixVar + | Self::DerivedFromPyvenvCfg + | Self::LocalVenv => true, + } + } } impl Display for SysPrefixPathOrigin { @@ -1378,7 +1455,7 @@ mod tests { let system = TestSystem::default(); assert!(matches!( PythonEnvironment::new("/env", SysPrefixPathOrigin::PythonCliFlag, &system), - Err(SitePackagesDiscoveryError::CanonicalizationError(..)) + Err(SitePackagesDiscoveryError::PathNotExecutableOrDirectory(..)) )); } @@ -1391,7 +1468,7 @@ mod tests { .unwrap(); assert!(matches!( PythonEnvironment::new("/env", SysPrefixPathOrigin::PythonCliFlag, &system), - Err(SitePackagesDiscoveryError::SysPrefixNotADirectory(..)) + Err(SitePackagesDiscoveryError::PathNotExecutableOrDirectory(..)) )); } diff --git a/crates/ty_test/src/lib.rs b/crates/ty_test/src/lib.rs index 234c894725..ba24781106 100644 --- a/crates/ty_test/src/lib.rs +++ b/crates/ty_test/src/lib.rs @@ -272,7 +272,7 @@ fn run_test( python_path: configuration .python() .map(|sys_prefix| { - PythonPath::SysPrefix( + PythonPath::IntoSysPrefix( sys_prefix.to_path_buf(), SysPrefixPathOrigin::PythonCliFlag, )