Avoid raising PT012 for simple with statements (#6081)

This commit is contained in:
Harutaka Kawamura 2023-07-26 10:43:31 +09:00 committed by GitHub
parent 9dfe484472
commit 62f821daaa
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 57 additions and 81 deletions

View file

@ -11,6 +11,10 @@ async def test_ok_trivial_with():
with context_manager_under_test(): with context_manager_under_test():
pass pass
with pytest.raises(ValueError):
with context_manager_under_test():
raise ValueError
with pytest.raises(AttributeError): with pytest.raises(AttributeError):
async with context_manager_under_test(): async with context_manager_under_test():
pass pass
@ -47,13 +51,10 @@ async def test_error_complex_statement():
while True: while True:
[].size [].size
with pytest.raises(AttributeError):
with context_manager_under_test():
[].size
with pytest.raises(AttributeError): with pytest.raises(AttributeError):
async with context_manager_under_test(): async with context_manager_under_test():
[].size if True:
raise Exception
def test_error_try(): def test_error_try():

View file

@ -1,4 +1,4 @@
use ruff_python_ast::helpers::find_keyword; use ruff_python_ast::helpers::{find_keyword, is_compound_statement};
use rustpython_parser::ast::{self, Expr, Keyword, Ranged, Stmt, WithItem}; use rustpython_parser::ast::{self, Expr, Keyword, Ranged, Stmt, WithItem};
use ruff_diagnostics::{Diagnostic, Violation}; use ruff_diagnostics::{Diagnostic, Violation};
@ -84,12 +84,10 @@ fn is_pytest_raises(func: &Expr, semantic: &SemanticModel) -> bool {
} }
const fn is_non_trivial_with_body(body: &[Stmt]) -> bool { const fn is_non_trivial_with_body(body: &[Stmt]) -> bool {
if body.len() > 1 { if let [stmt] = body {
true is_compound_statement(stmt)
} else if let Some(first_body_stmt) = body.first() {
!first_body_stmt.is_pass_stmt()
} else { } else {
false true
} }
} }
@ -124,8 +122,6 @@ pub(crate) fn complex_raises(
items: &[WithItem], items: &[WithItem],
body: &[Stmt], body: &[Stmt],
) { ) {
let mut is_too_complex = false;
let raises_called = items.iter().any(|item| match &item.context_expr { let raises_called = items.iter().any(|item| match &item.context_expr {
Expr::Call(ast::ExprCall { func, .. }) => is_pytest_raises(func, checker.semantic()), Expr::Call(ast::ExprCall { func, .. }) => is_pytest_raises(func, checker.semantic()),
_ => false, _ => false,
@ -133,28 +129,17 @@ pub(crate) fn complex_raises(
// Check body for `pytest.raises` context manager // Check body for `pytest.raises` context manager
if raises_called { if raises_called {
if body.len() > 1 { let is_too_complex = if let [stmt] = body {
is_too_complex = true; match stmt {
} else if let Some(first_stmt) = body.first() {
match first_stmt {
Stmt::With(ast::StmtWith { body, .. }) Stmt::With(ast::StmtWith { body, .. })
| Stmt::AsyncWith(ast::StmtAsyncWith { body, .. }) => { | Stmt::AsyncWith(ast::StmtAsyncWith { body, .. }) => {
if is_non_trivial_with_body(body) { is_non_trivial_with_body(body)
is_too_complex = true;
}
} }
Stmt::If(_) stmt => is_compound_statement(stmt),
| Stmt::For(_)
| Stmt::Match(_)
| Stmt::AsyncFor(_)
| Stmt::While(_)
| Stmt::Try(_)
| Stmt::TryStar(_) => {
is_too_complex = true;
}
_ => {}
} }
} } else {
true
};
if is_too_complex { if is_too_complex {
checker.diagnostics.push(Diagnostic::new( checker.diagnostics.push(Diagnostic::new(

View file

@ -1,35 +1,22 @@
--- ---
source: crates/ruff/src/rules/flake8_pytest_style/mod.rs source: crates/ruff/src/rules/flake8_pytest_style/mod.rs
--- ---
PT012.py:28:5: PT012 `pytest.raises()` block should contain a single simple statement PT012.py:32:5: PT012 `pytest.raises()` block should contain a single simple statement
| |
27 | def test_error_multiple_statements(): 31 | def test_error_multiple_statements():
28 | with pytest.raises(AttributeError): 32 | with pytest.raises(AttributeError):
| _____^ | _____^
29 | | len([]) 33 | | len([])
30 | | [].size 34 | | [].size
| |_______________^ PT012 | |_______________^ PT012
| |
PT012.py:34:5: PT012 `pytest.raises()` block should contain a single simple statement
|
33 | async def test_error_complex_statement():
34 | with pytest.raises(AttributeError):
| _____^
35 | | if True:
36 | | [].size
| |___________________^ PT012
37 |
38 | with pytest.raises(AttributeError):
|
PT012.py:38:5: PT012 `pytest.raises()` block should contain a single simple statement PT012.py:38:5: PT012 `pytest.raises()` block should contain a single simple statement
| |
36 | [].size 37 | async def test_error_complex_statement():
37 |
38 | with pytest.raises(AttributeError): 38 | with pytest.raises(AttributeError):
| _____^ | _____^
39 | | for i in []: 39 | | if True:
40 | | [].size 40 | | [].size
| |___________________^ PT012 | |___________________^ PT012
41 | 41 |
@ -42,7 +29,7 @@ PT012.py:42:5: PT012 `pytest.raises()` block should contain a single simple stat
41 | 41 |
42 | with pytest.raises(AttributeError): 42 | with pytest.raises(AttributeError):
| _____^ | _____^
43 | | async for i in []: 43 | | for i in []:
44 | | [].size 44 | | [].size
| |___________________^ PT012 | |___________________^ PT012
45 | 45 |
@ -55,7 +42,7 @@ PT012.py:46:5: PT012 `pytest.raises()` block should contain a single simple stat
45 | 45 |
46 | with pytest.raises(AttributeError): 46 | with pytest.raises(AttributeError):
| _____^ | _____^
47 | | while True: 47 | | async for i in []:
48 | | [].size 48 | | [].size
| |___________________^ PT012 | |___________________^ PT012
49 | 49 |
@ -68,7 +55,7 @@ PT012.py:50:5: PT012 `pytest.raises()` block should contain a single simple stat
49 | 49 |
50 | with pytest.raises(AttributeError): 50 | with pytest.raises(AttributeError):
| _____^ | _____^
51 | | with context_manager_under_test(): 51 | | while True:
52 | | [].size 52 | | [].size
| |___________________^ PT012 | |___________________^ PT012
53 | 53 |
@ -82,19 +69,20 @@ PT012.py:54:5: PT012 `pytest.raises()` block should contain a single simple stat
54 | with pytest.raises(AttributeError): 54 | with pytest.raises(AttributeError):
| _____^ | _____^
55 | | async with context_manager_under_test(): 55 | | async with context_manager_under_test():
56 | | [].size 56 | | if True:
| |___________________^ PT012 57 | | raise Exception
| |_______________________________^ PT012
| |
PT012.py:60:5: PT012 `pytest.raises()` block should contain a single simple statement PT012.py:61:5: PT012 `pytest.raises()` block should contain a single simple statement
| |
59 | def test_error_try(): 60 | def test_error_try():
60 | with pytest.raises(AttributeError): 61 | with pytest.raises(AttributeError):
| _____^ | _____^
61 | | try: 62 | | try:
62 | | [].size 63 | | [].size
63 | | except: 64 | | except:
64 | | raise 65 | | raise
| |_________________^ PT012 | |_________________^ PT012
| |

View file

@ -18,6 +18,25 @@ use crate::call_path::CallPath;
use crate::source_code::{Indexer, Locator}; use crate::source_code::{Indexer, Locator};
use crate::statement_visitor::{walk_body, walk_stmt, StatementVisitor}; use crate::statement_visitor::{walk_body, walk_stmt, StatementVisitor};
/// Return `true` if the `Stmt` is a compound statement (as opposed to a simple statement).
pub const fn is_compound_statement(stmt: &Stmt) -> bool {
matches!(
stmt,
Stmt::FunctionDef(_)
| Stmt::AsyncFunctionDef(_)
| Stmt::ClassDef(_)
| Stmt::While(_)
| Stmt::For(_)
| Stmt::AsyncFor(_)
| Stmt::Match(_)
| Stmt::With(_)
| Stmt::AsyncWith(_)
| Stmt::If(_)
| Stmt::Try(_)
| Stmt::TryStar(_)
)
}
fn is_iterable_initializer<F>(id: &str, is_builtin: F) -> bool fn is_iterable_initializer<F>(id: &str, is_builtin: F) -> bool
where where
F: Fn(&str) -> bool, F: Fn(&str) -> bool,

View file

@ -3,6 +3,7 @@ use rustpython_parser::ast::{Ranged, Stmt, Suite};
use ruff_formatter::{ use ruff_formatter::{
format_args, write, FormatOwnedWithRule, FormatRefWithRule, FormatRuleWithOptions, format_args, write, FormatOwnedWithRule, FormatRefWithRule, FormatRuleWithOptions,
}; };
use ruff_python_ast::helpers::is_compound_statement;
use ruff_python_trivia::lines_before; use ruff_python_trivia::lines_before;
use crate::context::NodeLevel; use crate::context::NodeLevel;
@ -142,24 +143,6 @@ const fn is_class_or_function_definition(stmt: &Stmt) -> bool {
) )
} }
const fn is_compound_statement(stmt: &Stmt) -> bool {
matches!(
stmt,
Stmt::FunctionDef(_)
| Stmt::AsyncFunctionDef(_)
| Stmt::ClassDef(_)
| Stmt::While(_)
| Stmt::For(_)
| Stmt::AsyncFor(_)
| Stmt::Match(_)
| Stmt::With(_)
| Stmt::AsyncWith(_)
| Stmt::If(_)
| Stmt::Try(_)
| Stmt::TryStar(_)
)
}
impl FormatRuleWithOptions<Suite, PyFormatContext<'_>> for FormatSuite { impl FormatRuleWithOptions<Suite, PyFormatContext<'_>> for FormatSuite {
type Options = SuiteLevel; type Options = SuiteLevel;