[ty] Fix --python argument for Windows, and improve error messages for bad --python arguments (#18457)

## Summary

Fixes https://github.com/astral-sh/ty/issues/556.

On Windows, system installations have different layouts to virtual
environments. In Windows virtual environments, the Python executable is
found at `<sys.prefix>/Scripts/python.exe`. But in Windows system
installations, the Python executable is found at
`<sys.prefix>/python.exe`. That means that Windows users were able to
point to Python executables inside virtual environments with the
`--python` flag, but they weren't able to point to Python executables
inside system installations.

This PR fixes that issue. It also makes a couple of other changes:
- Nearly all `sys.prefix` resolution is moved inside `site_packages.rs`.
That was the original design of the `site-packages` resolution logic,
but features implemented since the initial implementation have added
some resolution and validation to `resolver.rs` inside the module
resolver. That means that we've ended up with a somewhat confusing code
structure and a situation where several checks are unnecessarily
duplicated between the two modules.
- I noticed that we had quite bad error messages if you e.g. pointed to
a path that didn't exist on disk with `--python` (we just gave a
somewhat impenetrable message saying that we "failed to canonicalize"
the path). I improved the error messages here and added CLI tests for
`--python` and the `environment.python` configuration setting.

## Test Plan

- Existing tests pass
- Added new CLI tests
- I manually checked that virtual-environment discovery still works if
no configuration is given
- Micha did some manual testing to check that pointing `--python` to a
system-installation executable now works on Windows

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
         ");
     });
 

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<VendoredFileSystem> = std::sync::LazyLock::new(|| {
@@ -37,7 +37,8 @@ impl ModuleDb {
     ) -> Result<Self> {
         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();

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: `<temp_dir>/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: `<temp_dir>/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: `<temp_dir>/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]")?;

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,
+                    )
+                }),
         }
     }
 

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<SystemPathBuf> {
-    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) => (

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<SystemPathBuf>) -> Self {
-        Self::SysPrefix(path.into(), SysPrefixPathOrigin::VirtualEnvVar)
-    }
-
-    pub fn from_conda_prefix_var(path: impl Into<SystemPathBuf>) -> 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<SystemPathBuf>, origin: SysPrefixPathOrigin) -> Self {
+        Self::IntoSysPrefix(path.into(), origin)
     }
 }
 

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 `<sys.prefix>/python.exe`,
+                // whereas virtual environments have their executable at `<sys.prefix>/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<Self> {
@@ -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(..))
         ));
     }
 

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,
                     )