Skip to content

Commit 56539db

Browse files
Gankrazanieb
andauthored
[ty] Fix some configuration panics in the LSP (#22040)
## Summary This is a revival of #21047 now that we have a reproducer again. * Fixes astral-sh/ty#2031 * Fixes astral-sh/ty#859 ## Test Plan e2e test from @zanieb --------- Co-authored-by: Zanie Blue <contact@zanie.dev>
1 parent 8d32ad1 commit 56539db

12 files changed

Lines changed: 302 additions & 47 deletions

File tree

crates/ruff_db/src/system/test.rs

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use glob::PatternError;
22
use ruff_notebook::{Notebook, NotebookError};
3+
use rustc_hash::FxHashMap;
34
use std::panic::RefUnwindSafe;
45
use std::sync::{Arc, Mutex};
56

@@ -20,18 +21,44 @@ use super::walk_directory::WalkDirectoryBuilder;
2021
///
2122
/// ## Warning
2223
/// Don't use this system for production code. It's intended for testing only.
23-
#[derive(Debug, Clone)]
24+
#[derive(Debug)]
2425
pub struct TestSystem {
2526
inner: Arc<dyn WritableSystem + RefUnwindSafe + Send + Sync>,
27+
/// Environment variable overrides. If a key is present here, it takes precedence
28+
/// over the inner system's environment variables.
29+
env_overrides: Arc<Mutex<FxHashMap<String, Option<String>>>>,
30+
}
31+
32+
impl Clone for TestSystem {
33+
fn clone(&self) -> Self {
34+
Self {
35+
inner: self.inner.clone(),
36+
env_overrides: self.env_overrides.clone(),
37+
}
38+
}
2639
}
2740

2841
impl TestSystem {
2942
pub fn new(inner: impl WritableSystem + RefUnwindSafe + Send + Sync + 'static) -> Self {
3043
Self {
3144
inner: Arc::new(inner),
45+
env_overrides: Arc::new(Mutex::new(FxHashMap::default())),
3246
}
3347
}
3448

49+
/// Sets an environment variable override. This takes precedence over the inner system.
50+
pub fn set_env_var(&self, name: impl Into<String>, value: impl Into<String>) {
51+
self.env_overrides
52+
.lock()
53+
.unwrap()
54+
.insert(name.into(), Some(value.into()));
55+
}
56+
57+
/// Removes an environment variable override, making it appear as not set.
58+
pub fn remove_env_var(&self, name: impl Into<String>) {
59+
self.env_overrides.lock().unwrap().insert(name.into(), None);
60+
}
61+
3562
/// Returns the [`InMemorySystem`].
3663
///
3764
/// ## Panics
@@ -147,6 +174,18 @@ impl System for TestSystem {
147174
self.system().case_sensitivity()
148175
}
149176

177+
fn env_var(&self, name: &str) -> std::result::Result<String, std::env::VarError> {
178+
// Check overrides first
179+
if let Some(override_value) = self.env_overrides.lock().unwrap().get(name) {
180+
return match override_value {
181+
Some(value) => Ok(value.clone()),
182+
None => Err(std::env::VarError::NotPresent),
183+
};
184+
}
185+
// Fall back to inner system
186+
self.system().env_var(name)
187+
}
188+
150189
fn dyn_clone(&self) -> Box<dyn System> {
151190
Box::new(self.clone())
152191
}
@@ -156,6 +195,7 @@ impl Default for TestSystem {
156195
fn default() -> Self {
157196
Self {
158197
inner: Arc::new(InMemorySystem::default()),
198+
env_overrides: Arc::new(Mutex::new(FxHashMap::default())),
159199
}
160200
}
161201
}

crates/ty/tests/cli/python_environment.rs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2703,3 +2703,51 @@ fn pythonpath_multiple_dirs_is_respected() -> anyhow::Result<()> {
27032703

27042704
Ok(())
27052705
}
2706+
2707+
/// Test behavior when `VIRTUAL_ENV` is set but points to a non-existent path.
2708+
#[test]
2709+
fn missing_virtual_env() -> anyhow::Result<()> {
2710+
let working_venv_package1_path = if cfg!(windows) {
2711+
"project/.venv/Lib/site-packages/package1/__init__.py"
2712+
} else {
2713+
"project/.venv/lib/python3.13/site-packages/package1/__init__.py"
2714+
};
2715+
2716+
let case = CliTest::with_files([
2717+
(
2718+
"project/test.py",
2719+
r#"
2720+
from package1 import WorkingVenv
2721+
"#,
2722+
),
2723+
(
2724+
"project/.venv/pyvenv.cfg",
2725+
r#"
2726+
home = ./
2727+
2728+
"#,
2729+
),
2730+
(
2731+
working_venv_package1_path,
2732+
r#"
2733+
class WorkingVenv: ...
2734+
"#,
2735+
),
2736+
])?;
2737+
2738+
assert_cmd_snapshot!(case.command()
2739+
.current_dir(case.root().join("project"))
2740+
.env("VIRTUAL_ENV", case.root().join("nonexistent-venv")), @r"
2741+
success: false
2742+
exit_code: 2
2743+
----- stdout -----
2744+
2745+
----- stderr -----
2746+
ty failed
2747+
Cause: Failed to discover local Python environment
2748+
Cause: Invalid `VIRTUAL_ENV` environment variable `<temp_dir>/nonexistent-venv`: does not point to a directory on disk
2749+
Cause: No such file or directory (os error 2)
2750+
");
2751+
2752+
Ok(())
2753+
}

crates/ty_project/src/metadata.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use ruff_python_ast::name::Name;
55
use std::sync::Arc;
66
use thiserror::Error;
77
use ty_combine::Combine;
8-
use ty_python_semantic::ProgramSettings;
8+
use ty_python_semantic::{MisconfigurationMode, ProgramSettings};
99

1010
use crate::metadata::options::ProjectOptionsOverrides;
1111
use crate::metadata::pyproject::{Project, PyProject, PyProjectError, ResolveRequiresPythonError};
@@ -37,6 +37,9 @@ pub struct ProjectMetadata {
3737
/// The path ordering doesn't imply precedence.
3838
#[cfg_attr(test, serde(skip_serializing_if = "Vec::is_empty"))]
3939
pub(super) extra_configuration_paths: Vec<SystemPathBuf>,
40+
41+
#[cfg_attr(test, serde(skip))]
42+
pub(super) misconfiguration_mode: MisconfigurationMode,
4043
}
4144

4245
impl ProjectMetadata {
@@ -47,6 +50,7 @@ impl ProjectMetadata {
4750
root,
4851
extra_configuration_paths: Vec::default(),
4952
options: Options::default(),
53+
misconfiguration_mode: MisconfigurationMode::Fail,
5054
}
5155
}
5256

@@ -70,6 +74,7 @@ impl ProjectMetadata {
7074
root: system.current_directory().to_path_buf(),
7175
options,
7276
extra_configuration_paths: vec![path],
77+
misconfiguration_mode: MisconfigurationMode::Fail,
7378
})
7479
}
7580

@@ -82,6 +87,7 @@ impl ProjectMetadata {
8287
pyproject.tool.and_then(|tool| tool.ty).unwrap_or_default(),
8388
root,
8489
pyproject.project.as_ref(),
90+
MisconfigurationMode::Fail,
8591
)
8692
}
8793

