[ruff 0.8] [flake8-pytest-style] Remove deprecated rules PT004 and PT005 (#14385)

Co-authored-by: Micha Reiser <micha@reiser.io>

crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT004.py

@@ -1,58 +0,0 @@
-import abc
-from abc import abstractmethod
-
-import pytest
-
-
-@pytest.fixture()
-def _patch_something(mocker):  # OK simple
-    mocker.patch("some.thing")
-
-
-@pytest.fixture()
-def _patch_something(mocker):  # OK with return
-    if something:
-        return
-    mocker.patch("some.thing")
-
-
-@pytest.fixture()
-def _activate_context():  # OK with yield
-    with context:
-        yield
-
-
-class BaseTest:
-    @pytest.fixture()
-    @abc.abstractmethod
-    def my_fixture():  # OK abstract with import abc
-        raise NotImplementedError
-
-
-class BaseTest:
-    @pytest.fixture()
-    @abstractmethod
-    def my_fixture():  # OK abstract with from import
-        raise NotImplementedError
-
-
-@pytest.fixture()
-def my_fixture():  # OK ignoring yield from
-    yield from some_generator()
-
-
-
-@pytest.fixture()
-def my_fixture():  # OK ignoring yield value
-    yield 1
-
-
-@pytest.fixture()
-def patch_something(mocker):  # Error simple
-    mocker.patch("some.thing")
-
-
-@pytest.fixture()
-def activate_context():  # Error with yield
-    with context:
-        yield

crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT005.py

@@ -1,57 +0,0 @@
-import abc
-from abc import abstractmethod
-
-import pytest
-
-
-@pytest.fixture()
-def my_fixture(mocker):  # OK with return
-    return 0
-
-
-@pytest.fixture()
-def activate_context():  # OK with yield
-    with get_context() as context:
-        yield context
-
-
-@pytest.fixture()
-def _any_fixture(mocker):  # Ok nested function
-    def nested_function():
-        return 1
-
-    mocker.patch("...", nested_function)
-
-
-class BaseTest:
-    @pytest.fixture()
-    @abc.abstractmethod
-    def _my_fixture():  # OK abstract with import abc
-        return NotImplemented
-
-
-class BaseTest:
-    @pytest.fixture()
-    @abstractmethod
-    def _my_fixture():  # OK abstract with from import
-        return NotImplemented
-
-
-@pytest.fixture()
-def _my_fixture(mocker):  # Error with return
-    return 0
-
-
-@pytest.fixture()
-def _activate_context():  # Error with yield
-    with get_context() as context:
-        yield context
-
-
-@pytest.fixture()
-def _activate_context():  # Error with conditional yield from
-    if some_condition:
-        with get_context() as context:
-            yield context
-    else:
-        yield from other_context()

crates/ruff_linter/src/checkers/ast/analyze/statement.rs

@@ -293,8 +293,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
                 Rule::PytestFixtureIncorrectParenthesesStyle,
                 Rule::PytestFixturePositionalArgs,
                 Rule::PytestExtraneousScopeFunction,
-                Rule::PytestMissingFixtureNameUnderscore,
-                Rule::PytestIncorrectFixtureNameUnderscore,
                 Rule::PytestFixtureParamWithoutValue,
                 Rule::PytestDeprecatedYieldFixture,
                 Rule::PytestFixtureFinalizerCallback,
@@ -304,7 +302,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
             ]) {
                 flake8_pytest_style::rules::fixture(
                     checker,
-                    stmt,
                     name,
                     parameters,
                     returns.as_deref(),

crates/ruff_linter/src/codes.rs

@@ -798,8 +798,10 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Flake8PytestStyle, "001") => (RuleGroup::Stable, rules::flake8_pytest_style::rules::PytestFixtureIncorrectParenthesesStyle),
         (Flake8PytestStyle, "002") => (RuleGroup::Stable, rules::flake8_pytest_style::rules::PytestFixturePositionalArgs),
         (Flake8PytestStyle, "003") => (RuleGroup::Stable, rules::flake8_pytest_style::rules::PytestExtraneousScopeFunction),
-        (Flake8PytestStyle, "004") => (RuleGroup::Deprecated, rules::flake8_pytest_style::rules::PytestMissingFixtureNameUnderscore),
+        #[allow(deprecated)]
-        (Flake8PytestStyle, "005") => (RuleGroup::Deprecated, rules::flake8_pytest_style::rules::PytestIncorrectFixtureNameUnderscore),
+        (Flake8PytestStyle, "004") => (RuleGroup::Removed, rules::flake8_pytest_style::rules::PytestMissingFixtureNameUnderscore),
+        #[allow(deprecated)]
+        (Flake8PytestStyle, "005") => (RuleGroup::Removed, rules::flake8_pytest_style::rules::PytestIncorrectFixtureNameUnderscore),
         (Flake8PytestStyle, "006") => (RuleGroup::Stable, rules::flake8_pytest_style::rules::PytestParametrizeNamesWrongType),
         (Flake8PytestStyle, "007") => (RuleGroup::Stable, rules::flake8_pytest_style::rules::PytestParametrizeValuesWrongType),
         (Flake8PytestStyle, "008") => (RuleGroup::Stable, rules::flake8_pytest_style::rules::PytestPatchWithLambda),

crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs

@@ -45,18 +45,6 @@ mod tests {
         Settings::default(),
         "PT003"
     )]
-    #[test_case(
-        Rule::PytestMissingFixtureNameUnderscore,
-        Path::new("PT004.py"),
-        Settings::default(),
-        "PT004"
-    )]
-    #[test_case(
-        Rule::PytestIncorrectFixtureNameUnderscore,
-        Path::new("PT005.py"),
-        Settings::default(),
-        "PT005"
-    )]
     #[test_case(
         Rule::PytestParametrizeNamesWrongType,
         Path::new("PT006.py"),

crates/ruff_linter/src/rules/flake8_pytest_style/rules/fixture.rs

@@ -1,7 +1,6 @@
 use ruff_diagnostics::{AlwaysFixableViolation, Violation};
 use ruff_diagnostics::{Diagnostic, Edit, Fix};
 use ruff_macros::{derive_message_formats, violation};
-use ruff_python_ast::identifier::Identifier;
 use ruff_python_ast::name::UnqualifiedName;
 use ruff_python_ast::visitor;
 use ruff_python_ast::visitor::Visitor;
@@ -167,8 +166,8 @@ impl AlwaysFixableViolation for PytestExtraneousScopeFunction {
     }
 }
 
-/// ## Deprecation
+/// ## Removal
-/// Marking fixtures that do not return a value with an underscore
+/// This rule has been removed because marking fixtures that do not return a value with an underscore
 /// isn't a practice recommended by the pytest community.
 ///
 /// ## What it does
@@ -216,20 +215,22 @@ impl AlwaysFixableViolation for PytestExtraneousScopeFunction {
 /// ## References
 /// - [`pytest` documentation: `@pytest.fixture` functions](https://docs.pytest.org/en/latest/reference/reference.html#pytest-fixture)
 #[violation]
-pub struct PytestMissingFixtureNameUnderscore {
+#[deprecated(note = "PT004 has been removed")]
-    function: String,
+pub struct PytestMissingFixtureNameUnderscore;
-}
 
+#[allow(deprecated)]
 impl Violation for PytestMissingFixtureNameUnderscore {
-    #[derive_message_formats]
     fn message(&self) -> String {
-        let PytestMissingFixtureNameUnderscore { function } = self;
+        unreachable!("PT004 has been removed");
-        format!("Fixture `{function}` does not return anything, add leading underscore")
+    }
+
+    fn message_formats() -> &'static [&'static str] {
+        &["Fixture `{function}` does not return anything, add leading underscore"]
     }
 }
 
-/// ## Deprecation
+/// ## Removal
-/// Marking fixtures that do not return a value with an underscore
+/// This rule has been removed because marking fixtures that do not return a value with an underscore
 /// isn't a practice recommended by the pytest community.
 ///
 /// ## What it does
@@ -279,15 +280,17 @@ impl Violation for PytestMissingFixtureNameUnderscore {
 /// ## References
 /// - [`pytest` documentation: `@pytest.fixture` functions](https://docs.pytest.org/en/latest/reference/reference.html#pytest-fixture)
 #[violation]
-pub struct PytestIncorrectFixtureNameUnderscore {
+#[deprecated(note = "PT005 has been removed")]
-    function: String,
+pub struct PytestIncorrectFixtureNameUnderscore;
-}
 
+#[allow(deprecated)]
 impl Violation for PytestIncorrectFixtureNameUnderscore {
-    #[derive_message_formats]
     fn message(&self) -> String {
-        let PytestIncorrectFixtureNameUnderscore { function } = self;
+        unreachable!("PT005 has been removed");
-        format!("Fixture `{function}` returns a value, remove leading underscore")
+    }
+
+    fn message_formats() -> &'static [&'static str] {
+        &["Fixture `{function}` returns a value, remove leading underscore"]
     }
 }
 
@@ -749,43 +752,14 @@ fn check_fixture_decorator(checker: &mut Checker, func_name: &str, decorator: &D
     }
 }
 
-/// PT004, PT005, PT022
+/// PT022
-fn check_fixture_returns(
+fn check_fixture_returns(checker: &mut Checker, name: &str, body: &[Stmt], returns: Option<&Expr>) {
-    checker: &mut Checker,
-    stmt: &Stmt,
-    name: &str,
-    body: &[Stmt],
-    returns: Option<&Expr>,
-) {
     let mut visitor = SkipFunctionsVisitor::default();
 
     for stmt in body {
         visitor.visit_stmt(stmt);
     }
 
-    if checker.enabled(Rule::PytestIncorrectFixtureNameUnderscore)
-        && visitor.has_return_with_value
-        && name.starts_with('_')
-    {
-        checker.diagnostics.push(Diagnostic::new(
-            PytestIncorrectFixtureNameUnderscore {
-                function: name.to_string(),
-            },
-            stmt.identifier(),
-        ));
-    } else if checker.enabled(Rule::PytestMissingFixtureNameUnderscore)
-        && !visitor.has_return_with_value
-        && !visitor.has_yield_from
-        && !name.starts_with('_')
-    {
-        checker.diagnostics.push(Diagnostic::new(
-            PytestMissingFixtureNameUnderscore {
-                function: name.to_string(),
-            },
-            stmt.identifier(),
-        ));
-    }
-
     if checker.enabled(Rule::PytestUselessYieldFixture) {
         let Some(stmt) = body.last() else {
             return;
@@ -904,7 +878,6 @@ fn check_fixture_marks(checker: &mut Checker, decorators: &[Decorator]) {
 
 pub(crate) fn fixture(
     checker: &mut Checker,
-    stmt: &Stmt,
     name: &str,
     parameters: &Parameters,
     returns: Option<&Expr>,
@@ -924,12 +897,10 @@ pub(crate) fn fixture(
             check_fixture_decorator_name(checker, decorator);
         }
 
-        if (checker.enabled(Rule::PytestMissingFixtureNameUnderscore)
+        if checker.enabled(Rule::PytestUselessYieldFixture)
-            || checker.enabled(Rule::PytestIncorrectFixtureNameUnderscore)
-            || checker.enabled(Rule::PytestUselessYieldFixture))
             && !is_abstract(decorators, checker.semantic())
         {
-            check_fixture_returns(checker, stmt, name, body, returns);
+            check_fixture_returns(checker, name, body, returns);
         }
 
         if checker.enabled(Rule::PytestFixtureFinalizerCallback) {

crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT004.snap

@@ -1,20 +0,0 @@
----
-source: crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs
-snapshot_kind: text
----
-PT004.py:51:5: PT004 Fixture `patch_something` does not return anything, add leading underscore
-   |
-50 | @pytest.fixture()
-51 | def patch_something(mocker):  # Error simple
-   |     ^^^^^^^^^^^^^^^ PT004
-52 |     mocker.patch("some.thing")
-   |
-
-PT004.py:56:5: PT004 Fixture `activate_context` does not return anything, add leading underscore
-   |
-55 | @pytest.fixture()
-56 | def activate_context():  # Error with yield
-   |     ^^^^^^^^^^^^^^^^ PT004
-57 |     with context:
-58 |         yield
-   |

crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT005.snap

@@ -1,29 +0,0 @@
----
-source: crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs
-snapshot_kind: text
----
-PT005.py:41:5: PT005 Fixture `_my_fixture` returns a value, remove leading underscore
-   |
-40 | @pytest.fixture()
-41 | def _my_fixture(mocker):  # Error with return
-   |     ^^^^^^^^^^^ PT005
-42 |     return 0
-   |
-
-PT005.py:46:5: PT005 Fixture `_activate_context` returns a value, remove leading underscore
-   |
-45 | @pytest.fixture()
-46 | def _activate_context():  # Error with yield
-   |     ^^^^^^^^^^^^^^^^^ PT005
-47 |     with get_context() as context:
-48 |         yield context
-   |
-
-PT005.py:52:5: PT005 Fixture `_activate_context` returns a value, remove leading underscore
-   |
-51 | @pytest.fixture()
-52 | def _activate_context():  # Error with conditional yield from
-   |     ^^^^^^^^^^^^^^^^^ PT005
-53 |     if some_condition:
-54 |         with get_context() as context:
-   |

crates/ruff_macros/src/violation.rs

@@ -55,6 +55,7 @@ pub(crate) fn violation(violation: &ItemStruct) -> Result<TokenStream> {
 
             #[allow(deprecated)]
             #[automatically_derived]
+            #[allow(deprecated)]
             impl From<#ident> for ruff_diagnostics::DiagnosticKind {
                 fn from(value: #ident) -> Self {
                     use ruff_diagnostics::Violation;

ruff.schema.json

@@ -3641,8 +3641,6 @@
         "PT001",
         "PT002",
         "PT003",
-        "PT004",
-        "PT005",
         "PT006",
         "PT007",
         "PT008",