mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-28 12:55:05 +00:00
[pytest
] Reverse PT001
and PT0023
defaults (#12106)
## Summary This patch inverts the defaults for [pytest-fixture-incorrect-parentheses-style (PT001)](https://docs.astral.sh/ruff/rules/pytest-fixture-incorrect-parentheses-style/) and [pytest-incorrect-mark-parentheses-style (PT003)](https://docs.astral.sh/ruff/rules/pytest-incorrect-mark-parentheses-style/) to prefer dropping superfluous parentheses. Presently, Ruff defaults to adding superfluous parentheses on pytest mark and fixture decorators for documented purpose of consistency; for example, ```diff import pytest -@pytest.mark.foo +@pytest.mark.foo() def test_bar(): ... ``` This behaviour is counter to the official pytest recommendation and diverges from the flake8-pytest-style plugin as of version 2.0.0 (see https://github.com/m-burst/flake8-pytest-style/issues/272). Seeing as either default satisfies the documented benefit of consistency across a codebase, it makes sense to change the behaviour to be consistent with pytest and the flake8 plugin as well. This change is breaking, so is gated behind preview (at least under my understanding of Ruff versioning). The implementation of this gating feature is a bit hacky, but seemed to be the least disruptive solution without performing invasive surgery on the `#[option()]` macro. Related to #8796. ### Caveat Whilst updating the documentation, I sought to reference the pytest recommendation to drop superfluous parentheses, but couldn't find any official instruction beyond it being a revealed preference within the pytest documentation code examples (as well as the linked issues from a core pytest developer). Thus, the wording of the preference is deliberately timid; it's to cohere with pytest rather than follow an explicit guidance. ## Test Plan `cargo nextest run` I also ran ```sh cargo run -p ruff -- check crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT001.py --no-cache --diff --select PT001 ``` and compared against it with `--preview` to verify that the default does change under preview (I also repeated this with `echo '[tool.ruff]\npreview = true' > pyproject.toml` to verify that it works with a configuration file). --------- Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This commit is contained in:
parent
d80a9d9ce9
commit
d1aeadc009
6 changed files with 47 additions and 10 deletions
|
@ -34,6 +34,9 @@ use super::helpers::{
|
|||
/// Either removing those unnecessary parentheses _or_ requiring them for all
|
||||
/// fixtures is fine, but it's best to be consistent.
|
||||
///
|
||||
/// In [preview], this rule defaults to removing unnecessary parentheses, to match
|
||||
/// the behavior of official pytest projects.
|
||||
///
|
||||
/// ## Example
|
||||
/// ```python
|
||||
/// import pytest
|
||||
|
@ -59,6 +62,8 @@ use super::helpers::{
|
|||
///
|
||||
/// ## References
|
||||
/// - [`pytest` documentation: API Reference: Fixtures](https://docs.pytest.org/en/latest/reference/reference.html#fixtures-api)
|
||||
///
|
||||
/// [preview]: https://docs.astral.sh/ruff/preview/
|
||||
#[violation]
|
||||
pub struct PytestFixtureIncorrectParenthesesStyle {
|
||||
expected: Parentheses,
|
||||
|
|
|
@ -14,6 +14,9 @@ use super::helpers::get_mark_decorators;
|
|||
/// without parentheses, depending on the [`lint.flake8-pytest-style.mark-parentheses`]
|
||||
/// setting.
|
||||
///
|
||||
/// In [preview], this rule defaults to removing unnecessary parentheses, to match
|
||||
/// the behavior of official pytest projects.
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
/// If a `@pytest.mark.<marker>()` doesn't take any arguments, the parentheses are
|
||||
/// optional.
|
||||
|
@ -46,6 +49,8 @@ use super::helpers::get_mark_decorators;
|
|||
///
|
||||
/// ## References
|
||||
/// - [`pytest` documentation: Marks](https://docs.pytest.org/en/latest/reference/reference.html#marks)
|
||||
///
|
||||
/// [preview]: https://docs.astral.sh/ruff/preview/
|
||||
#[violation]
|
||||
pub struct PytestIncorrectMarkParenthesesStyle {
|
||||
mark_name: String,
|
||||
|
|
|
@ -6,7 +6,7 @@ use std::fmt::Formatter;
|
|||
use crate::display_settings;
|
||||
use ruff_macros::CacheKey;
|
||||
|
||||
use crate::settings::types::IdentifierPattern;
|
||||
use crate::settings::types::{IdentifierPattern, PreviewMode};
|
||||
|
||||
use super::types;
|
||||
|
||||
|
@ -49,6 +49,20 @@ impl Default for Settings {
|
|||
}
|
||||
}
|
||||
|
||||
impl Settings {
|
||||
pub fn resolve_default(preview: PreviewMode) -> Self {
|
||||
if preview.is_enabled() {
|
||||
Self {
|
||||
fixture_parentheses: false,
|
||||
mark_parentheses: false,
|
||||
..Default::default()
|
||||
}
|
||||
} else {
|
||||
Self::default()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Display for Settings {
|
||||
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
|
||||
display_settings! {
|
||||
|
|
|
@ -23,7 +23,7 @@ use ruff_linter::line_width::{IndentWidth, LineLength};
|
|||
use ruff_linter::registry::RuleNamespace;
|
||||
use ruff_linter::registry::{Rule, RuleSet, INCOMPATIBLE_CODES};
|
||||
use ruff_linter::rule_selector::{PreviewOptions, Specificity};
|
||||
use ruff_linter::rules::pycodestyle;
|
||||
use ruff_linter::rules::{flake8_pytest_style, pycodestyle};
|
||||
use ruff_linter::settings::fix_safety_table::FixSafetyTable;
|
||||
use ruff_linter::settings::rule_table::RuleTable;
|
||||
use ruff_linter::settings::types::{
|
||||
|
@ -327,9 +327,13 @@ impl Configuration {
|
|||
.unwrap_or_default(),
|
||||
flake8_pytest_style: lint
|
||||
.flake8_pytest_style
|
||||
.map(Flake8PytestStyleOptions::try_into_settings)
|
||||
.map(|options| {
|
||||
Flake8PytestStyleOptions::try_into_settings(options, lint_preview)
|
||||
})
|
||||
.transpose()?
|
||||
.unwrap_or_default(),
|
||||
.unwrap_or_else(|| {
|
||||
flake8_pytest_style::settings::Settings::resolve_default(lint_preview)
|
||||
}),
|
||||
flake8_quotes: lint
|
||||
.flake8_quotes
|
||||
.map(Flake8QuotesOptions::into_settings)
|
||||
|
|
|
@ -24,7 +24,7 @@ use ruff_linter::rules::{
|
|||
pycodestyle, pydocstyle, pyflakes, pylint, pyupgrade,
|
||||
};
|
||||
use ruff_linter::settings::types::{
|
||||
IdentifierPattern, OutputFormat, PythonVersion, RequiredVersion,
|
||||
IdentifierPattern, OutputFormat, PreviewMode, PythonVersion, RequiredVersion,
|
||||
};
|
||||
use ruff_linter::{warn_user_once, RuleSelector};
|
||||
use ruff_macros::{CombineOptions, OptionsMetadata};
|
||||
|
@ -1374,6 +1374,9 @@ pub struct Flake8PytestStyleOptions {
|
|||
/// default), `@pytest.fixture()` is valid and `@pytest.fixture` is
|
||||
/// invalid. If set to `false`, `@pytest.fixture` is valid and
|
||||
/// `@pytest.fixture()` is invalid.
|
||||
///
|
||||
/// If [preview](https://docs.astral.sh/ruff/preview/) is enabled, defaults to
|
||||
/// `false`.
|
||||
#[option(
|
||||
default = "true",
|
||||
value_type = "bool",
|
||||
|
@ -1457,6 +1460,9 @@ pub struct Flake8PytestStyleOptions {
|
|||
/// default), `@pytest.mark.foo()` is valid and `@pytest.mark.foo` is
|
||||
/// invalid. If set to `false`, `@pytest.fixture` is valid and
|
||||
/// `@pytest.mark.foo()` is invalid.
|
||||
///
|
||||
/// If [preview](https://docs.astral.sh/ruff/preview/) is enabled, defaults to
|
||||
/// `false`.
|
||||
#[option(
|
||||
default = "true",
|
||||
value_type = "bool",
|
||||
|
@ -1466,9 +1472,12 @@ pub struct Flake8PytestStyleOptions {
|
|||
}
|
||||
|
||||
impl Flake8PytestStyleOptions {
|
||||
pub fn try_into_settings(self) -> anyhow::Result<flake8_pytest_style::settings::Settings> {
|
||||
pub fn try_into_settings(
|
||||
self,
|
||||
preview: PreviewMode,
|
||||
) -> anyhow::Result<flake8_pytest_style::settings::Settings> {
|
||||
Ok(flake8_pytest_style::settings::Settings {
|
||||
fixture_parentheses: self.fixture_parentheses.unwrap_or(true),
|
||||
fixture_parentheses: self.fixture_parentheses.unwrap_or(preview.is_disabled()),
|
||||
parametrize_names_type: self.parametrize_names_type.unwrap_or_default(),
|
||||
parametrize_values_type: self.parametrize_values_type.unwrap_or_default(),
|
||||
parametrize_values_row_type: self.parametrize_values_row_type.unwrap_or_default(),
|
||||
|
@ -1494,7 +1503,7 @@ impl Flake8PytestStyleOptions {
|
|||
.transpose()
|
||||
.map_err(SettingsError::InvalidRaisesExtendRequireMatchFor)?
|
||||
.unwrap_or_default(),
|
||||
mark_parentheses: self.mark_parentheses.unwrap_or(true),
|
||||
mark_parentheses: self.mark_parentheses.unwrap_or(preview.is_disabled()),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
4
ruff.schema.json
generated
4
ruff.schema.json
generated
|
@ -1093,14 +1093,14 @@
|
|||
"type": "object",
|
||||
"properties": {
|
||||
"fixture-parentheses": {
|
||||
"description": "Boolean flag specifying whether `@pytest.fixture()` without parameters should have parentheses. If the option is set to `true` (the default), `@pytest.fixture()` is valid and `@pytest.fixture` is invalid. If set to `false`, `@pytest.fixture` is valid and `@pytest.fixture()` is invalid.",
|
||||
"description": "Boolean flag specifying whether `@pytest.fixture()` without parameters should have parentheses. If the option is set to `true` (the default), `@pytest.fixture()` is valid and `@pytest.fixture` is invalid. If set to `false`, `@pytest.fixture` is valid and `@pytest.fixture()` is invalid.\n\nIf [preview](https://docs.astral.sh/ruff/preview/) is enabled, defaults to `false`.",
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"mark-parentheses": {
|
||||
"description": "Boolean flag specifying whether `@pytest.mark.foo()` without parameters should have parentheses. If the option is set to `true` (the default), `@pytest.mark.foo()` is valid and `@pytest.mark.foo` is invalid. If set to `false`, `@pytest.fixture` is valid and `@pytest.mark.foo()` is invalid.",
|
||||
"description": "Boolean flag specifying whether `@pytest.mark.foo()` without parameters should have parentheses. If the option is set to `true` (the default), `@pytest.mark.foo()` is valid and `@pytest.mark.foo` is invalid. If set to `false`, `@pytest.fixture` is valid and `@pytest.mark.foo()` is invalid.\n\nIf [preview](https://docs.astral.sh/ruff/preview/) is enabled, defaults to `false`.",
|
||||
"type": [
|
||||
"boolean",
|
||||
"null"
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue