Check required-version before parsing rules (#22410)

Co-authored-by: Micha Reiser <[email protected]>

Author: jelle-openai

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]`
     ");
     });

crates/ruff_linter/src/settings/types.rs

@@ -607,13 +607,21 @@ impl TryFrom<String> for RequiredVersion {
     type Error = pep440_rs::VersionSpecifiersParseError;
 
     fn try_from(value: String) -> Result<Self, Self::Error> {
+        value.parse()
+    }
+}
+
+impl FromStr for RequiredVersion {
+    type Err = pep440_rs::VersionSpecifiersParseError;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
         // 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)?))
         }
     }
 }

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<Settings> {
         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);

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<bool>,
 }
 
+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)]

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<P: AsRef<Path>>(path: P) -> Result<Options> {
+fn parse_toml<P: AsRef<Path>, T: DeserializeOwned>(path: P, table_path: &[&str]) -> Result<T> {
     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<P: AsRef<Path>>(path: P) -> Result<Options> {
+    parse_toml(path, &[])
 }
 
 /// Parse a `pyproject.toml` file.
 fn parse_pyproject_toml<P: AsRef<Path>>(path: P) -> Result<Pyproject> {
-    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<P: AsRef<Path>>(path: P) -> Result<Option<PathBuf>> {
     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::<RequiredVersion>() {
+        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<P: AsRef<Path>>(path: P) -> Option<PythonVersion> {