@@ -90,6 +96,7 @@ impl ProjectMetadata {
9096
mut options: Options,
9197
root: SystemPathBuf,
9298
project: Option<&Project>,
99+
misconfiguration_mode: MisconfigurationMode,
93100
) -> Result<Self, ResolveRequiresPythonError> {
94101
let name = project
95102
.and_then(|project| project.name.as_deref())
@@ -117,6 +124,7 @@ impl ProjectMetadata {
117124
root,
118125
options,
119126
extra_configuration_paths: Vec::new(),
127+
misconfiguration_mode,
120128
})
121129
}
122130

@@ -194,6 +202,7 @@ impl ProjectMetadata {
194202
pyproject
195203
.as_ref()
196204
.and_then(|pyproject| pyproject.project.as_ref()),
205+
MisconfigurationMode::Fail,
197206
)
198207
.map_err(|err| {
199208
ProjectMetadataError::InvalidRequiresPythonConstraint {
@@ -273,8 +282,13 @@ impl ProjectMetadata {
273282
system: &dyn System,
274283
vendored: &VendoredFileSystem,
275284
) -> anyhow::Result<ProgramSettings> {
276-
self.options
277-
.to_program_settings(self.root(), self.name(), system, vendored)
285+
self.options.to_program_settings(
286+
self.root(),
287+
self.name(),
288+
system,
289+
vendored,
290+
self.misconfiguration_mode,
291+
)
278292
}
279293

280294
pub fn apply_overrides(&mut self, overrides: &ProjectOptionsOverrides) {

crates/ty_project/src/metadata/options.rs

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ use thiserror::Error;
3030
use ty_combine::Combine;
3131
use ty_python_semantic::lint::{Level, LintSource, RuleSelection};
3232
use ty_python_semantic::{
33-
ProgramSettings, PythonEnvironment, PythonPlatform, PythonVersionFileSource,
34-
PythonVersionSource, PythonVersionWithSource, SearchPathSettings, SearchPathValidationError,
35-
SearchPaths, SitePackagesPaths, SysPrefixPathOrigin,
33+
MisconfigurationMode, ProgramSettings, PythonEnvironment, PythonPlatform,
34+
PythonVersionFileSource, PythonVersionSource, PythonVersionWithSource, SearchPathSettings,
35+
SearchPathValidationError, SearchPaths, SitePackagesPaths, SysPrefixPathOrigin,
3636
};
3737
use ty_static::EnvVars;
3838

@@ -117,6 +117,7 @@ impl Options {
117117
project_name: &str,
118118
system: &dyn System,
119119
vendored: &VendoredFileSystem,
120+
misconfiguration_mode: MisconfigurationMode,
120121
) -> anyhow::Result<ProgramSettings> {
121122
let environment = self.environment.or_default();
122123

@@ -154,14 +155,25 @@ impl Options {
154155
ValueSource::Editor => SysPrefixPathOrigin::Editor,
155156
};
156157

157-
Some(PythonEnvironment::new(
158-
python_path.absolute(project_root, system),
159-
origin,
160-
system,
161-
)?)
158+
PythonEnvironment::new(python_path.absolute(project_root, system), origin, system)
159+
.map_err(anyhow::Error::from)
160+
.map(Some)
162161
} else {
163162
PythonEnvironment::discover(project_root, system)
164-
.context("Failed to discover local Python environment")?
163+
.context("Failed to discover local Python environment")
164+
};
165+
166+
// If in safe-mode, fallback to None if this fails instead of erroring.
167+
let python_environment = match python_environment {
168+
Ok(python_environment) => python_environment,
169+
Err(err) => {
170+
if misconfiguration_mode == MisconfigurationMode::UseDefault {
171+
tracing::debug!("Default settings failed to discover local Python environment");
172+
None
173+
} else {
174+
return Err(err);
175+
}
176+
}
165177
};
166178

167179
let self_site_packages = self_environment_search_paths(
@@ -174,11 +186,23 @@ impl Options {
174186
.unwrap_or_default();
175187

176188
let site_packages_paths = if let Some(python_environment) = python_environment.as_ref() {
177-
self_site_packages.concatenate(
178-
python_environment
179-
.site_packages_paths(system)
180-
.context("Failed to discover the site-packages directory")?,
181-
)
189+
let site_packages_paths = python_environment
190+
.site_packages_paths(system)
191+
.context("Failed to discover the site-packages directory");
192+
let site_packages_paths = match site_packages_paths {
193+
Ok(paths) => paths,
194+
Err(err) => {
195+
if misconfiguration_mode == MisconfigurationMode::UseDefault {
196+
tracing::debug!(
197+
"Default settings failed to discover site-packages directory"
198+
);
199+
SitePackagesPaths::default()
200+
} else {
201+
return Err(err);
202+
}
203+
}
204+
};
205+
self_site_packages.concatenate(site_packages_paths)
182206
} else {
183207
tracing::debug!("No virtual environment found");
184208
self_site_packages
@@ -201,13 +225,15 @@ impl Options {
201225
.or_else(|| site_packages_paths.python_version_from_layout())
202226
.unwrap_or_default();
203227

228+
// Safe mode is handled inside this function, so we just assume this can't fail
204229
let search_paths = self.to_search_paths(
205230
project_root,
206231
project_name,
207232
site_packages_paths,
208233
real_stdlib_path,
209234
system,
210235
vendored,
236+
misconfiguration_mode,
211237
)?;
212238

213239
tracing::info!(
@@ -222,6 +248,7 @@ impl Options {
222248
})
223249
}
224250

251+
#[expect(clippy::too_many_arguments)]
225252
fn to_search_paths(
226253
&self,
227254
project_root: &SystemPath,
@@ -230,6 +257,7 @@ impl Options {
230257
real_stdlib_path: Option<SystemPathBuf>,
231258
system: &dyn System,
232259
vendored: &VendoredFileSystem,
260+
misconfiguration_mode: MisconfigurationMode,
233261
) -> Result<SearchPaths, SearchPathValidationError> {
234262
let environment = self.environment.or_default();
235263
let src = self.src.or_default();
@@ -344,6 +372,7 @@ impl Options {
344372
.map(|path| path.absolute(project_root, system)),
345373
site_packages_paths: site_packages_paths.into_vec(),
346374
real_stdlib_path,
375+
misconfiguration_mode,
347376
};
348377

349378
settings.to_search_paths(system, vendored)

crates/ty_python_semantic/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ pub use module_resolver::{
1717
resolve_real_module_confident, resolve_real_shadowable_module, system_module_search_paths,
1818
};
1919
pub use program::{
20-
Program, ProgramSettings, PythonVersionFileSource, PythonVersionSource,
20+
MisconfigurationMode, Program, ProgramSettings, PythonVersionFileSource, PythonVersionSource,
2121
PythonVersionWithSource, SearchPathSettings,
2222
};
2323
pub use python_platform::PythonPlatform;

0 commit comments

Comments
 (0)