From b9ed3e3876ea579eda9d86a8aed521274788af01 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 13 Mar 2025 08:42:49 +0100 Subject: [PATCH] [`flake8-pytest-style`] Allow for loops with empty bodies (`PT012`, `PT031`) (#16678) ## Summary This PR stabilizes the behavior change introduced in https://github.com/astral-sh/ruff/pull/15542 to allow for statements with an empty body in `pytest.raises` and `pytest.warns` with statements. This raised an error before but is now allowed: ```py with pytest.raises(KeyError, match='unknown'): async for _ in gpt.generate(gpt_request): pass ``` The same applies to ```py with pytest.raises(KeyError, match='unknown'): async for _ in gpt.generate(gpt_request): ... ``` There have been now new issues or PRs related to PT012 or PT031 since this behavior change was introduced in ruff 0.9.3 (January 23rd). --- .../src/rules/flake8_pytest_style/mod.rs | 32 +---- .../rules/flake8_pytest_style/rules/raises.rs | 14 +- .../rules/flake8_pytest_style/rules/warns.rs | 13 +- ...es__flake8_pytest_style__tests__PT012.snap | 46 ------- ...e8_pytest_style__tests__PT012_preview.snap | 126 ------------------ ...es__flake8_pytest_style__tests__PT031.snap | 46 ------- ...e8_pytest_style__tests__PT031_preview.snap | 126 ------------------ 7 files changed, 11 insertions(+), 392 deletions(-) delete mode 100644 crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT012_preview.snap delete mode 100644 crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT031_preview.snap 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 dca6e3ced4..7cb3486d01 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs @@ -11,7 +11,7 @@ mod tests { use test_case::test_case; use crate::registry::Rule; - use crate::settings::types::{IdentifierPattern, PreviewMode}; + use crate::settings::types::IdentifierPattern; use crate::test::test_path; use crate::{assert_messages, settings}; @@ -358,36 +358,6 @@ mod tests { Ok(()) } - #[test_case( - Rule::PytestRaisesWithMultipleStatements, - Path::new("PT012.py"), - Settings::default(), - "PT012_preview" - )] - #[test_case( - Rule::PytestWarnsWithMultipleStatements, - Path::new("PT031.py"), - Settings::default(), - "PT031_preview" - )] - fn test_pytest_style_preview( - rule_code: Rule, - path: &Path, - plugin_settings: Settings, - name: &str, - ) -> Result<()> { - let diagnostics = test_path( - Path::new("flake8_pytest_style").join(path).as_path(), - &settings::LinterSettings { - preview: PreviewMode::Enabled, - flake8_pytest_style: plugin_settings, - ..settings::LinterSettings::for_rule(rule_code) - }, - )?; - assert_messages!(name, diagnostics); - Ok(()) - } - /// This test ensure that PT006 and PT007 don't conflict when both of them suggest a fix that /// edits `argvalues` for `pytest.mark.parametrize`. #[test] diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/raises.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/raises.rs index 4701b9e2fb..08428d3284 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/raises.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/raises.rs @@ -13,6 +13,10 @@ use super::helpers::is_empty_or_null_string; /// ## What it does /// Checks for `pytest.raises` context managers with multiple statements. /// +/// This rule allows `pytest.raises` bodies to contain `for` +/// loops with empty bodies (e.g., `pass` or `...` statements), to test +/// iterator behavior. +/// /// ## Why is this bad? /// When a `pytest.raises` is used as a context manager and contains multiple /// statements, it can lead to the test passing when it actually should fail. @@ -20,10 +24,6 @@ use super::helpers::is_empty_or_null_string; /// A `pytest.raises` context manager should only contain a single simple /// statement that raises the expected exception. /// -/// In [preview], this rule allows `pytest.raises` bodies to contain `for` -/// loops with empty bodies (e.g., `pass` or `...` statements), to test -/// iterator behavior. -/// /// ## Example /// ```python /// import pytest @@ -50,8 +50,6 @@ use super::helpers::is_empty_or_null_string; /// /// ## References /// - [`pytest` documentation: `pytest.raises`](https://docs.pytest.org/en/latest/reference/reference.html#pytest-raises) -/// -/// [preview]: https://docs.astral.sh/ruff/preview/ #[derive(ViolationMetadata)] pub(crate) struct PytestRaisesWithMultipleStatements; @@ -206,14 +204,12 @@ pub(crate) fn complex_raises(checker: &Checker, stmt: &Stmt, items: &[WithItem], // Check body for `pytest.raises` context manager if raises_called { let is_too_complex = if let [stmt] = body { - let in_preview = checker.settings.preview.is_enabled(); - match stmt { Stmt::With(ast::StmtWith { body, .. }) => is_non_trivial_with_body(body), // Allow function and class definitions to test decorators. Stmt::ClassDef(_) | Stmt::FunctionDef(_) => false, // Allow empty `for` loops to test iterators. - Stmt::For(ast::StmtFor { body, .. }) if in_preview => match &body[..] { + Stmt::For(ast::StmtFor { body, .. }) => match &body[..] { [Stmt::Pass(_)] => false, [Stmt::Expr(ast::StmtExpr { value, .. })] => !value.is_ellipsis_literal_expr(), _ => true, diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/warns.rs b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/warns.rs index 3b741f3af6..32594a1c24 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/rules/warns.rs +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/rules/warns.rs @@ -13,6 +13,10 @@ use super::helpers::is_empty_or_null_string; /// ## What it does /// Checks for `pytest.warns` context managers with multiple statements. /// +/// This rule allows `pytest.warns` bodies to contain `for` +/// loops with empty bodies (e.g., `pass` or `...` statements), to test +/// iterator behavior. +/// /// ## Why is this bad? /// When `pytest.warns` is used as a context manager and contains multiple /// statements, it can lead to the test passing when it should instead fail. @@ -20,9 +24,6 @@ use super::helpers::is_empty_or_null_string; /// A `pytest.warns` context manager should only contain a single /// simple statement that triggers the expected warning. /// -/// In [preview], this rule allows `pytest.warns` bodies to contain `for` -/// loops with empty bodies (e.g., `pass` or `...` statements), to test -/// iterator behavior. /// /// ## Example /// ```python @@ -48,8 +49,6 @@ use super::helpers::is_empty_or_null_string; /// /// ## References /// - [`pytest` documentation: `pytest.warns`](https://docs.pytest.org/en/latest/reference/reference.html#pytest-warns) -/// -/// [preview]: https://docs.astral.sh/ruff/preview/ #[derive(ViolationMetadata)] pub(crate) struct PytestWarnsWithMultipleStatements; @@ -206,14 +205,12 @@ pub(crate) fn complex_warns(checker: &Checker, stmt: &Stmt, items: &[WithItem], // Check body for `pytest.warns` context manager if warns_called { let is_too_complex = if let [stmt] = body { - let in_preview = checker.settings.preview.is_enabled(); - match stmt { Stmt::With(ast::StmtWith { body, .. }) => is_non_trivial_with_body(body), // Allow function and class definitions to test decorators. Stmt::ClassDef(_) | Stmt::FunctionDef(_) => false, // Allow empty `for` loops to test iterators. - Stmt::For(ast::StmtFor { body, .. }) if in_preview => match &body[..] { + Stmt::For(ast::StmtFor { body, .. }) => match &body[..] { [Stmt::Pass(_)] => false, [Stmt::Expr(ast::StmtExpr { value, .. })] => !value.is_ellipsis_literal_expr(), _ => true, diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT012.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT012.snap index 3bbaf9924f..43cf518d00 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT012.snap +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT012.snap @@ -124,49 +124,3 @@ PT012.py:95:5: PT012 `pytest.raises()` block should contain a single simple stat 97 | | assert foo | |______________________^ PT012 | - -PT012.py:102:5: PT012 `pytest.raises()` block should contain a single simple statement - | -100 | ## No errors in preview -101 | -102 | / with pytest.raises(RuntimeError): -103 | | for a in b: -104 | | pass - | |________________^ PT012 -105 | -106 | with pytest.raises(RuntimeError): - | - -PT012.py:106:5: PT012 `pytest.raises()` block should contain a single simple statement - | -104 | pass -105 | -106 | / with pytest.raises(RuntimeError): -107 | | for a in b: -108 | | ... - | |_______________^ PT012 -109 | -110 | with pytest.raises(RuntimeError): - | - -PT012.py:110:5: PT012 `pytest.raises()` block should contain a single simple statement - | -108 | ... -109 | -110 | / with pytest.raises(RuntimeError): -111 | | async for a in b: -112 | | pass - | |________________^ PT012 -113 | -114 | with pytest.raises(RuntimeError): - | - -PT012.py:114:5: PT012 `pytest.raises()` block should contain a single simple statement - | -112 | pass -113 | -114 | / with pytest.raises(RuntimeError): -115 | | async for a in b: -116 | | ... - | |_______________^ PT012 - | diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT012_preview.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT012_preview.snap deleted file mode 100644 index 43cf518d00..0000000000 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT012_preview.snap +++ /dev/null @@ -1,126 +0,0 @@ ---- -source: crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs ---- -PT012.py:42:5: PT012 `pytest.raises()` block should contain a single simple statement - | -41 | def test_error_multiple_statements(): -42 | / with pytest.raises(AttributeError): -43 | | len([]) -44 | | [].size - | |_______________^ PT012 - | - -PT012.py:48:5: PT012 `pytest.raises()` block should contain a single simple statement - | -47 | async def test_error_complex_statement(): -48 | / with pytest.raises(AttributeError): -49 | | if True: -50 | | [].size - | |___________________^ PT012 -51 | -52 | with pytest.raises(AttributeError): - | - -PT012.py:52:5: PT012 `pytest.raises()` block should contain a single simple statement - | -50 | [].size -51 | -52 | / with pytest.raises(AttributeError): -53 | | for i in []: -54 | | [].size - | |___________________^ PT012 -55 | -56 | with pytest.raises(AttributeError): - | - -PT012.py:56:5: PT012 `pytest.raises()` block should contain a single simple statement - | -54 | [].size -55 | -56 | / with pytest.raises(AttributeError): -57 | | async for i in []: -58 | | [].size - | |___________________^ PT012 -59 | -60 | with pytest.raises(AttributeError): - | - -PT012.py:60:5: PT012 `pytest.raises()` block should contain a single simple statement - | -58 | [].size -59 | -60 | / with pytest.raises(AttributeError): -61 | | while True: -62 | | [].size - | |___________________^ PT012 -63 | -64 | with pytest.raises(AttributeError): - | - -PT012.py:64:5: PT012 `pytest.raises()` block should contain a single simple statement - | -62 | [].size -63 | -64 | / with pytest.raises(AttributeError): -65 | | async with context_manager_under_test(): -66 | | if True: -67 | | raise Exception - | |_______________________________^ PT012 - | - -PT012.py:71:5: PT012 `pytest.raises()` block should contain a single simple statement - | -70 | def test_error_try(): -71 | / with pytest.raises(AttributeError): -72 | | try: -73 | | [].size -74 | | except: -75 | | raise - | |_________________^ PT012 - | - -PT012.py:83:5: PT012 `pytest.raises()` block should contain a single simple statement - | -81 | ## Errors -82 | -83 | / with pytest.raises(RuntimeError): -84 | | for a in b: -85 | | print() - | |___________________^ PT012 -86 | -87 | with pytest.raises(RuntimeError): - | - -PT012.py:87:5: PT012 `pytest.raises()` block should contain a single simple statement - | -85 | print() -86 | -87 | / with pytest.raises(RuntimeError): -88 | | for a in b: -89 | | assert foo - | |______________________^ PT012 -90 | -91 | with pytest.raises(RuntimeError): - | - -PT012.py:91:5: PT012 `pytest.raises()` block should contain a single simple statement - | -89 | assert foo -90 | -91 | / with pytest.raises(RuntimeError): -92 | | async for a in b: -93 | | print() - | |___________________^ PT012 -94 | -95 | with pytest.raises(RuntimeError): - | - -PT012.py:95:5: PT012 `pytest.raises()` block should contain a single simple statement - | -93 | print() -94 | -95 | / with pytest.raises(RuntimeError): -96 | | async for a in b: -97 | | assert foo - | |______________________^ PT012 - | diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT031.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT031.snap index 1df272526e..ebf9e8b2fc 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT031.snap +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT031.snap @@ -124,49 +124,3 @@ PT031.py:95:5: PT031 `pytest.warns()` block should contain a single simple state 97 | | assert foo | |______________________^ PT031 | - -PT031.py:102:5: PT031 `pytest.warns()` block should contain a single simple statement - | -100 | ## No errors in preview -101 | -102 | / with pytest.warns(RuntimeError): -103 | | for a in b: -104 | | pass - | |________________^ PT031 -105 | -106 | with pytest.warns(RuntimeError): - | - -PT031.py:106:5: PT031 `pytest.warns()` block should contain a single simple statement - | -104 | pass -105 | -106 | / with pytest.warns(RuntimeError): -107 | | for a in b: -108 | | ... - | |_______________^ PT031 -109 | -110 | with pytest.warns(RuntimeError): - | - -PT031.py:110:5: PT031 `pytest.warns()` block should contain a single simple statement - | -108 | ... -109 | -110 | / with pytest.warns(RuntimeError): -111 | | async for a in b: -112 | | pass - | |________________^ PT031 -113 | -114 | with pytest.warns(RuntimeError): - | - -PT031.py:114:5: PT031 `pytest.warns()` block should contain a single simple statement - | -112 | pass -113 | -114 | / with pytest.warns(RuntimeError): -115 | | async for a in b: -116 | | ... - | |_______________^ PT031 - | diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT031_preview.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT031_preview.snap deleted file mode 100644 index ebf9e8b2fc..0000000000 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT031_preview.snap +++ /dev/null @@ -1,126 +0,0 @@ ---- -source: crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs ---- -PT031.py:42:5: PT031 `pytest.warns()` block should contain a single simple statement - | -41 | def test_error_multiple_statements(): -42 | / with pytest.warns(UserWarning): -43 | | foo() -44 | | bar() - | |_____________^ PT031 - | - -PT031.py:48:5: PT031 `pytest.warns()` block should contain a single simple statement - | -47 | async def test_error_complex_statement(): -48 | / with pytest.warns(UserWarning): -49 | | if True: -50 | | foo() - | |_________________^ PT031 -51 | -52 | with pytest.warns(UserWarning): - | - -PT031.py:52:5: PT031 `pytest.warns()` block should contain a single simple statement - | -50 | foo() -51 | -52 | / with pytest.warns(UserWarning): -53 | | for i in []: -54 | | foo() - | |_________________^ PT031 -55 | -56 | with pytest.warns(UserWarning): - | - -PT031.py:56:5: PT031 `pytest.warns()` block should contain a single simple statement - | -54 | foo() -55 | -56 | / with pytest.warns(UserWarning): -57 | | async for i in []: -58 | | foo() - | |_________________^ PT031 -59 | -60 | with pytest.warns(UserWarning): - | - -PT031.py:60:5: PT031 `pytest.warns()` block should contain a single simple statement - | -58 | foo() -59 | -60 | / with pytest.warns(UserWarning): -61 | | while True: -62 | | foo() - | |_________________^ PT031 -63 | -64 | with pytest.warns(UserWarning): - | - -PT031.py:64:5: PT031 `pytest.warns()` block should contain a single simple statement - | -62 | foo() -63 | -64 | / with pytest.warns(UserWarning): -65 | | async with context_manager_under_test(): -66 | | if True: -67 | | foo() - | |_____________________^ PT031 - | - -PT031.py:71:5: PT031 `pytest.warns()` block should contain a single simple statement - | -70 | def test_error_try(): -71 | / with pytest.warns(UserWarning): -72 | | try: -73 | | foo() -74 | | except: -75 | | raise - | |_________________^ PT031 - | - -PT031.py:83:5: PT031 `pytest.warns()` block should contain a single simple statement - | -81 | ## Errors -82 | -83 | / with pytest.warns(RuntimeError): -84 | | for a in b: -85 | | print() - | |___________________^ PT031 -86 | -87 | with pytest.warns(RuntimeError): - | - -PT031.py:87:5: PT031 `pytest.warns()` block should contain a single simple statement - | -85 | print() -86 | -87 | / with pytest.warns(RuntimeError): -88 | | for a in b: -89 | | assert foo - | |______________________^ PT031 -90 | -91 | with pytest.warns(RuntimeError): - | - -PT031.py:91:5: PT031 `pytest.warns()` block should contain a single simple statement - | -89 | assert foo -90 | -91 | / with pytest.warns(RuntimeError): -92 | | async for a in b: -93 | | print() - | |___________________^ PT031 -94 | -95 | with pytest.warns(RuntimeError): - | - -PT031.py:95:5: PT031 `pytest.warns()` block should contain a single simple statement - | -93 | print() -94 | -95 | / with pytest.warns(RuntimeError): -96 | | async for a in b: -97 | | assert foo - | |______________________^ PT031 - |