diff --git a/Cargo.lock b/Cargo.lock index 136af27f59b03f..aaf3bc7aaed0f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3872,6 +3872,7 @@ dependencies = [ "countme", "crossbeam", "ctrlc", + "dunce", "filetime", "indicatif", "insta", diff --git a/crates/ruff_graph/src/db.rs b/crates/ruff_graph/src/db.rs index 06b3eca96cbeb4..2e84254f366a19 100644 --- a/crates/ruff_graph/src/db.rs +++ b/crates/ruff_graph/src/db.rs @@ -44,10 +44,10 @@ impl ModuleDb { Program::from_settings( &db, ProgramSettings { - python_version: PythonVersionWithSource { + python_version: Some(PythonVersionWithSource { version: python_version, source: PythonVersionSource::default(), - }, + }), python_platform: PythonPlatform::default(), search_paths, }, diff --git a/crates/ty/Cargo.toml b/crates/ty/Cargo.toml index cd8f7b32ea5bc0..6cfc036f29f753 100644 --- a/crates/ty/Cargo.toml +++ b/crates/ty/Cargo.toml @@ -41,6 +41,7 @@ wild = { workspace = true } ruff_db = { workspace = true, features = ["testing"] } ruff_python_trivia = { workspace = true } +dunce = { workspace = true } insta = { workspace = true, features = ["filters"] } insta-cmd = { workspace = true } filetime = { workspace = true } diff --git a/crates/ty/tests/cli.rs b/crates/ty/tests/cli.rs index a2892e4f71ce0f..201781ff9d9f72 100644 --- a/crates/ty/tests/cli.rs +++ b/crates/ty/tests/cli.rs @@ -308,6 +308,125 @@ fn config_file_annotation_showing_where_python_version_set_typing_error() -> any Ok(()) } +#[test] +fn pyvenv_cfg_file_annotation_showing_where_python_version_set() -> anyhow::Result<()> { + let case = TestCase::with_files([ + ( + "pyproject.toml", + r#" + [tool.ty.environment] + python = "venv" + "#, + ), + ( + "venv/pyvenv.cfg", + r#" + version = 3.8 + home = foo/bar/bin + "#, + ), + if cfg!(target_os = "windows") { + ("foo/bar/bin/python.exe", "") + } else { + ("foo/bar/bin/python", "") + }, + if cfg!(target_os = "windows") { + ("venv/Lib/site-packages/foo.py", "") + } else { + ("venv/lib/python3.8/site-packages/foo.py", "") + }, + ("test.py", "aiter"), + ])?; + + assert_cmd_snapshot!(case.command(), @r" + success: false + exit_code: 1 + ----- stdout ----- + error[unresolved-reference]: Name `aiter` used when not defined + --> test.py:1:1 + | + 1 | aiter + | ^^^^^ + | + info: `aiter` was added as a builtin in Python 3.10 + info: Python 3.8 was assumed when resolving types because of your virtual environment + --> venv/pyvenv.cfg:2:11 + | + 2 | version = 3.8 + | ^^^ Python version inferred from virtual environment metadata file + 3 | home = foo/bar/bin + | + info: No Python version was specified on the command line or in a configuration file + info: rule `unresolved-reference` is enabled by default + + Found 1 diagnostic + + ----- 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 pyvenv_cfg_file_annotation_no_trailing_newline() -> anyhow::Result<()> { + let case = TestCase::with_files([ + ( + "pyproject.toml", + r#" + [tool.ty.environment] + python = "venv" + "#, + ), + ( + "venv/pyvenv.cfg", + r#"home = foo/bar/bin + + + version = 3.8"#, + ), + if cfg!(target_os = "windows") { + ("foo/bar/bin/python.exe", "") + } else { + ("foo/bar/bin/python", "") + }, + if cfg!(target_os = "windows") { + ("venv/Lib/site-packages/foo.py", "") + } else { + ("venv/lib/python3.8/site-packages/foo.py", "") + }, + ("test.py", "aiter"), + ])?; + + assert_cmd_snapshot!(case.command(), @r" + success: false + exit_code: 1 + ----- stdout ----- + error[unresolved-reference]: Name `aiter` used when not defined + --> test.py:1:1 + | + 1 | aiter + | ^^^^^ + | + info: `aiter` was added as a builtin in Python 3.10 + info: Python 3.8 was assumed when resolving types because of your virtual environment + --> venv/pyvenv.cfg:4:23 + | + 4 | version = 3.8 + | ^^^ Python version inferred from virtual environment metadata file + | + info: No Python version was specified on the command line or in a configuration file + info: rule `unresolved-reference` is enabled by default + + Found 1 diagnostic + + ----- 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_annotation_showing_where_python_version_set_syntax_error() -> anyhow::Result<()> { let case = TestCase::with_files([ @@ -1772,10 +1891,14 @@ impl TestCase { // Canonicalize the tempdir path because macos uses symlinks for tempdirs // and that doesn't play well with our snapshot filtering. - let project_dir = temp_dir - .path() - .canonicalize() - .context("Failed to canonicalize project path")?; + // Simplify with dunce because otherwise we get UNC paths on Windows. + let project_dir = dunce::simplified( + &temp_dir + .path() + .canonicalize() + .context("Failed to canonicalize project path")?, + ) + .to_path_buf(); let mut settings = insta::Settings::clone_current(); settings.add_filter(&tempdir_filter(&project_dir), "/"); diff --git a/crates/ty_ide/src/inlay_hints.rs b/crates/ty_ide/src/inlay_hints.rs index 4539b54308bef2..0ef71ddc05eff1 100644 --- a/crates/ty_ide/src/inlay_hints.rs +++ b/crates/ty_ide/src/inlay_hints.rs @@ -191,7 +191,7 @@ mod tests { Program::from_settings( &db, ProgramSettings { - python_version: PythonVersionWithSource::default(), + python_version: Some(PythonVersionWithSource::default()), python_platform: PythonPlatform::default(), search_paths: SearchPathSettings { extra_paths: vec![], diff --git a/crates/ty_ide/src/lib.rs b/crates/ty_ide/src/lib.rs index e403996788d1d4..99e3c123cf7b59 100644 --- a/crates/ty_ide/src/lib.rs +++ b/crates/ty_ide/src/lib.rs @@ -230,7 +230,7 @@ mod tests { Program::from_settings( &db, ProgramSettings { - python_version: PythonVersionWithSource::default(), + python_version: Some(PythonVersionWithSource::default()), python_platform: PythonPlatform::default(), search_paths: SearchPathSettings { extra_paths: vec![], diff --git a/crates/ty_project/src/lib.rs b/crates/ty_project/src/lib.rs index 6c7b95c256fe05..ecd9501a47c5d6 100644 --- a/crates/ty_project/src/lib.rs +++ b/crates/ty_project/src/lib.rs @@ -677,7 +677,7 @@ mod tests { Program::from_settings( &db, ProgramSettings { - python_version: PythonVersionWithSource::default(), + python_version: Some(PythonVersionWithSource::default()), python_platform: PythonPlatform::default(), search_paths: SearchPathSettings::new(vec![SystemPathBuf::from(".")]), }, diff --git a/crates/ty_project/src/metadata/options.rs b/crates/ty_project/src/metadata/options.rs index 2e089d334f2fd4..a9a52a4b078097 100644 --- a/crates/ty_project/src/metadata/options.rs +++ b/crates/ty_project/src/metadata/options.rs @@ -11,8 +11,8 @@ use std::fmt::Debug; use thiserror::Error; use ty_python_semantic::lint::{GetLintError, Level, LintSource, RuleSelection}; use ty_python_semantic::{ - ProgramSettings, PythonPath, PythonPlatform, PythonVersionSource, PythonVersionWithSource, - SearchPathSettings, + ProgramSettings, PythonPath, PythonPlatform, PythonVersionFileSource, PythonVersionSource, + PythonVersionWithSource, SearchPathSettings, }; use super::settings::{Settings, TerminalSettings}; @@ -88,12 +88,11 @@ impl Options { version: **ranged_version, source: match ranged_version.source() { ValueSource::Cli => PythonVersionSource::Cli, - ValueSource::File(path) => { - PythonVersionSource::File(path.clone(), ranged_version.range()) - } + ValueSource::File(path) => PythonVersionSource::ConfigFile( + PythonVersionFileSource::new(path.clone(), ranged_version.range()), + ), }, - }) - .unwrap_or_default(); + }); let python_platform = self .environment .as_ref() diff --git a/crates/ty_python_semantic/src/db.rs b/crates/ty_python_semantic/src/db.rs index 99e560905b6d78..11c6e2922072ac 100644 --- a/crates/ty_python_semantic/src/db.rs +++ b/crates/ty_python_semantic/src/db.rs @@ -182,10 +182,10 @@ pub(crate) mod tests { Program::from_settings( &db, ProgramSettings { - python_version: PythonVersionWithSource { + python_version: Some(PythonVersionWithSource { version: self.python_version, source: PythonVersionSource::default(), - }, + }), python_platform: self.python_platform, search_paths: SearchPathSettings::new(vec![src_root]), }, diff --git a/crates/ty_python_semantic/src/lib.rs b/crates/ty_python_semantic/src/lib.rs index 11dfbae3a03aa0..1a204734c0f55f 100644 --- a/crates/ty_python_semantic/src/lib.rs +++ b/crates/ty_python_semantic/src/lib.rs @@ -8,8 +8,8 @@ pub use db::Db; pub use module_name::ModuleName; pub use module_resolver::{KnownModule, Module, resolve_module, system_module_search_paths}; pub use program::{ - Program, ProgramSettings, PythonPath, PythonVersionSource, PythonVersionWithSource, - SearchPathSettings, + Program, ProgramSettings, PythonPath, PythonVersionFileSource, PythonVersionSource, + PythonVersionWithSource, SearchPathSettings, }; pub use python_platform::PythonPlatform; pub use semantic_model::{HasType, SemanticModel}; diff --git a/crates/ty_python_semantic/src/module_resolver/resolver.rs b/crates/ty_python_semantic/src/module_resolver/resolver.rs index 4013b54da12dc1..6447c203cfd04c 100644 --- a/crates/ty_python_semantic/src/module_resolver/resolver.rs +++ b/crates/ty_python_semantic/src/module_resolver/resolver.rs @@ -14,10 +14,8 @@ use ruff_python_ast::PythonVersion; use crate::db::Db; use crate::module_name::ModuleName; use crate::module_resolver::typeshed::{TypeshedVersions, vendored_typeshed_versions}; -use crate::site_packages::{ - PythonEnvironment, SitePackagesDiscoveryError, SitePackagesPaths, SysPrefixPathOrigin, -}; -use crate::{Program, PythonPath, SearchPathSettings}; +use crate::site_packages::{PythonEnvironment, SitePackagesPaths, SysPrefixPathOrigin}; +use crate::{Program, PythonPath, PythonVersionWithSource, SearchPathSettings}; use super::module::{Module, ModuleKind}; use super::path::{ModulePath, SearchPath, SearchPathValidationError}; @@ -165,6 +163,11 @@ pub struct SearchPaths { site_packages: Vec, typeshed_versions: TypeshedVersions, + + /// The Python version for the search paths, if any. + /// + /// This is read from the `pyvenv.cfg` if present. + python_version: Option, } impl SearchPaths { @@ -239,7 +242,7 @@ impl SearchPaths { static_paths.push(stdlib_path); - let site_packages_paths = match python_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}')" @@ -248,8 +251,7 @@ impl SearchPaths { // 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) - .and_then(|env| env.site_packages_directories(system))? + PythonEnvironment::new(sys_prefix, *origin, system)?.into_settings(system)? } PythonPath::Resolve(target, origin) => { @@ -275,45 +277,43 @@ impl SearchPaths { // handle the error. .unwrap_or(target); - PythonEnvironment::new(root, *origin, system) - .and_then(|venv| venv.site_packages_directories(system))? + PythonEnvironment::new(root, *origin, system)?.into_settings(system)? } PythonPath::Discover(root) => { tracing::debug!("Discovering virtual environment in `{root}`"); - let virtual_env_path = discover_venv_in(db.system(), root); - if let Some(virtual_env_path) = virtual_env_path { - tracing::debug!("Found `.venv` folder at `{}`", virtual_env_path); - - let handle_invalid_virtual_env = |error: SitePackagesDiscoveryError| { - tracing::debug!( - "Ignoring automatically detected virtual environment at `{}`: {}", - virtual_env_path, - error - ); - SitePackagesPaths::default() - }; - - match PythonEnvironment::new( - virtual_env_path.clone(), - SysPrefixPathOrigin::LocalVenv, - system, - ) { - Ok(venv) => venv - .site_packages_directories(system) - .unwrap_or_else(handle_invalid_virtual_env), - Err(error) => handle_invalid_virtual_env(error), - } - } else { - tracing::debug!("No virtual environment found"); - SitePackagesPaths::default() - } + 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| { + tracing::debug!( + "Ignoring automatically detected virtual environment at `{}`: {}", + virtual_env_path, + err + ); + }) + .ok() + }) + .unwrap_or_else(|| { + tracing::debug!("No virtual environment found"); + (SitePackagesPaths::default(), None) + }) } - PythonPath::KnownSitePackages(paths) => paths - .iter() - .map(|path| canonicalize(path, system)) - .collect(), + PythonPath::KnownSitePackages(paths) => ( + paths + .iter() + .map(|path| canonicalize(path, system)) + .collect(), + None, + ), }; let mut site_packages: Vec<_> = Vec::with_capacity(site_packages_paths.len()); @@ -347,6 +347,7 @@ impl SearchPaths { static_paths, site_packages, typeshed_versions, + python_version, }) } @@ -371,6 +372,10 @@ impl SearchPaths { pub(super) fn typeshed_versions(&self) -> &TypeshedVersions { &self.typeshed_versions } + + pub fn python_version(&self) -> Option<&PythonVersionWithSource> { + self.python_version.as_ref() + } } /// Collect all dynamic search paths. For each `site-packages` path: @@ -389,6 +394,7 @@ pub(crate) fn dynamic_resolution_paths(db: &dyn Db) -> Vec { static_paths, site_packages, typeshed_versions: _, + python_version: _, } = Program::get(db).search_paths(db); let mut dynamic_paths = Vec::new(); @@ -1472,10 +1478,10 @@ mod tests { Program::from_settings( &db, ProgramSettings { - python_version: PythonVersionWithSource { + python_version: Some(PythonVersionWithSource { version: PythonVersion::PY38, source: PythonVersionSource::default(), - }, + }), python_platform: PythonPlatform::default(), search_paths: SearchPathSettings { extra_paths: vec![], @@ -1991,7 +1997,7 @@ not_a_directory Program::from_settings( &db, ProgramSettings { - python_version: PythonVersionWithSource::default(), + python_version: Some(PythonVersionWithSource::default()), python_platform: PythonPlatform::default(), search_paths: SearchPathSettings { extra_paths: vec![], @@ -2070,7 +2076,7 @@ not_a_directory Program::from_settings( &db, ProgramSettings { - python_version: PythonVersionWithSource::default(), + python_version: Some(PythonVersionWithSource::default()), python_platform: PythonPlatform::default(), search_paths: SearchPathSettings { extra_paths: vec![], @@ -2113,7 +2119,7 @@ not_a_directory Program::from_settings( &db, ProgramSettings { - python_version: PythonVersionWithSource::default(), + python_version: Some(PythonVersionWithSource::default()), python_platform: PythonPlatform::default(), search_paths: SearchPathSettings { extra_paths: vec![], diff --git a/crates/ty_python_semantic/src/module_resolver/testing.rs b/crates/ty_python_semantic/src/module_resolver/testing.rs index 5f1e9c4b6e7ef5..57f366c28c9a01 100644 --- a/crates/ty_python_semantic/src/module_resolver/testing.rs +++ b/crates/ty_python_semantic/src/module_resolver/testing.rs @@ -237,10 +237,10 @@ impl TestCaseBuilder { Program::from_settings( &db, ProgramSettings { - python_version: PythonVersionWithSource { + python_version: Some(PythonVersionWithSource { version: python_version, source: PythonVersionSource::default(), - }, + }), python_platform, search_paths: SearchPathSettings { extra_paths: vec![], @@ -298,10 +298,10 @@ impl TestCaseBuilder { Program::from_settings( &db, ProgramSettings { - python_version: PythonVersionWithSource { + python_version: Some(PythonVersionWithSource { version: python_version, source: PythonVersionSource::default(), - }, + }), python_platform, search_paths: SearchPathSettings { python_path: PythonPath::KnownSitePackages(vec![site_packages.clone()]), diff --git a/crates/ty_python_semantic/src/program.rs b/crates/ty_python_semantic/src/program.rs index ef059abff96388..6057b19419768b 100644 --- a/crates/ty_python_semantic/src/program.rs +++ b/crates/ty_python_semantic/src/program.rs @@ -6,6 +6,8 @@ use crate::python_platform::PythonPlatform; use crate::site_packages::SysPrefixPathOrigin; use anyhow::Context; +use ruff_db::diagnostic::Span; +use ruff_db::files::system_path_to_file; use ruff_db::system::{SystemPath, SystemPathBuf}; use ruff_python_ast::PythonVersion; use ruff_text_size::TextRange; @@ -32,14 +34,17 @@ impl Program { search_paths, } = settings; + let search_paths = SearchPaths::from_settings(db, &search_paths) + .with_context(|| "Invalid search path settings")?; + + let python_version_with_source = + Self::resolve_python_version(python_version_with_source, search_paths.python_version()); + tracing::info!( "Python version: Python {python_version}, platform: {python_platform}", python_version = python_version_with_source.version ); - let search_paths = SearchPaths::from_settings(db, &search_paths) - .with_context(|| "Invalid search path settings")?; - Ok( Program::builder(python_version_with_source, python_platform, search_paths) .durability(Durability::HIGH) @@ -51,32 +56,54 @@ impl Program { self.python_version_with_source(db).version } + fn resolve_python_version( + config_value: Option, + environment_value: Option<&PythonVersionWithSource>, + ) -> PythonVersionWithSource { + config_value + .or_else(|| environment_value.cloned()) + .unwrap_or_default() + } + pub fn update_from_settings( self, db: &mut dyn Db, settings: ProgramSettings, ) -> anyhow::Result<()> { let ProgramSettings { - python_version, + python_version: python_version_with_source, python_platform, search_paths, } = settings; + let search_paths = SearchPaths::from_settings(db, &search_paths)?; + + let new_python_version = + Self::resolve_python_version(python_version_with_source, search_paths.python_version()); + + if self.search_paths(db) != &search_paths { + tracing::debug!("Updating search paths"); + self.set_search_paths(db).to(search_paths); + } + if &python_platform != self.python_platform(db) { tracing::debug!("Updating python platform: `{python_platform:?}`"); self.set_python_platform(db).to(python_platform); } - if &python_version != self.python_version_with_source(db) { - tracing::debug!("Updating python version: `{python_version:?}`"); - self.set_python_version_with_source(db).to(python_version); + if &new_python_version != self.python_version_with_source(db) { + tracing::debug!( + "Updating python version: Python {version}", + version = new_python_version.version + ); + self.set_python_version_with_source(db) + .to(new_python_version); } - self.update_search_paths(db, &search_paths)?; - Ok(()) } + /// Update the search paths for the program. pub fn update_search_paths( self, db: &mut dyn Db, @@ -84,8 +111,21 @@ impl Program { ) -> anyhow::Result<()> { let search_paths = SearchPaths::from_settings(db, search_path_settings)?; + let current_python_version = self.python_version_with_source(db); + let python_version_from_environment = + search_paths.python_version().cloned().unwrap_or_default(); + + if current_python_version != &python_version_from_environment + && current_python_version.source.priority() + <= python_version_from_environment.source.priority() + { + tracing::debug!("Updating Python version from environment"); + self.set_python_version_with_source(db) + .to(python_version_from_environment); + } + if self.search_paths(db) != &search_paths { - tracing::debug!("Update search paths"); + tracing::debug!("Updating search paths"); self.set_search_paths(db).to(search_paths); } @@ -99,7 +139,7 @@ impl Program { #[derive(Clone, Debug, Eq, PartialEq)] pub struct ProgramSettings { - pub python_version: PythonVersionWithSource, + pub python_version: Option, pub python_platform: PythonPlatform, pub search_paths: SearchPathSettings, } @@ -107,7 +147,11 @@ pub struct ProgramSettings { #[derive(Clone, Debug, Eq, PartialEq, Default)] pub enum PythonVersionSource { /// Value loaded from a project's configuration file. - File(Arc, Option), + ConfigFile(PythonVersionFileSource), + + /// Value loaded from the `pyvenv.cfg` file of the virtual environment. + /// The virtual environment might have been configured, activated or inferred. + PyvenvCfgFile(PythonVersionFileSource), /// The value comes from a CLI argument, while it's left open if specified using a short argument, /// long argument (`--extra-paths`) or `--config key=value`. @@ -118,6 +162,55 @@ pub enum PythonVersionSource { Default, } +impl PythonVersionSource { + fn priority(&self) -> PythonSourcePriority { + match self { + PythonVersionSource::Default => PythonSourcePriority::Default, + PythonVersionSource::PyvenvCfgFile(_) => PythonSourcePriority::PyvenvCfgFile, + PythonVersionSource::ConfigFile(_) => PythonSourcePriority::ConfigFile, + PythonVersionSource::Cli => PythonSourcePriority::Cli, + } + } +} + +/// The priority in which Python version sources are considered. +/// A higher value means a higher priority. +/// +/// For example, if a Python version is specified in a pyproject.toml file +/// but *also* via a CLI argument, the CLI argument will take precedence. +#[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord)] +#[cfg_attr(test, derive(strum_macros::EnumIter))] +enum PythonSourcePriority { + Default = 0, + PyvenvCfgFile = 1, + ConfigFile = 2, + Cli = 3, +} + +/// Information regarding the file and [`TextRange`] of the configuration +/// from which we inferred the Python version. +#[derive(Debug, PartialEq, Eq, Clone)] +pub struct PythonVersionFileSource { + path: Arc, + range: Option, +} + +impl PythonVersionFileSource { + pub fn new(path: Arc, range: Option) -> Self { + Self { path, range } + } + + /// Attempt to resolve a [`Span`] that corresponds to the location of + /// the configuration setting that specified the Python version. + /// + /// Useful for subdiagnostics when informing the user + /// what the inferred Python version of their project is. + pub(crate) fn span(&self, db: &dyn Db) -> Option { + let file = system_path_to_file(db.upcast(), &*self.path).ok()?; + Some(Span::from(file).with_optional_range(self.range)) + } +} + #[derive(Eq, PartialEq, Debug, Clone)] pub struct PythonVersionWithSource { pub version: PythonVersion, @@ -210,3 +303,54 @@ impl PythonPath { Self::Resolve(path, SysPrefixPathOrigin::PythonCliFlag) } } + +#[cfg(test)] +mod tests { + use super::*; + use strum::IntoEnumIterator; + + #[test] + fn test_python_version_source_priority() { + for priority in PythonSourcePriority::iter() { + match priority { + // CLI source takes priority over all other sources. + PythonSourcePriority::Cli => { + for other in PythonSourcePriority::iter() { + assert!(priority >= other, "{other:?}"); + } + } + // Config files have lower priority than CLI arguments, + // but higher than pyvenv.cfg files and the fallback default. + PythonSourcePriority::ConfigFile => { + for other in PythonSourcePriority::iter() { + match other { + PythonSourcePriority::Cli => assert!(other > priority, "{other:?}"), + PythonSourcePriority::ConfigFile => assert_eq!(priority, other), + PythonSourcePriority::PyvenvCfgFile | PythonSourcePriority::Default => { + assert!(priority > other, "{other:?}"); + } + } + } + } + // Pyvenv.cfg files have lower priority than CLI flags and config files, + // but higher than the default fallback. + PythonSourcePriority::PyvenvCfgFile => { + for other in PythonSourcePriority::iter() { + match other { + PythonSourcePriority::Cli | PythonSourcePriority::ConfigFile => { + assert!(other > priority, "{other:?}"); + } + PythonSourcePriority::PyvenvCfgFile => assert_eq!(priority, other), + PythonSourcePriority::Default => assert!(priority > other, "{other:?}"), + } + } + } + PythonSourcePriority::Default => { + for other in PythonSourcePriority::iter() { + assert!(priority <= other, "{other:?}"); + } + } + } + } + } +} diff --git a/crates/ty_python_semantic/src/site_packages.rs b/crates/ty_python_semantic/src/site_packages.rs index c01db5483e8f3a..295bc188942ae3 100644 --- a/crates/ty_python_semantic/src/site_packages.rs +++ b/crates/ty_python_semantic/src/site_packages.rs @@ -8,15 +8,19 @@ //! reasonably ask us to type-check code assuming that the code runs //! on Linux.) -use std::fmt; use std::fmt::Display; use std::io; use std::num::NonZeroUsize; use std::ops::Deref; +use std::{fmt, sync::Arc}; use indexmap::IndexSet; use ruff_db::system::{System, SystemPath, SystemPathBuf}; use ruff_python_ast::PythonVersion; +use ruff_python_trivia::Cursor; +use ruff_text_size::{TextLen, TextRange}; + +use crate::{PythonVersionFileSource, PythonVersionSource, PythonVersionWithSource}; type SitePackagesDiscoveryResult = Result; @@ -108,10 +112,23 @@ impl PythonEnvironment { } } - /// Returns the `site-packages` directories for this Python environment. + /// Returns the `site-packages` directories for this Python environment, + /// as well as the Python version that was used to create this environment + /// (the latter will only be available for virtual environments that specify + /// the metadata in their `pyvenv.cfg` files). /// /// See the documentation for [`site_packages_directory_from_sys_prefix`] for more details. - pub(crate) fn site_packages_directories( + pub(crate) fn into_settings( + self, + system: &dyn System, + ) -> SitePackagesDiscoveryResult<(SitePackagesPaths, Option)> { + Ok(match self { + Self::Virtual(venv) => (venv.site_packages_directories(system)?, venv.version), + Self::System(env) => (env.site_packages_directories(system)?, None), + }) + } + + fn site_packages_directories( &self, system: &dyn System, ) -> SitePackagesDiscoveryResult { @@ -126,13 +143,14 @@ impl PythonEnvironment { /// /// We only need to distinguish cases that change the on-disk layout. /// Everything else can be treated like CPython. -#[derive(Debug, Copy, Clone, Eq, PartialEq)] +#[derive(Debug, Copy, Clone, Eq, PartialEq, Default)] pub(crate) enum PythonImplementation { CPython, PyPy, GraalPy, /// Fallback when the value is missing or unrecognised. /// We treat it like CPython but keep the information for diagnostics. + #[default] Unknown, } @@ -169,7 +187,7 @@ pub(crate) struct VirtualEnvironment { /// so it's possible that we might not be able to find this information /// in an acceptable format under any of the keys we expect. /// This field will be `None` if so. - version: Option, + version: Option, implementation: PythonImplementation, /// If this virtual environment was created using uv, @@ -186,10 +204,6 @@ impl VirtualEnvironment { path: SysPrefixPath, system: &dyn System, ) -> SitePackagesDiscoveryResult { - fn pyvenv_cfg_line_number(index: usize) -> NonZeroUsize { - index.checked_add(1).and_then(NonZeroUsize::new).unwrap() - } - let pyvenv_cfg_path = path.join("pyvenv.cfg"); tracing::debug!("Attempting to parse virtual environment metadata at '{pyvenv_cfg_path}'"); @@ -197,62 +211,24 @@ impl VirtualEnvironment { .read_to_string(&pyvenv_cfg_path) .map_err(|io_err| SitePackagesDiscoveryError::NoPyvenvCfgFile(path.origin, io_err))?; - let mut include_system_site_packages = false; - let mut base_executable_home_path = None; - let mut version_info_string = None; - let mut implementation = PythonImplementation::Unknown; - let mut created_with_uv = false; - let mut parent_environment = None; - - // A `pyvenv.cfg` file *looks* like a `.ini` file, but actually isn't valid `.ini` syntax! - // The Python standard-library's `site` module parses these files by splitting each line on - // '=' characters, so that's what we should do as well. - // - // See also: https://snarky.ca/how-virtual-environments-work/ - for (index, line) in pyvenv_cfg.lines().enumerate() { - if let Some((key, value)) = line.split_once('=') { - let key = key.trim(); - if key.is_empty() { - return Err(SitePackagesDiscoveryError::PyvenvCfgParseError( - pyvenv_cfg_path, - PyvenvCfgParseErrorKind::MalformedKeyValuePair { - line_number: pyvenv_cfg_line_number(index), - }, - )); - } - - let value = value.trim(); - if value.is_empty() { - return Err(SitePackagesDiscoveryError::PyvenvCfgParseError( - pyvenv_cfg_path, - PyvenvCfgParseErrorKind::MalformedKeyValuePair { - line_number: pyvenv_cfg_line_number(index), - }, - )); - } - - match key { - "include-system-site-packages" => { - include_system_site_packages = value.eq_ignore_ascii_case("true"); - } - "home" => base_executable_home_path = Some(value), - // `virtualenv` and `uv` call this key `version_info`, - // but the stdlib venv module calls it `version` - "version" | "version_info" => version_info_string = Some(value), - "implementation" => { - implementation = match value.to_ascii_lowercase().as_str() { - "cpython" => PythonImplementation::CPython, - "graalvm" => PythonImplementation::GraalPy, - "pypy" => PythonImplementation::PyPy, - _ => PythonImplementation::Unknown, - }; - } - "uv" => created_with_uv = true, - "extends-environment" => parent_environment = Some(value), - _ => continue, - } - } - } + let parsed_pyvenv_cfg = + PyvenvCfgParser::new(&pyvenv_cfg) + .parse() + .map_err(|pyvenv_parse_error| { + SitePackagesDiscoveryError::PyvenvCfgParseError( + pyvenv_cfg_path.clone(), + pyvenv_parse_error, + ) + })?; + + let RawPyvenvCfg { + include_system_site_packages, + base_executable_home_path, + version, + implementation, + created_with_uv, + parent_environment, + } = parsed_pyvenv_cfg; // The `home` key is read by the standard library's `site.py` module, // so if it's missing from the `pyvenv.cfg` file @@ -264,6 +240,7 @@ impl VirtualEnvironment { PyvenvCfgParseErrorKind::NoHomeKey, )); }; + let base_executable_home_path = PythonHomePath::new(base_executable_home_path, system) .map_err(|io_err| { SitePackagesDiscoveryError::PyvenvCfgParseError( @@ -298,10 +275,15 @@ impl VirtualEnvironment { // created the `pyvenv.cfg` file. Lenient parsing is appropriate here: // the file isn't really *invalid* if it doesn't have this key, // or if the value doesn't parse according to our expectations. - let version = version_info_string.and_then(|version_string| { + let version = version.and_then(|(version_string, range)| { let mut version_info_parts = version_string.split('.'); let (major, minor) = (version_info_parts.next()?, version_info_parts.next()?); - PythonVersion::try_from((major, minor)).ok() + let version = PythonVersion::try_from((major, minor)).ok()?; + let source = PythonVersionSource::PyvenvCfgFile(PythonVersionFileSource::new( + Arc::new(pyvenv_cfg_path), + Some(range), + )); + Some(PythonVersionWithSource { version, source }) }); let metadata = Self { @@ -333,8 +315,10 @@ impl VirtualEnvironment { parent_environment, } = self; + let version = version.as_ref().map(|v| v.version); + let mut site_packages_directories = SitePackagesPaths::single( - site_packages_directory_from_sys_prefix(root_path, *version, *implementation, system)?, + site_packages_directory_from_sys_prefix(root_path, version, *implementation, system)?, ); if let Some(parent_env_site_packages) = parent_environment.as_deref() { @@ -362,7 +346,7 @@ impl VirtualEnvironment { if let Some(sys_prefix_path) = system_sys_prefix { match site_packages_directory_from_sys_prefix( &sys_prefix_path, - *version, + version, *implementation, system, ) { @@ -390,6 +374,119 @@ System site-packages will not be used for module resolution.", } } +/// A parser for `pyvenv.cfg` files: metadata files for virtual environments. +/// +/// Note that a `pyvenv.cfg` file *looks* like a `.ini` file, but actually isn't valid `.ini` syntax! +/// +/// See also: +#[derive(Debug)] +struct PyvenvCfgParser<'s> { + source: &'s str, + cursor: Cursor<'s>, + line_number: NonZeroUsize, + data: RawPyvenvCfg<'s>, +} + +impl<'s> PyvenvCfgParser<'s> { + fn new(source: &'s str) -> Self { + Self { + source, + cursor: Cursor::new(source), + line_number: NonZeroUsize::new(1).unwrap(), + data: RawPyvenvCfg::default(), + } + } + + /// Parse the `pyvenv.cfg` file and return the parsed data. + fn parse(mut self) -> Result, PyvenvCfgParseErrorKind> { + while !self.cursor.is_eof() { + self.parse_line()?; + self.line_number = self.line_number.checked_add(1).unwrap(); + } + Ok(self.data) + } + + /// Parse a single line of the `pyvenv.cfg` file and advance the cursor + /// to the beginning of the next line. + fn parse_line(&mut self) -> Result<(), PyvenvCfgParseErrorKind> { + let PyvenvCfgParser { + source, + cursor, + line_number, + data, + } = self; + + let line_number = *line_number; + + cursor.eat_while(|c| c.is_whitespace() && c != '\n'); + + let key_start = cursor.offset(); + cursor.eat_while(|c| !matches!(c, '\n' | '=')); + let key_end = cursor.offset(); + + if !cursor.eat_char('=') { + // Skip over any lines that do not contain '=' characters, same as the CPython stdlib + // + cursor.eat_char('\n'); + return Ok(()); + } + + let key = source[TextRange::new(key_start, key_end)].trim(); + + cursor.eat_while(|c| c.is_whitespace() && c != '\n'); + let value_start = cursor.offset(); + cursor.eat_while(|c| c != '\n'); + let value = source[TextRange::new(value_start, cursor.offset())].trim(); + cursor.eat_char('\n'); + + if value.is_empty() { + return Err(PyvenvCfgParseErrorKind::MalformedKeyValuePair { line_number }); + } + + match key { + "include-system-site-packages" => { + data.include_system_site_packages = value.eq_ignore_ascii_case("true"); + } + "home" => data.base_executable_home_path = Some(value), + // `virtualenv` and `uv` call this key `version_info`, + // but the stdlib venv module calls it `version` + "version" | "version_info" => { + let version_range = TextRange::at(value_start, value.text_len()); + data.version = Some((value, version_range)); + } + "implementation" => { + data.implementation = match value.to_ascii_lowercase().as_str() { + "cpython" => PythonImplementation::CPython, + "graalvm" => PythonImplementation::GraalPy, + "pypy" => PythonImplementation::PyPy, + _ => PythonImplementation::Unknown, + }; + } + "uv" => data.created_with_uv = true, + "extends-environment" => data.parent_environment = Some(value), + "" => { + return Err(PyvenvCfgParseErrorKind::MalformedKeyValuePair { line_number }); + } + _ => {} + } + + Ok(()) + } +} + +/// A `key:value` mapping derived from parsing a `pyvenv.cfg` file. +/// +/// This data contained within is still mostly raw and unvalidated. +#[derive(Debug, Default)] +struct RawPyvenvCfg<'s> { + include_system_site_packages: bool, + base_executable_home_path: Option<&'s str>, + version: Option<(&'s str, TextRange)>, + implementation: PythonImplementation, + created_with_uv: bool, + parent_environment: Option<&'s str>, +} + /// A Python environment that is _not_ a virtual environment. /// /// This environment may or may not be one that is managed by the operating system itself, e.g., @@ -969,7 +1066,7 @@ mod tests { if self_venv.pyvenv_cfg_version_field.is_some() { assert_eq!( - venv.version, + venv.version.as_ref().map(|v| v.version), Some(PythonVersion { major: 3, minor: self.minor_version @@ -1462,4 +1559,29 @@ mod tests { if path == pyvenv_cfg_path )); } + + #[test] + fn pyvenv_cfg_with_carriage_return_line_endings_parses() { + let pyvenv_cfg = "home = /somewhere/python\r\nversion_info = 3.13\r\nimplementation = PyPy"; + let parsed = PyvenvCfgParser::new(pyvenv_cfg).parse().unwrap(); + assert_eq!(parsed.base_executable_home_path, Some("/somewhere/python")); + let version = parsed.version.unwrap(); + assert_eq!(version.0, "3.13"); + assert_eq!(&pyvenv_cfg[version.1], version.0); + assert_eq!(parsed.implementation, PythonImplementation::PyPy); + } + + #[test] + fn pyvenv_cfg_with_strange_whitespace_parses() { + let pyvenv_cfg = " home= /a path with whitespace/python\t \t \nversion_info = 3.13 \n\n\n\nimplementation =PyPy"; + let parsed = PyvenvCfgParser::new(pyvenv_cfg).parse().unwrap(); + assert_eq!( + parsed.base_executable_home_path, + Some("/a path with whitespace/python") + ); + let version = parsed.version.unwrap(); + assert_eq!(version.0, "3.13"); + assert_eq!(&pyvenv_cfg[version.1], version.0); + assert_eq!(parsed.implementation, PythonImplementation::PyPy); + } } diff --git a/crates/ty_python_semantic/src/util/diagnostics.rs b/crates/ty_python_semantic/src/util/diagnostics.rs index c9949f0bddb044..c2419c2feb4f5b 100644 --- a/crates/ty_python_semantic/src/util/diagnostics.rs +++ b/crates/ty_python_semantic/src/util/diagnostics.rs @@ -1,8 +1,5 @@ use crate::{Db, Program, PythonVersionWithSource}; -use ruff_db::{ - diagnostic::{Annotation, Diagnostic, Severity, Span, SubDiagnostic}, - files::system_path_to_file, -}; +use ruff_db::diagnostic::{Annotation, Diagnostic, Severity, SubDiagnostic}; /// Add a subdiagnostic to `diagnostic` that explains why a certain Python version was inferred. /// @@ -22,23 +19,47 @@ pub fn add_inferred_python_version_hint_to_diagnostic( "Python {version} was assumed when {action} because it was specified on the command line", )); } - crate::PythonVersionSource::File(path, range) => { - if let Ok(file) = system_path_to_file(db.upcast(), &**path) { + crate::PythonVersionSource::ConfigFile(source) => { + if let Some(span) = source.span(db) { let mut sub_diagnostic = SubDiagnostic::new( Severity::Info, format_args!("Python {version} was assumed when {action}"), ); - sub_diagnostic.annotate( - Annotation::primary(Span::from(file).with_optional_range(*range)).message( - format_args!("Python {version} assumed due to this configuration setting"), + sub_diagnostic.annotate(Annotation::primary(span).message(format_args!( + "Python {version} assumed due to this configuration setting" + ))); + diagnostic.sub(sub_diagnostic); + } else { + diagnostic.info(format_args!( + "Python {version} was assumed when {action} because of your configuration file(s)", + )); + } + } + crate::PythonVersionSource::PyvenvCfgFile(source) => { + if let Some(span) = source.span(db) { + let mut sub_diagnostic = SubDiagnostic::new( + Severity::Info, + format_args!( + "Python {version} was assumed when {action} because of your virtual environment" ), ); + sub_diagnostic.annotate( + Annotation::primary(span) + .message("Python version inferred from virtual environment metadata file"), + ); + // TODO: it would also be nice to tell them how we resolved their virtual environment... diagnostic.sub(sub_diagnostic); } else { diagnostic.info(format_args!( - "Python {version} was assumed when {action} because of your configuration file(s)", + "Python {version} was assumed when {action} because \ + your virtual environment's pyvenv.cfg file indicated \ + it was the Python version being used", )); } + diagnostic.info( + "No Python version was specified on the command line \ + or in a configuration file", + ); } crate::PythonVersionSource::Default => { diagnostic.info(format_args!( diff --git a/crates/ty_test/src/assertion.rs b/crates/ty_test/src/assertion.rs index 8df0b1caf2bf8a..38ff0494d0856d 100644 --- a/crates/ty_test/src/assertion.rs +++ b/crates/ty_test/src/assertion.rs @@ -501,7 +501,7 @@ mod tests { let mut db = Db::setup(); let settings = ProgramSettings { - python_version: PythonVersionWithSource::default(), + python_version: Some(PythonVersionWithSource::default()), python_platform: PythonPlatform::default(), search_paths: SearchPathSettings::new(Vec::new()), }; diff --git a/crates/ty_test/src/lib.rs b/crates/ty_test/src/lib.rs index 36533cf6dea592..234c894725ab61 100644 --- a/crates/ty_test/src/lib.rs +++ b/crates/ty_test/src/lib.rs @@ -258,10 +258,10 @@ fn run_test( let configuration = test.configuration(); let settings = ProgramSettings { - python_version: PythonVersionWithSource { + python_version: Some(PythonVersionWithSource { version: python_version, source: PythonVersionSource::Cli, - }, + }), python_platform: configuration .python_platform() .unwrap_or(PythonPlatform::Identifier("linux".to_string())), diff --git a/crates/ty_test/src/matcher.rs b/crates/ty_test/src/matcher.rs index ff90010f786357..92e150d8603b9a 100644 --- a/crates/ty_test/src/matcher.rs +++ b/crates/ty_test/src/matcher.rs @@ -385,7 +385,7 @@ mod tests { let mut db = crate::db::Db::setup(); let settings = ProgramSettings { - python_version: PythonVersionWithSource::default(), + python_version: Some(PythonVersionWithSource::default()), python_platform: PythonPlatform::default(), search_paths: SearchPathSettings::new(Vec::new()), }; diff --git a/fuzz/fuzz_targets/ty_check_invalid_syntax.rs b/fuzz/fuzz_targets/ty_check_invalid_syntax.rs index dbc814644c78a1..5635d75c3028fc 100644 --- a/fuzz/fuzz_targets/ty_check_invalid_syntax.rs +++ b/fuzz/fuzz_targets/ty_check_invalid_syntax.rs @@ -118,7 +118,7 @@ fn setup_db() -> TestDb { Program::from_settings( &db, ProgramSettings { - python_version: PythonVersionWithSource::default(), + python_version: Some(PythonVersionWithSource::default()), python_platform: PythonPlatform::default(), search_paths: SearchPathSettings::new(vec![src_root]), },