mirrors/ruff
0
0 Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2026-10-28 10:45:37 +08:00
Code Activity

[ty] Skip scripts with an invalid PEP 723 configuration (#27505)

Browse Source 
## Summary

Skip checking scripts with invalid PEP 723 metadata blocks, that are,
blocks that ty can't parse successfully or configurations that don't
resolve successfully. Instead, emit one diagnostic that points to the
invalid metadata.

ty still initializes a `Program` for each of those scripts so that LSP
operations continue to work. But we skip those scripts in all code paths
where we perform semantic checks.

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

## Test plan

Added tests
This commit is contained in:
Micha Reiser authored and GitHub committed 2026-08-20 10:34:53 +02:00
1 parent 653d8519c7
commit 248de0b701
18 files changed
+957 -153
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/ruff_db/src/diagnostic/mod.rs
+4
View File
@@ -1009,6 +1009,9 @@ pub enum DiagnosticId {
/// A glob pattern doesn't follow the expected syntax.
InvalidGlob,
/// A PEP 723 script contains invalid metadata or configuration.
InvalidScriptMetadata,
/// An `include` glob without any patterns.
///
/// ## Why is this bad?
@@ -1137,6 +1140,7 @@ impl DiagnosticId {
DiagnosticId::RevealedType => "revealed-type",
DiagnosticId::UnknownRule => "unknown-rule",
DiagnosticId::InvalidGlob => "invalid-glob",
DiagnosticId::InvalidScriptMetadata => "invalid-script-metadata",
DiagnosticId::EmptyInclude => "empty-include",
DiagnosticId::UnnecessaryOverridesSection => "unnecessary-overrides-section",
DiagnosticId::UselessOverridesSection => "useless-overrides-section",
crates/ruff_python_ast/src/script.rs
-5
View File
@@ -32,11 +32,6 @@ impl ScriptTag {
&self.source_map
}
/// Consumes this tag and returns its TOML together with its original-source mapping.
pub fn into_metadata_and_source_map(self) -> (String, ScriptSourceMap) {
(self.metadata, self.source_map)
}
/// Given the contents of a Python file, extract the `script` metadata block with leading
/// comment hashes removed and map its offsets to the original Python script.
///
crates/ty/tests/cli/file_selection.rs
+21
View File
@@ -90,6 +90,27 @@ fn exclude_scripts_only_applies_to_implicitly_discovered_files() -> anyhow::Resu
Ok(())
}
#[test]
fn exclude_scripts_ignores_scripts_with_invalid_toml() -> anyhow::Result<()> {
let case = CliTest::with_files([
("main.py", "value: int = 1"),
(
"script.py",
r#"
# /// script
# requires-python =
# ///
value: int = "script"
"#,
),
])?;
let output = case.command().arg("--exclude-scripts").output()?;
assert!(output.status.success(), "{output:?}");
Ok(())
}
/// Test exclude CLI argument functionality
#[test]
fn exclude_argument() -> anyhow::Result<()> {
crates/ty/tests/cli/scripts.rs
+153 -43
View File
@@ -1041,7 +1041,79 @@ fn scripts_use_an_activated_virtual_environment() -> anyhow::Result<()> {
}
#[test]
fn invalid_python_requirement_falls_back_to_project_configuration() -> anyhow::Result<()> {
fn invalid_toml_reports_configuration_error() -> anyhow::Result<()> {
let case = CliTest::with_file(
"script.py",
r#"
# /// script
# requires-python =
# ///
print(missing)
"#,
)?;
assert_cmd_snapshot!(case.command(), @"
success: false
exit_code: 1
----- stdout -----
error[invalid-script-metadata]: string values must be quoted, expected literal string
--> script.py:3:20
|
3 | # requires-python =
| ^
Found 1 diagnostic
----- stderr -----
");
assert_cmd_snapshot!(case.command().arg("--output-format").arg("concise"), @"
success: false
exit_code: 1
----- stdout -----
script.py:3:20: error[invalid-script-metadata] string values must be quoted, expected literal string
Found 1 diagnostic
----- stderr -----
");
Ok(())
}
#[test]
fn invalid_metadata_options_report_configuration_error() -> anyhow::Result<()> {
let case = CliTest::with_file(
"script.py",
r#"
# /// script
# [tool.ty.environment]
# python-version = true
# ///
print(missing)
"#,
)?;
assert_cmd_snapshot!(case.command(), @"
success: false
exit_code: 1
----- stdout -----
error[invalid-script-metadata]: wanted string or table
--> script.py:4:20
|
4 | # python-version = true
| ^^^^
Found 1 diagnostic
----- stderr -----
");
Ok(())
}
#[test]
fn invalid_python_requirement_reports_configuration_error() -> anyhow::Result<()> {
let case = CliTest::with_files([
(
"pyproject.toml",
@@ -1077,39 +1149,32 @@ fn invalid_python_requirement_falls_back_to_project_configuration() -> anyhow::R
success: false
exit_code: 1
----- stdout -----
info[revealed-type]: Revealed type
--> script.py:13:13
|
13 | reveal_type(sys.platform)
| ^^^^^^^^^^^^ `Literal["linux"]`
error[invalid-script-metadata]: value `<3.12` does not contain a lower bound
--> script.py:3:21
|
3 | # requires-python = "<3.12"
| ^^^^^^^
info: Add a lower bound to indicate the minimum compatible Python version (e.g., `>=3.13`) or specify a version in `environment.python-version`.
error[unresolved-reference]: Name `missing` used when not defined
--> script.py:14:7
|
14 | print(missing)
| ^^^^^^^
Found 2 diagnostics
Found 1 diagnostic
----- stderr -----
"#);
assert_cmd_snapshot!(case.command().arg("--output-format").arg("concise"), @r#"
assert_cmd_snapshot!(case.command().arg("--output-format").arg("concise"), @"
success: false
exit_code: 1
----- stdout -----
script.py:13:13: info[revealed-type] Revealed type: `Literal["linux"]`
script.py:14:7: error[unresolved-reference] Name `missing` used when not defined
Found 2 diagnostics
script.py:3:21: error[invalid-script-metadata] value `<3.12` does not contain a lower bound
Found 1 diagnostic
----- stderr -----
"#);
");
Ok(())
}
#[test]
fn invalid_script_settings_fall_back_to_project_configuration() -> anyhow::Result<()> {
// FIXME: Scripts with invalid settings should not be checked.
fn invalid_script_settings_report_configuration_error() -> anyhow::Result<()> {
let case = CliTest::with_files([
(
"pyproject.toml",
@@ -1137,14 +1202,14 @@ fn invalid_script_settings_fall_back_to_project_configuration() -> anyhow::Resul
])?;
assert_cmd_snapshot!(case.command(), @r#"
success: true
exit_code: 0
success: false
exit_code: 1
----- stdout -----
info[revealed-type]: Revealed type
--> script.py:12:13
|
12 | reveal_type(sys.platform)
| ^^^^^^^^^^^^ `Literal["linux"]`
error[invalid-glob]: Invalid pattern
--> script.py:4:14
|
4 | # include = ["src/**test/"]
| ^^^^^^^^^^^^^ Too many stars at position 5
Found 1 diagnostic
@@ -1155,8 +1220,7 @@ fn invalid_script_settings_fall_back_to_project_configuration() -> anyhow::Resul
}
#[test]
fn invalid_script_environment_falls_back_to_project_configuration() -> anyhow::Result<()> {
// FIXME: Scripts with invalid environments should not be checked.
fn invalid_script_environment_reports_configuration_error() -> anyhow::Result<()> {
let case = CliTest::with_files([
(
"pyproject.toml",
@@ -1186,25 +1250,71 @@ fn invalid_script_environment_falls_back_to_project_configuration() -> anyhow::R
])?;
assert_cmd_snapshot!(case.command(), @r#"
success: true
exit_code: 0
success: false
exit_code: 1
----- stdout -----
info[revealed-type]: Revealed type
--> script.py:12:13
|
12 | reveal_type(sys.version_info[:2])
| ^^^^^^^^^^^^^^^^^^^^ `tuple[Literal[3], Literal[13]]`
error[invalid-script-metadata]: Invalid `environment.python` setting in script metadata `<temp_dir>/missing-environment`: does not point to a Python executable or a directory on disk
--> script.py:4:12
|
4 | # python = "./missing-environment"
| ^^^^^^^^^^^^^^^^^^^^^^^
info[revealed-type]: Revealed type
--> script.py:13:13
|
13 | reveal_type(sys.platform)
| ^^^^^^^^^^^^ `Literal["linux"]`
Found 2 diagnostics
Found 1 diagnostic
----- stderr -----
"#);
Ok(())
}
#[test]
fn invalid_script_search_paths_do_not_blame_python_environment() -> anyhow::Result<()> {
let dependency = if cfg!(windows) {
"environment/Lib/site-packages/dependency.py"
} else {
"environment/lib/python3.13/site-packages/dependency.py"
};
let case = CliTest::with_files([
("environment/pyvenv.cfg", "home = ./\nversion = 3.13\n"),
(dependency, "value = 1\n"),
(
"script.py",
r#"
# /// script
# [tool.ty.environment]
# python = "./environment"
# typeshed = "./missing-typeshed"
# ///
print(missing)
"#,
),
])?;
assert_cmd_snapshot!(case.command(), @r#"
success: false
exit_code: 1
----- stdout -----
error[invalid-script-metadata]: Failed to read the custom typeshed versions file '<temp_dir>/missing-typeshed/stdlib/VERSIONS'
--> script.py:5:14
|
5 | # typeshed = "./missing-typeshed"
| ^^^^^^^^^^^^^^^^^^^^
Found 1 diagnostic
----- stderr -----
"#);
assert_cmd_snapshot!(case.command().arg("--output-format").arg("concise"), @"
success: false
exit_code: 1
----- stdout -----
script.py:5:14: error[invalid-script-metadata] Failed to read the custom typeshed versions file '<temp_dir>/missing-typeshed/stdlib/VERSIONS'
Found 1 diagnostic
----- stderr -----
");
Ok(())
}
crates/ty_ide/src/hints.rs
+1 -1
View File
@@ -42,7 +42,7 @@ impl HintKind {
pub fn hints(db: &dyn Db, file: ProgramFile<'_>) -> Vec<Hint> {
let source_file = file.file(db);
if !db.should_check_file(source_file) {
if !ty_project::should_check_semantics(db, source_file) {
return Vec::new();
}
crates/ty_project/src/lib.rs
+26 -5
View File
@@ -394,7 +394,12 @@ impl Project {
// This is outside `check_file_impl` to avoid that opening or closing
// a file invalidates the `check_file_impl` query of every file!
if !open_files.contains(&file) {
// Scripts with invalid settings are never parsed by `check_file_impl`, so
// they have no AST to clear.
if !open_files.contains(&file)
&& Script::for_file(db, file)
.is_none_or(|script| script.has_valid_settings(db))
{
let python_file = program_file.python_file(db);
// The module has already been parsed by `check_file_impl`.
// We only retrieve it here so that we can call `clear` on it.
@@ -658,6 +663,15 @@ fn check_file(db: &dyn Db, file: File) -> Vec<Diagnostic> {
.unwrap_or_else(|diagnostic| vec![diagnostic.clone()])
}
/// Returns whether semantic checking and semantic diagnostics should run for `file`.
///
/// Scripts with invalid configuration still produce configuration diagnostics and retain a program
/// for editor operations, but their semantic diagnostics must not be reported.
pub fn should_check_semantics(db: &dyn Db, file: File) -> bool {
db.should_check_file(file)
&& Script::for_file(db, file).is_none_or(|script| script.has_valid_settings(db))
}
/// Returns `true` if the file should be checked.
///
/// This depends on the project's check mode:
@@ -743,18 +757,25 @@ pub(crate) fn check_file_impl(
{
let db = AssertUnwindSafe(db);
match catch(&**db, source_file, || {
let script = Script::for_file(*db, source_file);
if let Some(script) = script
&& !script.has_valid_settings(*db)
{
return Ok(script.settings_diagnostics(*db).to_vec().into_boxed_slice());
}
let diagnostics = ty_python_semantic::check_file(*db, file)?;
let Some(script) = Script::for_file(*db, source_file) else {
let Some(script) = script else {
return Ok(diagnostics);
};
let script_diagnostics = script.diagnostics(*db);
if script_diagnostics.is_empty() {
let settings_diagnostics = script.settings_diagnostics(*db);
if settings_diagnostics.is_empty() {
return Ok(diagnostics);
}
let mut diagnostics = diagnostics.into_vec();
diagnostics.extend(script_diagnostics.iter().cloned());
diagnostics.extend(settings_diagnostics.iter().cloned());
Ok(diagnostics.into_boxed_slice())
}) {
Ok(result) => result,
crates/ty_project/src/metadata.rs
+6 -3
View File
@@ -14,7 +14,7 @@ use ty_static::EnvVars;
use crate::Db;
use crate::metadata::options::{
EnvironmentOptions, OptionDiagnostic, OptionsContext, ProgramSettingsDiagnostic,
ToSettingsError,
ToProgramSettingsError, ToSettingsError,
};
use crate::metadata::pyproject::{Project, PyProject, PyProjectError, ResolveRequiresPythonError};
use crate::metadata::settings::Settings;
@@ -573,8 +573,10 @@ impl MergedOptions<'_> {
system: &dyn System,
vendored: &VendoredFileSystem,
strategy: &Strategy,
) -> Result<(ProgramSettings, Vec<ProgramSettingsDiagnostic>), Strategy::Error<anyhow::Error>>
{
) -> Result<
(ProgramSettings, Vec<ProgramSettingsDiagnostic>),
Strategy::Error<ToProgramSettingsError>,
> {
self.options.to_program_settings(
OptionsContext::Project(self.metadata.root()),
self.metadata.name(),
@@ -591,6 +593,7 @@ impl MergedOptions<'_> {
) -> anyhow::Result<Option<PythonEnvironment>> {
self.options
.python_environment(self.metadata.root(), system)
.map_err(anyhow::Error::from)
}
pub fn to_settings<Strategy: MisconfigurationStrategy>(
crates/ty_project/src/metadata/options.rs
+82 -19
View File
@@ -6,7 +6,6 @@ use crate::metadata::settings::{OverrideSettings, SrcSettings};
use super::settings::{Override, Settings, TerminalSettings};
use crate::metadata::value::{RelativeGlobPattern, RelativePathBuf};
use anyhow::Context;
use ordermap::OrderMap;
use pep440_rs::VersionSpecifiers;
use ruff_db::RustDoc;
@@ -20,6 +19,7 @@ use ruff_macros::{Combine, OptionsMetadata, RustDoc};
use ruff_options_metadata::{OptionSet, OptionsMetadata, Visit};
use ruff_python_ast::PythonVersion;
use ruff_ranged_value::{RangedValue, ValueSource, ValueSourceGuard};
use ruff_text_size::TextRange;
use rustc_hash::FxHasher;
use serde::{Deserialize, Serialize};
use std::borrow::Cow;
@@ -39,7 +39,7 @@ use ty_python_core::program::{MisconfigurationStrategy, ProgramSettings};
use ty_python_semantic::lint::{Level, LintSource, RuleSelection};
use ty_python_semantic::{
AnalysisSettings, PythonEnvironment, PythonVersionFileSource, PythonVersionSource,
PythonVersionWithSource, SitePackagesPaths, SysPrefixPathOrigin,
PythonVersionWithSource, SitePackagesDiscoveryError, SitePackagesPaths, SysPrefixPathOrigin,
inferred_python_version_source_annotation,
};
use ty_static::EnvVars;
@@ -188,8 +188,10 @@ impl Options {
system: &dyn System,
vendored: &VendoredFileSystem,
strategy: &Strategy,
) -> Result<(ProgramSettings, Vec<ProgramSettingsDiagnostic>), Strategy::Error<anyhow::Error>>
{
) -> Result<
(ProgramSettings, Vec<ProgramSettingsDiagnostic>),
Strategy::Error<ToProgramSettingsError>,
> {
let mut diagnostics = Vec::new();
let environment = self.environment.or_default();
@@ -210,8 +212,8 @@ impl Options {
let python_environment = match self.python_environment(context.configuration_root(), system)
{
Ok(None) => PythonEnvironment::discover(context.project_root(), system)
.context("Failed to discover local Python environment"),
configured => configured,
.map_err(ToProgramSettingsError::PythonEnvironmentDiscovery),
configured => configured.map_err(ToProgramSettingsError::PythonEnvironment),
};
// If in safe-mode, fallback to None if this fails instead of erroring.
@@ -232,7 +234,7 @@ impl Options {
let site_packages_paths = if let Some(python_environment) = python_environment.as_ref() {
let site_packages_paths = python_environment
.site_packages_paths(system)
.context("Failed to discover the site-packages directory");
.map_err(ToProgramSettingsError::SitePackagesDiscovery);
let site_packages_paths = strategy.fallback(site_packages_paths, |_| {
tracing::debug!("Default settings failed to discover site-packages directory");
SitePackagesPaths::default()
@@ -281,16 +283,18 @@ impl Options {
.and_then(|resolution| resolution.into_program_version(&mut diagnostics))
.unwrap_or_default();
// Safe mode is handled inside this function, so we just assume this can't fail
let search_paths = strategy.to_anyhow(self.to_search_paths(
context,
project_name,
site_packages_paths,
real_stdlib_path,
system,
vendored,
strategy,
))?;
let search_paths = strategy.map_err(
self.to_search_paths(
context,
project_name,
site_packages_paths,
real_stdlib_path,
system,
vendored,
strategy,
),
ToProgramSettingsError::SearchPaths,
)?;
tracing::info!(
"Python version: Python {python_version}, platform: {python_platform}",
@@ -312,7 +316,7 @@ impl Options {
&self,
configuration_root: &SystemPath,
system: &dyn System,
) -> anyhow::Result<Option<PythonEnvironment>> {
) -> Result<Option<PythonEnvironment>, SitePackagesDiscoveryError> {
let environment = self.environment.or_default();
let Some(python_path) = environment.python.as_ref() else {
return Ok(None);
@@ -333,7 +337,6 @@ impl Options {
origin,
system,
)
.map_err(anyhow::Error::from)
.map(Some)
}
@@ -2151,6 +2154,66 @@ pub(super) struct InnerOverrideOptions {
pub(super) analysis: Option<AnalysisOptions>,
}
/// A failure to resolve a project's or standalone script's program settings.
#[derive(Debug, Error)]
pub enum ToProgramSettingsError {
/// The explicitly configured Python environment could not be resolved.
#[error(transparent)]
PythonEnvironment(SitePackagesDiscoveryError),
/// No explicitly configured Python environment was available, and discovery failed.
#[error("Failed to discover local Python environment")]
PythonEnvironmentDiscovery(#[source] SitePackagesDiscoveryError),
/// The resolved Python environment did not contain usable site-packages directories.
#[error("Failed to discover the site-packages directory")]
SitePackagesDiscovery(#[source] SitePackagesDiscoveryError),
/// One of the configured Python module search paths could not be resolved.
#[error(transparent)]
SearchPaths(#[from] SearchPathSettingsError),
}
impl ToProgramSettingsError {
/// Returns the program-settings error without its optional diagnostic detail.
pub(crate) fn message(&self) -> String {
self.to_string()
}
/// Returns details for failures whose message only identifies the failed operation.
pub(crate) fn hint(&self) -> Option<String> {
match self {
Self::PythonEnvironmentDiscovery(error) | Self::SitePackagesDiscovery(error) => {
Some(error.to_string())
}
Self::PythonEnvironment(_) | Self::SearchPaths(_) => None,
}
}
pub(crate) fn setting_source<'a>(
&self,
options: &'a Options,
) -> Option<(&'a ValueSource, Option<TextRange>)> {
let environment = options.environment.as_ref()?;
match self {
Self::PythonEnvironment(_) | Self::SitePackagesDiscovery(_) => environment
.python
.as_ref()
.map(|setting| (setting.source(), setting.range())),
Self::SearchPaths(
SearchPathSettingsError::FailedToReadVersionsFile { .. }
| SearchPathSettingsError::VersionsParseError(_),
) => environment
.typeshed
.as_ref()
.map(|setting| (setting.source(), setting.range())),
Self::PythonEnvironmentDiscovery(_)
| Self::SearchPaths(SearchPathSettingsError::InvalidSearchPath(_)) => None,
}
}
}
/// Error returned when the settings can't be resolved because of a hard error.
#[derive(Debug)]
pub struct ToSettingsError {
crates/ty_project/src/metadata/pyproject.rs
+51 -9
View File
@@ -5,6 +5,7 @@ use ruff_python_ast::PythonVersion;
use ruff_ranged_value::{RangedValue, ValueSource, ValueSourceGuard};
use serde::{Deserialize, Deserializer, Serialize};
use std::collections::Bound;
use std::fmt;
use std::ops::Deref;
use strum::IntoEnumIterator;
use thiserror::Error;
@@ -130,22 +131,63 @@ pub(super) fn resolve_requires_python_lower_bound(
))
}
#[derive(Debug, Error)]
#[derive(Debug)]
pub enum ResolveRequiresPythonError {
#[error("The major version `{0}` is larger than the maximum supported value 255")]
TooLargeMajor(u64),
#[error("The minor version `{0}` is larger than the maximum supported value 255")]
TooLargeMinor(u64),
#[error(
"value `{0}` does not contain a lower bound. Add a lower bound to indicate the minimum compatible Python version (e.g., `>=3.13`) or specify a version in `environment.python-version`."
)]
NoLowerBound(String),
#[error(
"value `{0}` does not include any Python version supported by ty. Adjust `requires-python` to include a supported Python 3 version or specify `environment.python-version` explicitly."
)]
NoSupportedVersion(String),
}
impl ResolveRequiresPythonError {
/// Returns the error without its optional recovery guidance.
pub fn message(&self) -> String {
match self {
Self::TooLargeMajor(version) => {
format!(
"The major version `{version}` is larger than the maximum supported value 255"
)
}
Self::TooLargeMinor(version) => {
format!(
"The minor version `{version}` is larger than the maximum supported value 255"
)
}
Self::NoLowerBound(version) => {
format!("value `{version}` does not contain a lower bound")
}
Self::NoSupportedVersion(version) => {
format!("value `{version}` does not include any Python version supported by ty")
}
}
}
/// Returns guidance for fixing the invalid Python requirement, when available.
pub fn hint(&self) -> Option<&'static str> {
match self {
Self::NoLowerBound(_) => Some(
"Add a lower bound to indicate the minimum compatible Python version (e.g., `>=3.13`) or specify a version in `environment.python-version`.",
),
Self::NoSupportedVersion(_) => Some(
"Adjust `requires-python` to include a supported Python 3 version or specify `environment.python-version` explicitly.",
),
Self::TooLargeMajor(_) | Self::TooLargeMinor(_) => None,
}
}
}
impl fmt::Display for ResolveRequiresPythonError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str(&self.message())?;
if let Some(hint) = self.hint() {
write!(f, ". {hint}")?;
}
Ok(())
}
}
impl std::error::Error for ResolveRequiresPythonError {}
#[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)]
#[serde(rename_all = "kebab-case")]
pub struct Tool {
crates/ty_project/src/script.rs
+180 -65
View File
@@ -1,13 +1,16 @@
use pep440_rs::VersionSpecifiers;
use ruff_db::Db as SourceDb;
use ruff_db::diagnostic::Diagnostic;
use ruff_db::diagnostic::{
Annotation, Diagnostic, DiagnosticId, Severity, Span, SubDiagnostic, SubDiagnosticSeverity,
};
use ruff_db::files::File;
use ruff_db::source::source_text;
use ruff_python_ast::script::ScriptTag;
use ruff_ranged_value::{RangedValue, ValueSource, ValueSourceGuard};
use ruff_text_size::{TextRange, TextSize};
use serde::Deserialize;
use ty_combine::Combine;
use ty_python_core::program::{FallibleStrategy, Program, ProgramSettings};
use ty_python_core::program::{FallibleStrategy, Program, ProgramSettings, UseDefaultStrategy};
use ty_python_semantic::PythonVersionWithSource;
use crate::metadata::options::{Options, OptionsContext};
@@ -33,17 +36,28 @@ pub(crate) struct Script<'db> {
#[returns(ref)]
pub(crate) python_version_with_source: PythonVersionWithSource,
/// Whether the script's metadata, settings, and Python environment resolved without errors.
///
/// For a script with invalid settings, `Program` is a best effort approximation
/// of the script's configuration. It's, therefore, important that ty doesn't run any destructive
/// operations or shows misleading diagnostics. That means, `--fix` should be a no-op and
/// `check_file` (and similar operations) should bail and only show the setting related diagnostics.
#[tracked]
#[returns(copy)]
pub(crate) has_valid_settings: bool,
/// Diagnostics generated while parsing the script metadata and resolving its settings.
#[tracked]
#[returns(deref)]
pub(crate) diagnostics: Box<[Diagnostic]>,
pub(crate) settings_diagnostics: Box<[Diagnostic]>,
}
impl<'db> Script<'db> {
/// Returns the script for `file` without creating a second Salsa memo for ordinary files.
pub(crate) fn for_file(db: &'db dyn Db, file: File) -> Option<Self> {
// Most files are not scripts. Check the existing metadata query first so ordinary files
// Most files are not scripts. Check the existing tag query first so ordinary files
// do not also allocate a tracked `script` memo just to cache another `None`.
script_metadata(db, file)?;
script_tag(db, file)?;
script(db, file)
}
}
@@ -54,13 +68,16 @@ impl get_size2::GetSize for Script<'_> {}
#[salsa::tracked(returns(copy), heap_size=ruff_memory_usage::heap_size)]
pub(crate) fn script(db: &dyn Db, file: File) -> Option<Script<'_>> {
// Files without script metadata must not depend on the low-durability open-file set.
let metadata = script_metadata(db, file)?;
let tag = script_tag(db, file)?;
// Never treat third-party files as scripts.
if !crate::should_check_file(db, file) {
return None;
}
let mut diagnostics = ScriptConfigurationDiagnostics::default();
let metadata = parse_script_metadata(file, tag, &mut diagnostics);
let configuration_root = file
.path(db)
.as_system_path()
@@ -70,18 +87,16 @@ pub(crate) fn script(db: &dyn Db, file: File) -> Option<Script<'_>> {
let project_metadata = db.project().metadata(db);
let mut diagnostics = Vec::new();
// FIXME: Report configuration errors as diagnostics and skip checking the script entirely so
// that fixes cannot be applied using the enclosing project's configuration.
let options = resolve_script_options(project_metadata, metadata)?;
let settings = resolve_script_settings(db, &options, context, &mut diagnostics)?;
let options = resolve_script_options(project_metadata, &metadata, file, &mut diagnostics);
let settings = resolve_script_settings(db, &options, context, &mut diagnostics);
let program_settings = resolve_script_program_settings(
db,
&options,
context,
project_metadata.name(),
file,
&mut diagnostics,
)?;
);
program_settings.search_paths.try_register_static_roots(db);
@@ -93,20 +108,79 @@ pub(crate) fn script(db: &dyn Db, file: File) -> Option<Script<'_>> {
settings,
program,
program_settings.python_version,
diagnostics.into_boxed_slice(),
!diagnostics.has_invalid_settings,
diagnostics.diagnostics.into_boxed_slice(),
))
}
/// Returns the PEP 723 script tag embedded in `file`.
///
/// Most files have no script tag. Boxing keeps the cached result compact when it is `None`.
#[salsa::tracked(returns(as_deref))]
pub(crate) fn script_tag(db: &dyn SourceDb, file: File) -> Option<Box<ScriptTag>> {
let path = file.path(db);
if path.is_vendored_path() {
return None;
}
let source = source_text(db, file);
if source.is_notebook() {
return None;
}
ScriptTag::parse(source.as_bytes()).map(Box::new)
}
fn parse_script_metadata(
file: File,
tag: &ScriptTag,
diagnostics: &mut ScriptConfigurationDiagnostics,
) -> ScriptMetadata {
let result = {
let _guard = ValueSourceGuard::with_source_map(
ValueSource::ScriptMetadata(file),
tag.source_map().clone(),
);
toml::from_str::<ScriptMetadata>(tag.metadata())
};
let mut metadata = match result {
Ok(metadata) => metadata,
Err(error) => {
let range = error.span().and_then(|span| {
let start = TextSize::try_from(span.start).ok()?;
let end = TextSize::try_from(span.end).ok()?;
Some(tag.source_map().map_range(TextRange::new(start, end)))
});
diagnostics.report_invalid(invalid_script_metadata_diagnostic(
file,
error.message(),
range,
));
return ScriptMetadata::default();
}
};
if let Some(options) = metadata.tool.as_mut().and_then(|tool| tool.ty.as_mut()) {
options.prioritize_all_selectors();
}
metadata
}
fn resolve_script_options(
project_metadata: &ProjectMetadata,
metadata: &ScriptMetadata,
) -> Option<Options> {
file: File,
diagnostics: &mut ScriptConfigurationDiagnostics,
) -> Options {
// When using `--config-file <FILE>`, use the settings from `<FILE>`
let inline = if project_metadata.config_file_override().is_some() {
project_metadata.options().clone()
} else {
// Otherwise use the script's settings.
metadata.to_options()?
metadata.to_options(file, diagnostics)
};
let mut options = Options::default();
@@ -122,23 +196,30 @@ fn resolve_script_options(
.root
.get_or_insert_default();
Some(options)
options
}
fn resolve_script_settings(
db: &dyn Db,
options: &Options,
context: OptionsContext<'_>,
diagnostics: &mut Vec<Diagnostic>,
) -> Option<Settings> {
let (settings, settings_diagnostics) =
options.to_settings(db, context, &FallibleStrategy).ok()?;
diagnostics: &mut ScriptConfigurationDiagnostics,
) -> Settings {
let (settings, settings_diagnostics) = match options.to_settings(db, context, &FallibleStrategy)
{
Ok(settings) => settings,
Err(error) => {
diagnostics.report_invalid(error.into_diagnostic().to_diagnostic());
let Ok(settings) = options.to_settings(db, context, &UseDefaultStrategy);
settings
}
};
diagnostics.extend(
settings_diagnostics
.into_iter()
.map(|diagnostic| diagnostic.to_diagnostic()),
);
Some(settings)
settings
}
fn resolve_script_program_settings(
@@ -146,54 +227,50 @@ fn resolve_script_program_settings(
options: &Options,
context: OptionsContext<'_>,
project_name: &str,
diagnostics: &mut Vec<Diagnostic>,
) -> Option<ProgramSettings> {
let (settings, settings_diagnostics) = options
.to_program_settings(
context,
project_name,
db.system(),
db.vendored(),
&FallibleStrategy,
)
.ok()?;
file: File,
diagnostics: &mut ScriptConfigurationDiagnostics,
) -> ProgramSettings {
let (settings, settings_diagnostics) = match options.to_program_settings(
context,
project_name,
db.system(),
db.vendored(),
&FallibleStrategy,
) {
Ok(settings) => settings,
Err(error) => {
let (source_file, range) = error
.setting_source(options)
.and_then(|(source, range)| source.file(db).map(|file| (file, range)))
.unwrap_or((file, None));
let mut diagnostic =
invalid_script_metadata_diagnostic(source_file, error.message(), range);
if let Some(hint) = error.hint() {
diagnostic.sub(SubDiagnostic::new(SubDiagnosticSeverity::Info, hint));
}
diagnostics.report_invalid(diagnostic);
let Ok(settings) = options.to_program_settings(
context,
project_name,
db.system(),
db.vendored(),
&UseDefaultStrategy,
);
settings
}
};
diagnostics.extend(
settings_diagnostics
.into_iter()
.map(|diagnostic| diagnostic.into_diagnostic(db).to_diagnostic()),
);
Some(settings)
}
/// Returns the PEP 723 metadata embedded in `file`.
///
/// Most files have no script metadata. Boxing keeps the cached result compact when it is `None`.
#[salsa::tracked(returns(as_deref))]
pub(crate) fn script_metadata(db: &dyn SourceDb, file: File) -> Option<Box<ScriptMetadata>> {
let path = file.path(db);
if path.is_vendored_path() {
return None;
}
let source = source_text(db, file);
if source.is_notebook() {
return None;
}
let (content, source_map) = ScriptTag::parse(source.as_bytes())?.into_metadata_and_source_map();
let _guard = ValueSourceGuard::with_source_map(ValueSource::ScriptMetadata(file), source_map);
// FIXME: Report invalid TOML in script metadata instead of silently ignoring it.
let mut metadata: ScriptMetadata = toml::from_str(&content).ok()?;
if let Some(options) = metadata.tool.as_mut().and_then(|tool| tool.ty.as_mut()) {
options.prioritize_all_selectors();
}
Some(Box::new(metadata))
settings
}
/// PEP 723 metadata, whose Python requirement belongs at the top level rather than in `project`.
#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
#[derive(Debug, Default, Clone, PartialEq, Eq, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub(crate) struct ScriptMetadata {
requires_python: Option<RangedValue<VersionSpecifiers>>,
@@ -201,12 +278,17 @@ pub(crate) struct ScriptMetadata {
}
impl ScriptMetadata {
fn to_options(&self) -> Option<Options> {
fn to_options(&self, file: File, diagnostics: &mut ScriptConfigurationDiagnostics) -> Options {
let mut options = self.ty().cloned().unwrap_or_default();
if let Err(error) = options.apply_requires_python(self.requires_python.as_ref()) {
let range = self.requires_python.as_ref().and_then(RangedValue::range);
let mut diagnostic = invalid_script_metadata_diagnostic(file, error.message(), range);
if let Some(hint) = error.hint() {
diagnostic.sub(SubDiagnostic::new(SubDiagnosticSeverity::Info, hint));
}
diagnostics.report_invalid(diagnostic);
}
options
.apply_requires_python(self.requires_python.as_ref())
.ok()?;
Some(options)
}
fn ty(&self) -> Option<&Options> {
@@ -214,6 +296,39 @@ impl ScriptMetadata {
}
}
#[derive(Default)]
struct ScriptConfigurationDiagnostics {
diagnostics: Vec<Diagnostic>,
has_invalid_settings: bool,
}
impl ScriptConfigurationDiagnostics {
fn report_invalid(&mut self, diagnostic: Diagnostic) {
self.has_invalid_settings = true;
self.diagnostics.push(diagnostic);
}
fn extend(&mut self, diagnostics: impl IntoIterator<Item = Diagnostic>) {
self.diagnostics.extend(diagnostics);
}
}
fn invalid_script_metadata_diagnostic(
file: File,
message: impl std::fmt::Display,
range: Option<TextRange>,
) -> Diagnostic {
let mut diagnostic = Diagnostic::new(
DiagnosticId::InvalidScriptMetadata,
Severity::Error,
message,
);
diagnostic.annotate(Annotation::primary(
Span::from(file).with_optional_range(range),
));
diagnostic
}
#[cfg(test)]
mod tests {
use ruff_db::files::system_path_to_file;
crates/ty_project/src/walk.rs
+2 -2
View File
@@ -1,5 +1,5 @@
use crate::glob::IncludeExcludeFilter;
use crate::script::script_metadata;
use crate::script::script_tag;
use crate::{Db, GlobFilterCheckMode, IncludeResult, Project};
use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity};
use ruff_db::files::{File, system_path_to_file};
@@ -263,7 +263,7 @@ impl ProjectFilesWalker {
if let Ok(file) = system_path_to_file(&*db, entry.path()) {
if entry.depth() > 0
&& exclude_scripts
&& script_metadata(&*db, file).is_some()
&& script_tag(&*db, file).is_some()
{
tracing::debug!(
"Ignoring implicitly discovered PEP 723 script `{path}` \
crates/ty_python_semantic/src/lib.rs
+1 -1
View File
@@ -39,7 +39,7 @@ use ty_python_core::{
};
pub use ty_site_packages::{
PythonEnvironment, PythonVersionFileSource, PythonVersionSource, PythonVersionWithSource,
SitePackagesPaths, SysPrefixPathOrigin,
SitePackagesDiscoveryError, SitePackagesPaths, SysPrefixPathOrigin,
};
pub use types::ide_support::{
ImplementationsFinder, ImportAliasResolution, ResolvedDefinition, TypeHierarchyClass,
crates/ty_server/tests/e2e/hover.rs
+103
View File
@@ -40,6 +40,109 @@ fn supports_only_plain_text() -> Result<()> {
Ok(())
}
#[test]
fn invalid_script_environment_retains_configured_platform_for_hover() -> Result<()> {
let workspace_root = SystemPath::new("src");
let script = SystemPath::new("src/script.py");
let content = r#"# /// script
# [tool.ty.environment]
# python = "./missing-environment"
# python-version = "3.12"
# python-platform = "win32"
# ///
import sys
sys.platform
"#;
let mut server = TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.with_file(
"src/pyproject.toml",
r#"[tool.ty.environment]
python-platform = "linux"
"#,
)?
.with_file(script, content)?
.build()
.wait_until_workspaces_are_initialized();
server.open_text_document(script, content, 1);
let hover = server.hover_request(script, Position::new(8, 5));
insta::assert_json_snapshot!(hover, @r#"
{
"contents": {
"kind": "plaintext",
"value": "Literal[\"win32\"]"
},
"range": {
"start": {
"line": 8,
"character": 4
},
"end": {
"line": 8,
"character": 12
}
}
}
"#);
Ok(())
}
#[test]
fn invalid_script_retains_valid_environment_settings_for_hover() -> Result<()> {
let workspace_root = SystemPath::new("src");
let script = SystemPath::new("src/script.py");
let content = r#"# /// script
# requires-python = "<3.12"
# [tool.ty.environment]
# python-platform = "win32"
# ///
import sys
sys.platform
"#;
let mut server = TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.with_file(
"src/pyproject.toml",
r#"[tool.ty.environment]
python-platform = "linux"
"#,
)?
.with_file(script, content)?
.build()
.wait_until_workspaces_are_initialized();
server.open_text_document(script, content, 1);
let hover = server.hover_request(script, Position::new(7, 5));
insta::assert_json_snapshot!(hover, @r#"
{
"contents": {
"kind": "plaintext",
"value": "Literal[\"win32\"]"
},
"range": {
"start": {
"line": 7,
"character": 4
},
"end": {
"line": 7,
"character": 12
}
}
}
"#);
Ok(())
}
#[test]
fn shared_import_hover_uses_each_script_python_version() -> Result<()> {
let workspace_root = SystemPath::new("src");
crates/ty_server/tests/e2e/publish_diagnostics.rs
+99
View File
@@ -235,6 +235,80 @@ def foo() -> str:
Ok(())
}
#[test]
fn on_did_open_invalid_script_reports_only_configuration_diagnostics() -> Result<()> {
let workspace_root = SystemPath::new("src");
let script = SystemPath::new("src/script.py");
let content = r#"# /// script
# requires-python =
# ///
def function():
unused = 1
return missing
"#;
let mut server = TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.with_file(script, content)?
.enable_pull_diagnostics(false)
.build()
.wait_until_workspaces_are_initialized();
server.open_text_document(script, content, 1);
let diagnostics = server.await_notification::<PublishDiagnosticsNotification>();
insta::assert_debug_snapshot!(diagnostics);
Ok(())
}
#[test]
fn on_did_change_invalid_script_metadata_restores_semantic_diagnostics() -> Result<()> {
let workspace_root = SystemPath::new("src");
let script = SystemPath::new("src/script.py");
let invalid = r#"# /// script
# requires-python =
# ///
missing
"#;
let valid = r#"# /// script
# requires-python = ">=3.12"
# ///
missing
"#;
let mut server = TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.with_file(script, invalid)?
.enable_pull_diagnostics(false)
.build()
.wait_until_workspaces_are_initialized();
server.open_text_document(script, invalid, 1);
let initial = server.await_notification::<PublishDiagnosticsNotification>();
insta::assert_debug_snapshot!(initial);
server.change_text_document(
script,
vec![
lsp_types::TextDocumentContentChangeEvent::TextDocumentContentChangeWholeDocument(
TextDocumentContentChangeWholeDocument {
text: valid.to_string(),
},
),
],
2,
);
let updated = server.await_notification::<PublishDiagnosticsNotification>();
insta::assert_debug_snapshot!(updated);
Ok(())
}
#[test]
fn on_did_change_script_python_requirement() -> Result<()> {
let workspace_root = SystemPath::new("src");
@@ -299,6 +373,31 @@ PythonFinalizationError
Ok(())
}
#[test]
fn on_did_open_virtual_script_reports_invalid_metadata() -> Result<()> {
let workspace_root = SystemPath::new("src");
let script = SystemVirtualPath::new("untitled:script.py");
let content = r#"# /// script
# requires-python =
# ///
missing
"#;
let mut server = TestServerBuilder::new()?
.with_workspace(workspace_root, None)?
.enable_pull_diagnostics(false)
.build()
.wait_until_workspaces_are_initialized();
server.open_virtual_text_document(script, content, 1)?;
let diagnostics = server.await_notification::<PublishDiagnosticsNotification>();
insta::assert_debug_snapshot!(diagnostics);
Ok(())
}
#[test]
fn on_did_open_virtual_script_uses_its_python_requirement() -> Result<()> {
let workspace_root = SystemPath::new("src");
crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_change_invalid_script_metadata_restores_semantic_diagnostics-2.snap
+72
View File
@@ -0,0 +1,72 @@
---
source: crates/ty_server/tests/e2e/publish_diagnostics.rs
expression: updated
---
PublishDiagnosticsParams {
uri: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "<temp_dir>/src/script.py",
query: None,
fragment: None,
},
version: Some(
2,
),
diagnostics: [
Diagnostic {
range: Range {
start: Position {
line: 4,
character: 0,
},
end: Position {
line: 4,
character: 7,
},
},
severity: Some(
Error,
),
code: Some(
String(
"unresolved-reference",
),
),
code_description: Some(
CodeDescription {
href: Url {
scheme: "https",
cannot_be_a_base: false,
username: "",
password: None,
host: Some(
Domain(
"ty.dev",
),
),
port: None,
path: "/rules",
query: None,
fragment: Some(
"unresolved-reference",
),
},
},
),
source: Some(
"ty",
),
message: String(
"Name `missing` used when not defined",
),
tags: None,
related_information: None,
data: None,
},
],
}
crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_change_invalid_script_metadata_restores_semantic_diagnostics.snap
+52
View File
@@ -0,0 +1,52 @@
---
source: crates/ty_server/tests/e2e/publish_diagnostics.rs
expression: initial
---
PublishDiagnosticsParams {
uri: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "<temp_dir>/src/script.py",
query: None,
fragment: None,
},
version: Some(
1,
),
diagnostics: [
Diagnostic {
range: Range {
start: Position {
line: 1,
character: 19,
},
end: Position {
line: 1,
character: 19,
},
},
severity: Some(
Error,
),
code: Some(
String(
"invalid-script-metadata",
),
),
code_description: None,
source: Some(
"ty",
),
message: String(
"string values must be quoted, expected literal string",
),
tags: None,
related_information: None,
data: None,
},
],
}
crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_open_invalid_script_reports_only_configuration_diagnostics.snap
+52
View File
@@ -0,0 +1,52 @@
---
source: crates/ty_server/tests/e2e/publish_diagnostics.rs
expression: diagnostics
---
PublishDiagnosticsParams {
uri: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "<temp_dir>/src/script.py",
query: None,
fragment: None,
},
version: Some(
1,
),
diagnostics: [
Diagnostic {
range: Range {
start: Position {
line: 1,
character: 19,
},
end: Position {
line: 1,
character: 19,
},
},
severity: Some(
Error,
),
code: Some(
String(
"invalid-script-metadata",
),
),
code_description: None,
source: Some(
"ty",
),
message: String(
"string values must be quoted, expected literal string",
),
tags: None,
related_information: None,
data: None,
},
],
}
crates/ty_server/tests/e2e/snapshots/e2e__publish_diagnostics__on_did_open_virtual_script_reports_invalid_metadata.snap
+52
View File
@@ -0,0 +1,52 @@
---
source: crates/ty_server/tests/e2e/publish_diagnostics.rs
expression: diagnostics
---
PublishDiagnosticsParams {
uri: Url {
scheme: "untitled",
cannot_be_a_base: true,
username: "",
password: None,
host: None,
port: None,
path: "script.py",
query: None,
fragment: None,
},
version: Some(
1,
),
diagnostics: [
Diagnostic {
range: Range {
start: Position {
line: 1,
character: 19,
},
end: Position {
line: 1,
character: 19,
},
},
severity: Some(
Error,
),
code: Some(
String(
"invalid-script-metadata",
),
),
code_description: None,
source: Some(
"ty",
),
message: String(
"string values must be quoted, expected literal string",
),
tags: None,
related_information: None,
data: None,
},
],
}