mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-01 06:11:21 +00:00
[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).
This commit is contained in:
Micha Reiser
parent
e740286bbd
commit
b9ed3e3876
7 changed files with 11 additions and 392 deletions
|
|
@ -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]
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
||||
|
|
@ -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
|
||||
|
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
||||
Loading…
Add table
Add a link
Reference in a new issue