[ruff 0.8] [flake8-pytest-style] Remove deprecated rules PT004 and …

…PT005 (#14385)

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

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),
-        (Flake8PytestStyle, "005") => (RuleGroup::Deprecated, rules::flake8_pytest_style::rules::PytestIncorrectFixtureNameUnderscore),
+        #[allow(deprecated)]
+        (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
-/// Marking fixtures that do not return a value with an underscore
+/// ## Removal
+/// 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 {
-    function: String,
-}
+#[deprecated(note = "PT004 has been removed")]
+pub struct PytestMissingFixtureNameUnderscore;
 
+#[allow(deprecated)]
 impl Violation for PytestMissingFixtureNameUnderscore {
-    #[derive_message_formats]
     fn message(&self) -> String {
-        let PytestMissingFixtureNameUnderscore { function } = self;
-        format!("Fixture `{function}` does not return anything, add leading underscore")
+        unreachable!("PT004 has been removed");
+    }
+
+    fn message_formats() -> &'static [&'static str] {
+        &["Fixture `{function}` does not return anything, add leading underscore"]
     }
 }
 
-/// ## Deprecation
-/// Marking fixtures that do not return a value with an underscore
+/// ## Removal
+/// 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 {
-    function: String,
-}
+#[deprecated(note = "PT005 has been removed")]
+pub struct PytestIncorrectFixtureNameUnderscore;
 
+#[allow(deprecated)]
 impl Violation for PytestIncorrectFixtureNameUnderscore {
-    #[derive_message_formats]
     fn message(&self) -> String {
-        let PytestIncorrectFixtureNameUnderscore { function } = self;
-        format!("Fixture `{function}` returns a value, remove leading underscore")
+        unreachable!("PT005 has been removed");
+    }
+
+    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
-fn check_fixture_returns(
-    checker: &mut Checker,
-    stmt: &Stmt,
-    name: &str,
-    body: &[Stmt],
-    returns: Option<&Expr>,
-) {
+/// PT022
+fn check_fixture_returns(checker: &mut Checker, 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)
-            || checker.enabled(Rule::PytestIncorrectFixtureNameUnderscore)
-            || checker.enabled(Rule::PytestUselessYieldFixture))
+        if 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_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;