diff --git a/crates/ruff/tests/cli/lint.rs b/crates/ruff/tests/cli/lint.rs index 04cabca512cf8c..ec0bbc4f477b24 100644 --- a/crates/ruff/tests/cli/lint.rs +++ b/crates/ruff/tests/cli/lint.rs @@ -1126,6 +1126,35 @@ import os Ok(()) } +#[test] +fn required_version_fails_to_parse() -> Result<()> { + let fixture = CliTest::with_file( + "ruff.toml", + r#" +required-version = "pikachu" +"#, + )?; + assert_cmd_snapshot!(fixture + .check_command(), @r#" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + ruff failed + Cause: Failed to load configuration `[TMP]/ruff.toml` + Cause: Failed to parse [TMP]/ruff.toml + Cause: TOML parse error at line 2, column 20 + | + 2 | required-version = "pikachu" + | ^^^^^^^^^ + Failed to parse version: Unexpected end of version specifier, expected operator: + pikachu + ^^^^^^^ + "#); + Ok(()) +} + #[test] fn required_version_exact_mismatch() -> Result<()> { let version = env!("CARGO_PKG_VERSION"); @@ -1137,10 +1166,10 @@ required-version = "0.1.0" "#, )?; - insta::with_settings!({ - filters => vec![(version, "[VERSION]")] - }, { - assert_cmd_snapshot!(fixture + let mut settings = insta::Settings::clone_current(); + settings.add_filter(version, "[VERSION]"); + settings.bind(|| { + assert_cmd_snapshot!(fixture .check_command() .arg("--config") .arg("ruff.toml") @@ -1154,6 +1183,7 @@ import os ----- stderr ----- ruff failed + Cause: Failed to load configuration `[TMP]/ruff.toml` Cause: Required version `==0.1.0` does not match the running version `[VERSION]` "); }); @@ -1212,10 +1242,10 @@ required-version = ">{version}" ), )?; - insta::with_settings!({ - filters => vec![(version, "[VERSION]")] - }, { - assert_cmd_snapshot!(fixture + let mut settings = insta::Settings::clone_current(); + settings.add_filter(version, "[VERSION]"); + settings.bind(|| { + assert_cmd_snapshot!(fixture .check_command() .arg("--config") .arg("ruff.toml") @@ -1229,6 +1259,48 @@ import os ----- stderr ----- ruff failed + Cause: Failed to load configuration `[TMP]/ruff.toml` + Cause: Required version `>[VERSION]` does not match the running version `[VERSION]` + "); + }); + + Ok(()) +} + +#[test] +fn required_version_precedes_rule_validation() -> Result<()> { + let version = env!("CARGO_PKG_VERSION"); + + let fixture = CliTest::with_file( + "ruff.toml", + &format!( + r#" +required-version = ">{version}" + +[lint] +select = ["RUF999"] +"# + ), + )?; + + let mut settings = insta::Settings::clone_current(); + settings.add_filter(version, "[VERSION]"); + settings.bind(|| { + assert_cmd_snapshot!(fixture + .check_command() + .arg("--config") + .arg("ruff.toml") + .arg("-") + .pass_stdin(r#" +import os +"#), @" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + ruff failed + Cause: Failed to load configuration `[TMP]/ruff.toml` Cause: Required version `>[VERSION]` does not match the running version `[VERSION]` "); }); diff --git a/crates/ruff_linter/src/settings/types.rs b/crates/ruff_linter/src/settings/types.rs index 05cd13909b69bf..50793cd761253f 100644 --- a/crates/ruff_linter/src/settings/types.rs +++ b/crates/ruff_linter/src/settings/types.rs @@ -604,13 +604,21 @@ impl TryFrom for RequiredVersion { type Error = pep440_rs::VersionSpecifiersParseError; fn try_from(value: String) -> Result { + value.parse() + } +} + +impl FromStr for RequiredVersion { + type Err = pep440_rs::VersionSpecifiersParseError; + + fn from_str(value: &str) -> Result { // Treat `0.3.1` as `==0.3.1`, for backwards compatibility. - if let Ok(version) = pep440_rs::Version::from_str(&value) { + if let Ok(version) = pep440_rs::Version::from_str(value) { Ok(Self(VersionSpecifiers::from( VersionSpecifier::equals_version(version), ))) } else { - Ok(Self(VersionSpecifiers::from_str(&value)?)) + Ok(Self(VersionSpecifiers::from_str(value)?)) } } } diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index bf50749a45dc66..77d64a3f665172 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -7,7 +7,6 @@ use std::collections::BTreeMap; use std::env::VarError; use std::num::{NonZeroU8, NonZeroU16}; use std::path::{Path, PathBuf}; -use std::str::FromStr; use anyhow::{Context, Result, anyhow}; use glob::{GlobError, Paths, PatternError, glob}; @@ -36,8 +35,7 @@ use ruff_linter::settings::{ DEFAULT_SELECTORS, DUMMY_VARIABLE_RGX, LinterSettings, TASK_TAGS, TargetVersion, }; use ruff_linter::{ - RUFF_PKG_VERSION, RuleSelector, fs, warn_user_once, warn_user_once_by_id, - warn_user_once_by_message, + RuleSelector, fs, warn_user_once, warn_user_once_by_id, warn_user_once_by_message, }; use ruff_python_ast as ast; use ruff_python_formatter::{ @@ -53,6 +51,7 @@ use crate::options::{ Flake8UnusedArgumentsOptions, FormatOptions, IsortOptions, LintCommonOptions, LintOptions, McCabeOptions, Options, Pep8NamingOptions, PyUpgradeOptions, PycodestyleOptions, PydoclintOptions, PydocstyleOptions, PyflakesOptions, PylintOptions, RuffOptions, + validate_required_version, }; use crate::settings::{ EXCLUDE, FileResolverSettings, FormatterSettings, INCLUDE, INCLUDE_PREVIEW, LineEnding, @@ -155,13 +154,7 @@ pub struct Configuration { impl Configuration { pub fn into_settings(self, project_root: &Path) -> Result { if let Some(required_version) = &self.required_version { - let ruff_pkg_version = pep440_rs::Version::from_str(RUFF_PKG_VERSION) - .expect("RUFF_PKG_VERSION is not a valid PEP 440 version specifier"); - if !required_version.contains(&ruff_pkg_version) { - return Err(anyhow!( - "Required version `{required_version}` does not match the running version `{RUFF_PKG_VERSION}`" - )); - } + validate_required_version(required_version)?; } let linter_target_version = TargetVersion(self.target_version); diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index 087914d8f8cc4e..f2c15755c170a5 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -1,15 +1,19 @@ +use anyhow::Result; use regex::Regex; use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; use serde::de::{self}; use serde::{Deserialize, Deserializer, Serialize}; use std::collections::{BTreeMap, BTreeSet}; use std::path::PathBuf; +use std::str::FromStr; use strum::IntoEnumIterator; use unicode_normalization::UnicodeNormalization; use crate::settings::LineEnding; use ruff_formatter::IndentStyle; use ruff_graph::Direction; +use ruff_linter::RUFF_PKG_VERSION; + use ruff_linter::line_width::{IndentWidth, LineLength}; use ruff_linter::rules::flake8_import_conventions::settings::BannedAliases; use ruff_linter::rules::flake8_pytest_style::settings::SettingsError; @@ -556,6 +560,17 @@ pub struct LintOptions { pub future_annotations: Option, } +pub fn validate_required_version(required_version: &RequiredVersion) -> anyhow::Result<()> { + let ruff_pkg_version = pep440_rs::Version::from_str(RUFF_PKG_VERSION) + .expect("RUFF_PKG_VERSION is not a valid PEP 440 version specifier"); + if !required_version.contains(&ruff_pkg_version) { + return Err(anyhow::anyhow!( + "Required version `{required_version}` does not match the running version `{RUFF_PKG_VERSION}`" + )); + } + Ok(()) +} + /// Newtype wrapper for [`LintCommonOptions`] that allows customizing the JSON schema and omitting the fields from the [`OptionsMetadata`]. #[derive(Clone, Debug, PartialEq, Eq, Default, Serialize, Deserialize)] #[serde(transparent)] diff --git a/crates/ruff_workspace/src/pyproject.rs b/crates/ruff_workspace/src/pyproject.rs index 53649a31e800a5..95c0893986f1c2 100644 --- a/crates/ruff_workspace/src/pyproject.rs +++ b/crates/ruff_workspace/src/pyproject.rs @@ -5,12 +5,13 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; use log::debug; use pep440_rs::{Operator, Version, VersionSpecifiers}; +use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use strum::IntoEnumIterator; -use ruff_linter::settings::types::PythonVersion; +use ruff_linter::settings::types::{PythonVersion, RequiredVersion}; -use crate::options::Options; +use crate::options::{Options, validate_required_version}; #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] struct Tools { @@ -40,20 +41,38 @@ impl Pyproject { } } -/// Parse a `ruff.toml` file. -fn parse_ruff_toml>(path: P) -> Result { +fn parse_toml, T: DeserializeOwned>(path: P, table_path: &[&str]) -> Result { let path = path.as_ref(); let contents = std::fs::read_to_string(path) .with_context(|| format!("Failed to read {}", path.display()))?; - toml::from_str(&contents).with_context(|| format!("Failed to parse {}", path.display())) + + // Parse the TOML document once into a spanned representation so we can: + // - Inspect `required-version` without triggering strict deserialization errors. + // - Deserialize with precise spans (line/column and excerpt) on errors. + let root = toml::de::DeTable::parse(&contents) + .with_context(|| format!("Failed to parse {}", path.display()))?; + + check_required_version(root.get_ref(), table_path)?; + + let deserializer = toml::de::Deserializer::from(root); + T::deserialize(deserializer) + .map_err(|mut err| { + // `Deserializer::from` doesn't have access to the original input, but we do. + // Attach it so TOML errors include line/column and a source excerpt. + err.set_input(Some(&contents)); + err + }) + .with_context(|| format!("Failed to parse {}", path.display())) +} + +/// Parse a `ruff.toml` file. +fn parse_ruff_toml>(path: P) -> Result { + parse_toml(path, &[]) } /// Parse a `pyproject.toml` file. fn parse_pyproject_toml>(path: P) -> Result { - let path = path.as_ref(); - let contents = std::fs::read_to_string(path) - .with_context(|| format!("Failed to read {}", path.display()))?; - toml::from_str(&contents).with_context(|| format!("Failed to parse {}", path.display())) + parse_toml(path, &["tool", "ruff"]) } /// Return `true` if a `pyproject.toml` contains a `[tool.ruff]` section. @@ -98,6 +117,33 @@ pub fn find_settings_toml>(path: P) -> Result> { Ok(None) } +fn check_required_version(value: &toml::de::DeTable, table_path: &[&str]) -> Result<()> { + let mut current = value; + for key in table_path { + let Some(next) = current.get(*key) else { + return Ok(()); + }; + let toml::de::DeValue::Table(next) = next.get_ref() else { + return Ok(()); + }; + current = next; + } + + let required_version = current + .get("required-version") + .and_then(|value| value.get_ref().as_str()); + + let Some(required_version) = required_version else { + return Ok(()); + }; + + // If it doesn't parse, we just fall through to normal parsing; it will give a nicer error message. + if let Ok(required_version) = required_version.parse::() { + validate_required_version(&required_version)?; + } + Ok(()) +} + /// Derive target version from `required-version` in `pyproject.toml`, if /// such a file exists in an ancestor directory. pub fn find_fallback_target_version>(path: P) -> Option {