Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 49 additions & 6 deletions crates/ruff/tests/cli/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1137,9 +1137,9 @@ required-version = "0.1.0"
"#,
)?;

insta::with_settings!({
filters => vec![(version, "[VERSION]")]
}, {
let mut settings = insta::Settings::clone_current();
settings.add_filter(version, "[VERSION]");
settings.bind(|| {
assert_cmd_snapshot!(fixture
.check_command()
.arg("--config")
Expand All @@ -1154,6 +1154,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]`
");
});
Expand Down Expand Up @@ -1212,9 +1213,50 @@ required-version = ">{version}"
),
)?;

insta::with_settings!({
filters => vec![(version, "[VERSION]")]
}, {
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]`
");
});

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")
Expand All @@ -1229,6 +1271,7 @@ import os

----- stderr -----
ruff failed
Cause: Failed to load configuration `[TMP]/ruff.toml`
Cause: Required version `>[VERSION]` does not match the running version `[VERSION]`
");
});
Expand Down
12 changes: 3 additions & 9 deletions crates/ruff_workspace/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -36,7 +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,
RuleSelector, fs, warn_user_once, warn_user_once_by_id,
warn_user_once_by_message,
};
use ruff_python_ast as ast;
Expand All @@ -53,6 +52,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,
Expand Down Expand Up @@ -155,13 +155,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);
Expand Down
15 changes: 15 additions & 0 deletions crates/ruff_workspace/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@ use std::collections::{BTreeMap, BTreeSet};
use std::path::PathBuf;
use strum::IntoEnumIterator;
use unicode_normalization::UnicodeNormalization;
use anyhow::Result;
use std::str::FromStr;

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;
Expand Down Expand Up @@ -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)]
Expand Down
34 changes: 32 additions & 2 deletions crates/ruff_workspace/src/pyproject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ use pep440_rs::{Operator, Version, VersionSpecifiers};
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 {
Expand Down Expand Up @@ -45,6 +45,7 @@ fn parse_ruff_toml<P: AsRef<Path>>(path: P) -> Result<Options> {
let path = path.as_ref();
let contents = std::fs::read_to_string(path)
.with_context(|| format!("Failed to read {}", path.display()))?;
check_required_version(&contents, path, &[])?;
toml::from_str(&contents).with_context(|| format!("Failed to parse {}", path.display()))
}

Expand All @@ -53,6 +54,7 @@ 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()))?;
check_required_version(&contents, path, &["tool", "ruff"])?;
toml::from_str(&contents).with_context(|| format!("Failed to parse {}", path.display()))
}

Expand Down Expand Up @@ -98,6 +100,34 @@ pub fn find_settings_toml<P: AsRef<Path>>(path: P) -> Result<Option<PathBuf>> {
Ok(None)
}

fn check_required_version(contents: &str, path: &Path, table_path: &[&str]) -> Result<()> {
let value: toml::Value =
toml::from_str(contents).with_context(|| format!("Failed to parse {}", path.display()))?;
let mut current = &value;
for key in table_path {
match current.get(*key) {
Some(next) => {
current = next;
}
None => return Ok(()),
}
}

let required_version = current
.get("required-version")
.or_else(|| current.get("required_version"))
.and_then(|value| value.as_str());

let Some(required_version) = required_version else {
return Ok(());
};

let required_version = RequiredVersion::try_from(required_version.to_string())
.with_context(|| format!("Failed to parse {}", path.display()))?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that we lose the span in the error message compared to when deserializing the value as part of the serde deserialization. I'm inclined to return Ok here and defer to the "full" deserialization in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a test for this case too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You were right, changed it as you suggested.

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> {
Expand Down
Loading