mirror of https://github.com/astral-sh/ruff
Make the default settings try harder to avoid crashes
This introduces a safe_mode to our settings type that tells our initialization code to more aggressively believe Settings Failure Is Not An Error. This allows the full settings initialization logic to be used as much as possible for the default database while trying to keep any discarded results as narrow as possible for graceful degradation. Finally, Python has SFINAE.
This commit is contained in:
parent
4b0fa5f270
commit
3e2da64f23
|
|
@ -94,9 +94,23 @@ pub struct Options {
|
||||||
#[serde(skip_serializing_if = "Option::is_none")]
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
#[option_group]
|
#[option_group]
|
||||||
pub overrides: Option<OverridesOptions>,
|
pub overrides: Option<OverridesOptions>,
|
||||||
|
|
||||||
|
/// Settings Failure Is Not An Error.
|
||||||
|
///
|
||||||
|
/// This is used by the default database, which we are incentivized to make infallible,
|
||||||
|
/// while still trying to "do our best" to set things up properly where we can.
|
||||||
|
#[serde(default, skip)]
|
||||||
|
pub safe_mode: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Options {
|
impl Options {
|
||||||
|
pub fn safe() -> Self {
|
||||||
|
Self {
|
||||||
|
safe_mode: true,
|
||||||
|
..Self::default()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
pub fn from_toml_str(content: &str, source: ValueSource) -> Result<Self, TyTomlError> {
|
pub fn from_toml_str(content: &str, source: ValueSource) -> Result<Self, TyTomlError> {
|
||||||
let _guard = ValueSourceGuard::new(source, true);
|
let _guard = ValueSourceGuard::new(source, true);
|
||||||
let options = toml::from_str(content)?;
|
let options = toml::from_str(content)?;
|
||||||
|
|
@ -156,20 +170,44 @@ impl Options {
|
||||||
ValueSource::PythonVSCodeExtension => SysPrefixPathOrigin::PythonVSCodeExtension,
|
ValueSource::PythonVSCodeExtension => SysPrefixPathOrigin::PythonVSCodeExtension,
|
||||||
};
|
};
|
||||||
|
|
||||||
Some(PythonEnvironment::new(
|
PythonEnvironment::new(python_path.absolute(project_root, system), origin, system)
|
||||||
python_path.absolute(project_root, system),
|
.map_err(anyhow::Error::from)
|
||||||
origin,
|
.map(Some)
|
||||||
system,
|
|
||||||
)?)
|
|
||||||
} else {
|
} else {
|
||||||
PythonEnvironment::discover(project_root, system)
|
PythonEnvironment::discover(project_root, system)
|
||||||
.context("Failed to discover local Python environment")?
|
.context("Failed to discover local Python environment")
|
||||||
|
};
|
||||||
|
|
||||||
|
// If in safe-mode, fallback to None if this fails instead of erroring.
|
||||||
|
let python_environment = match python_environment {
|
||||||
|
Ok(python_environment) => python_environment,
|
||||||
|
Err(err) => {
|
||||||
|
if self.safe_mode {
|
||||||
|
tracing::debug!("Default settings failed to discover local Python environment");
|
||||||
|
None
|
||||||
|
} else {
|
||||||
|
return Err(err);
|
||||||
|
}
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
let site_packages_paths = if let Some(python_environment) = python_environment.as_ref() {
|
let site_packages_paths = if let Some(python_environment) = python_environment.as_ref() {
|
||||||
python_environment
|
let site_packages_paths = python_environment
|
||||||
.site_packages_paths(system)
|
.site_packages_paths(system)
|
||||||
.context("Failed to discover the site-packages directory")?
|
.context("Failed to discover the site-packages directory");
|
||||||
|
match site_packages_paths {
|
||||||
|
Ok(paths) => paths,
|
||||||
|
Err(err) => {
|
||||||
|
if self.safe_mode {
|
||||||
|
tracing::debug!(
|
||||||
|
"Default settings failed to discover site-packages directory"
|
||||||
|
);
|
||||||
|
SitePackagesPaths::default()
|
||||||
|
} else {
|
||||||
|
return Err(err);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
tracing::debug!("No virtual environment found");
|
tracing::debug!("No virtual environment found");
|
||||||
|
|
||||||
|
|
@ -193,6 +231,7 @@ impl Options {
|
||||||
.or_else(|| site_packages_paths.python_version_from_layout())
|
.or_else(|| site_packages_paths.python_version_from_layout())
|
||||||
.unwrap_or_default();
|
.unwrap_or_default();
|
||||||
|
|
||||||
|
// Safe mode is handled inside this function, so we just assume this can't fail
|
||||||
let search_paths = self.to_search_paths(
|
let search_paths = self.to_search_paths(
|
||||||
project_root,
|
project_root,
|
||||||
project_name,
|
project_name,
|
||||||
|
|
@ -352,6 +391,7 @@ impl Options {
|
||||||
.map(|path| path.absolute(project_root, system)),
|
.map(|path| path.absolute(project_root, system)),
|
||||||
site_packages_paths: site_packages_paths.into_vec(),
|
site_packages_paths: site_packages_paths.into_vec(),
|
||||||
real_stdlib_path,
|
real_stdlib_path,
|
||||||
|
safe_mode: self.safe_mode,
|
||||||
};
|
};
|
||||||
|
|
||||||
settings.to_search_paths(system, vendored)
|
settings.to_search_paths(system, vendored)
|
||||||
|
|
|
||||||
|
|
@ -236,6 +236,7 @@ impl SearchPaths {
|
||||||
custom_typeshed: typeshed,
|
custom_typeshed: typeshed,
|
||||||
site_packages_paths,
|
site_packages_paths,
|
||||||
real_stdlib_path,
|
real_stdlib_path,
|
||||||
|
safe_mode,
|
||||||
} = settings;
|
} = settings;
|
||||||
|
|
||||||
let mut static_paths = vec![];
|
let mut static_paths = vec![];
|
||||||
|
|
@ -244,12 +245,30 @@ impl SearchPaths {
|
||||||
let path = canonicalize(path, system);
|
let path = canonicalize(path, system);
|
||||||
tracing::debug!("Adding extra search-path `{path}`");
|
tracing::debug!("Adding extra search-path `{path}`");
|
||||||
|
|
||||||
static_paths.push(SearchPath::extra(system, path)?);
|
match SearchPath::extra(system, path) {
|
||||||
|
Ok(path) => static_paths.push(path),
|
||||||
|
Err(err) => {
|
||||||
|
if *safe_mode {
|
||||||
|
tracing::debug!("Skipping invalid extra search-path: {err}");
|
||||||
|
} else {
|
||||||
|
return Err(err);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
for src_root in src_roots {
|
for src_root in src_roots {
|
||||||
tracing::debug!("Adding first-party search path `{src_root}`");
|
tracing::debug!("Adding first-party search path `{src_root}`");
|
||||||
static_paths.push(SearchPath::first_party(system, src_root.to_path_buf())?);
|
match SearchPath::first_party(system, src_root.to_path_buf()) {
|
||||||
|
Ok(path) => static_paths.push(path),
|
||||||
|
Err(err) => {
|
||||||
|
if *safe_mode {
|
||||||
|
tracing::debug!("Skipping invalid first-party search-path: {err}");
|
||||||
|
} else {
|
||||||
|
return Err(err);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
let (typeshed_versions, stdlib_path) = if let Some(typeshed) = typeshed {
|
let (typeshed_versions, stdlib_path) = if let Some(typeshed) = typeshed {
|
||||||
|
|
@ -258,18 +277,31 @@ impl SearchPaths {
|
||||||
|
|
||||||
let versions_path = typeshed.join("stdlib/VERSIONS");
|
let versions_path = typeshed.join("stdlib/VERSIONS");
|
||||||
|
|
||||||
let versions_content = system.read_to_string(&versions_path).map_err(|error| {
|
let results = system
|
||||||
SearchPathValidationError::FailedToReadVersionsFile {
|
.read_to_string(&versions_path)
|
||||||
|
.map_err(
|
||||||
|
|error| SearchPathValidationError::FailedToReadVersionsFile {
|
||||||
path: versions_path,
|
path: versions_path,
|
||||||
error,
|
error,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.and_then(|versions_content| Ok(versions_content.parse()?))
|
||||||
|
.and_then(|parsed| Ok((parsed, SearchPath::custom_stdlib(system, &typeshed)?)));
|
||||||
|
|
||||||
|
match results {
|
||||||
|
Ok(results) => results,
|
||||||
|
Err(err) => {
|
||||||
|
if settings.safe_mode {
|
||||||
|
tracing::debug!("Skipping custom-stdlib search-path: {err}");
|
||||||
|
(
|
||||||
|
vendored_typeshed_versions(vendored),
|
||||||
|
SearchPath::vendored_stdlib(),
|
||||||
|
)
|
||||||
|
} else {
|
||||||
|
return Err(err);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
})?;
|
|
||||||
|
|
||||||
let parsed: TypeshedVersions = versions_content.parse()?;
|
|
||||||
|
|
||||||
let search_path = SearchPath::custom_stdlib(system, &typeshed)?;
|
|
||||||
|
|
||||||
(parsed, search_path)
|
|
||||||
} else {
|
} else {
|
||||||
tracing::debug!("Using vendored stdlib");
|
tracing::debug!("Using vendored stdlib");
|
||||||
(
|
(
|
||||||
|
|
@ -279,7 +311,17 @@ impl SearchPaths {
|
||||||
};
|
};
|
||||||
|
|
||||||
let real_stdlib_path = if let Some(path) = real_stdlib_path {
|
let real_stdlib_path = if let Some(path) = real_stdlib_path {
|
||||||
Some(SearchPath::real_stdlib(system, path.clone())?)
|
match SearchPath::real_stdlib(system, path.clone()) {
|
||||||
|
Ok(path) => Some(path),
|
||||||
|
Err(err) => {
|
||||||
|
if *safe_mode {
|
||||||
|
tracing::debug!("Skipping invalid real-stdlib search-path: {err}");
|
||||||
|
None
|
||||||
|
} else {
|
||||||
|
return Err(err);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
None
|
None
|
||||||
};
|
};
|
||||||
|
|
@ -288,7 +330,16 @@ impl SearchPaths {
|
||||||
|
|
||||||
for path in site_packages_paths {
|
for path in site_packages_paths {
|
||||||
tracing::debug!("Adding site-packages search path `{path}`");
|
tracing::debug!("Adding site-packages search path `{path}`");
|
||||||
site_packages.push(SearchPath::site_packages(system, path.clone())?);
|
match SearchPath::site_packages(system, path.clone()) {
|
||||||
|
Ok(path) => site_packages.push(path),
|
||||||
|
Err(err) => {
|
||||||
|
if settings.safe_mode {
|
||||||
|
tracing::debug!("Skipping invalid real-stdlib search-path: {err}");
|
||||||
|
} else {
|
||||||
|
return Err(err);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TODO vendor typeshed's third-party stubs as well as the stdlib and
|
// TODO vendor typeshed's third-party stubs as well as the stdlib and
|
||||||
|
|
|
||||||
|
|
@ -184,6 +184,12 @@ pub struct SearchPathSettings {
|
||||||
/// We should ideally only ever use this for things like goto-definition,
|
/// We should ideally only ever use this for things like goto-definition,
|
||||||
/// where typeshed isn't the right answer.
|
/// where typeshed isn't the right answer.
|
||||||
pub real_stdlib_path: Option<SystemPathBuf>,
|
pub real_stdlib_path: Option<SystemPathBuf>,
|
||||||
|
|
||||||
|
/// Settings Failure Is Not An Error.
|
||||||
|
///
|
||||||
|
/// This is used by the default database, which we are incentivized to make infallible,
|
||||||
|
/// while still trying to "do our best" to set things up properly where we can.
|
||||||
|
pub safe_mode: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl SearchPathSettings {
|
impl SearchPathSettings {
|
||||||
|
|
@ -201,6 +207,7 @@ impl SearchPathSettings {
|
||||||
custom_typeshed: None,
|
custom_typeshed: None,
|
||||||
site_packages_paths: vec![],
|
site_packages_paths: vec![],
|
||||||
real_stdlib_path: None,
|
real_stdlib_path: None,
|
||||||
|
safe_mode: false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -540,7 +540,7 @@ impl Session {
|
||||||
));
|
));
|
||||||
|
|
||||||
let db_with_default_settings =
|
let db_with_default_settings =
|
||||||
ProjectMetadata::from_options(Options::default(), root, None)
|
ProjectMetadata::from_options(Options::safe(), root, None)
|
||||||
.context("Failed to convert default options to metadata")
|
.context("Failed to convert default options to metadata")
|
||||||
.and_then(|metadata| ProjectDatabase::new(metadata, system))
|
.and_then(|metadata| ProjectDatabase::new(metadata, system))
|
||||||
.expect("Default configuration to be valid");
|
.expect("Default configuration to be valid");
|
||||||
|
|
|
||||||
|
|
@ -307,6 +307,7 @@ fn run_test(
|
||||||
custom_typeshed: custom_typeshed_path.map(SystemPath::to_path_buf),
|
custom_typeshed: custom_typeshed_path.map(SystemPath::to_path_buf),
|
||||||
site_packages_paths,
|
site_packages_paths,
|
||||||
real_stdlib_path: None,
|
real_stdlib_path: None,
|
||||||
|
safe_mode: false,
|
||||||
}
|
}
|
||||||
.to_search_paths(db.system(), db.vendored())
|
.to_search_paths(db.system(), db.vendored())
|
||||||
.expect("Failed to resolve search path settings"),
|
.expect("Failed to resolve search path settings"),
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue