From 0e128b9884c71658093cf3389a46b79bc4f0e219 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 19 Nov 2024 09:01:33 +0000 Subject: [PATCH] [ruff 0.8] [`flake8-pytest-style`] Remove deprecated rules PT004 and PT005 (#14385) Co-authored-by: Micha Reiser --- .../fixtures/flake8_pytest_style/PT004.py | 58 -------------- .../fixtures/flake8_pytest_style/PT005.py | 57 -------------- .../src/checkers/ast/analyze/statement.rs | 3 - crates/ruff_linter/src/codes.rs | 6 +- .../src/rules/flake8_pytest_style/mod.rs | 12 --- .../flake8_pytest_style/rules/fixture.rs | 77 ++++++------------- ...es__flake8_pytest_style__tests__PT004.snap | 20 ----- ...es__flake8_pytest_style__tests__PT005.snap | 29 ------- crates/ruff_macros/src/violation.rs | 1 + ruff.schema.json | 2 - 10 files changed, 29 insertions(+), 236 deletions(-) delete mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT004.py delete mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT005.py delete mode 100644 crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT004.snap delete mode 100644 crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT005.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT004.py b/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT004.py deleted file mode 100644 index 0904267775383..0000000000000 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT004.py +++ /dev/null @@ -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 diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT005.py b/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT005.py deleted file mode 100644 index bc19fa7d8ee9f..0000000000000 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pytest_style/PT005.py +++ /dev/null @@ -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() diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index ba923acb1985a..81374e8fe544a 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/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(), diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 4c14005cf828c..6972b45a4e33a 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/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), diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs index d6dae62dba6ed..db190ccaad1ca 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs +++ b/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"), diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/fixture.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/fixture.rs index 842d389f3d1b3..6562d4d032887 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/fixture.rs +++ b/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) { diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT004.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT004.snap deleted file mode 100644 index c20fe525af082..0000000000000 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT004.snap +++ /dev/null @@ -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 - | diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT005.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT005.snap deleted file mode 100644 index f9ffa661480b6..0000000000000 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT005.snap +++ /dev/null @@ -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: - | diff --git a/crates/ruff_macros/src/violation.rs b/crates/ruff_macros/src/violation.rs index 93a9efa5bd7be..281d2cfcb9217 100644 --- a/crates/ruff_macros/src/violation.rs +++ b/crates/ruff_macros/src/violation.rs @@ -55,6 +55,7 @@ pub(crate) fn violation(violation: &ItemStruct) -> Result { #[allow(deprecated)] #[automatically_derived] + #[allow(deprecated)] impl From<#ident> for ruff_diagnostics::DiagnosticKind { fn from(value: #ident) -> Self { use ruff_diagnostics::Violation; diff --git a/ruff.schema.json b/ruff.schema.json index c000dbdb77791..2acad6e5fef5d 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3641,8 +3641,6 @@ "PT001", "PT002", "PT003", - "PT004", - "PT005", "PT006", "PT007", "PT008",