mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-04 02:38:25 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Support parenthesized expressions when splitting compound assertions (#5219)

Browse source 
## Summary

I'm looking into the Black stability tests, and here's one failing case.

We split `assert a and (b and c)` into:

```python
assert a
assert (b and c)
```

We fail to split `assert (b and c)` due to the parentheses. But Black
then removes then, and when running Ruff again, we get:

```python
assert a
assert b
assert c
```

This PR just enables us to fix to this in one pass.
This commit is contained in:
Charlie Marsh 2023-06-20 13:47:01 -04:00 • committed by GitHub
parent 4547002eb7
commit 30734f06fd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 187 additions and 131 deletions
Download patch file Download diff file Expand all files Collapse all files

15
crates/ruff/resources/test/fixtures/flake8_pytest_style/PT018.py vendored
View file

@ -21,6 +21,13 @@ def test_error():
assert something and something_else == """error
message
"""
assert (
something
and something_else
== """error
message
"""
)
# recursive case
assert not (a or not (b or c))
@ -31,14 +38,6 @@ def test_error():
assert not (something or something_else and something_third), "with message"
# detected, but no autofix for mixed conditions (e.g. `a or b and c`)
assert not (something or something_else and something_third)
# detected, but no autofix for parenthesized conditions
assert (
something
and something_else
== """error
message
"""
)
assert something # OK

62
crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs
View file

@ -9,7 +9,6 @@ use libcst_native::{
};
use rustpython_parser::ast::{self, BoolOp, ExceptHandler, Expr, Keyword, Ranged, Stmt, UnaryOp};
use crate::autofix::codemods::CodegenStylist;
use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::{has_comments_in, Truthiness};
@ -17,6 +16,7 @@ use ruff_python_ast::source_code::{Locator, Stylist};
use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::{visitor, whitespace};
use crate::autofix::codemods::CodegenStylist;
use crate::checkers::ast::Checker;
use crate::cst::matchers::match_indented_block;
use crate::cst::matchers::match_module;
@ -315,6 +315,54 @@ fn negate<'a>(expression: &Expression<'a>) -> Expression<'a> {
}))
}
/// Propagate parentheses from a parent to a child expression, if necessary.
///
/// For example, when splitting:
/// ```python
/// assert (a and b ==
/// """)
/// ```
///
/// The parentheses need to be propagated to the right-most expression:
/// ```python
/// assert a
/// assert (b ==
/// "")
/// ```
fn parenthesize<'a>(expression: Expression<'a>, parent: &Expression<'a>) -> Expression<'a> {
if matches!(
expression,
Expression::Comparison(_)
| Expression::UnaryOperation(_)
| Expression::BinaryOperation(_)
| Expression::BooleanOperation(_)
| Expression::Attribute(_)
| Expression::Tuple(_)
| Expression::Call(_)
| Expression::GeneratorExp(_)
| Expression::ListComp(_)
| Expression::SetComp(_)
| Expression::DictComp(_)
| Expression::List(_)
| Expression::Set(_)
| Expression::Dict(_)
| Expression::Subscript(_)
| Expression::StarredElement(_)
| Expression::IfExp(_)
| Expression::Lambda(_)
| Expression::Yield(_)
| Expression::Await(_)
| Expression::ConcatenatedString(_)
| Expression::FormattedString(_)
| Expression::NamedExpr(_)
) {
if let (Some(left), Some(right)) = (parent.lpar().first(), parent.rpar().first()) {
return expression.with_parens(left.clone(), right.clone());
}
}
expression
}
/// Replace composite condition `assert a == "hello" and b == "world"` with two statements
/// `assert a == "hello"` and `assert b == "world"`.
fn fix_composite_condition(stmt: &Stmt, locator: &Locator, stylist: &Stylist) -> Result<Edit> {
@ -363,10 +411,6 @@ fn fix_composite_condition(stmt: &Stmt, locator: &Locator, stylist: &Stylist) ->
bail!("Expected simple statement to be an assert")
};
if !(assert_statement.test.lpar().is_empty() && assert_statement.test.rpar().is_empty()) {
bail!("Unable to split parenthesized condition");
}
// Extract the individual conditions.
let mut conditions: Vec<Expression> = Vec::with_capacity(2);
match &assert_statement.test {
@ -374,8 +418,8 @@ fn fix_composite_condition(stmt: &Stmt, locator: &Locator, stylist: &Stylist) ->
if matches!(op.operator, libcst_native::UnaryOp::Not { .. }) {
if let Expression::BooleanOperation(op) = &*op.expression {
if matches!(op.operator, BooleanOp::Or { .. }) {
conditions.push(negate(&op.left));
conditions.push(negate(&op.right));
conditions.push(parenthesize(negate(&op.left), &assert_statement.test));
conditions.push(parenthesize(negate(&op.right), &assert_statement.test));
} else {
bail!("Expected assert statement to be a composite condition");
}
@ -386,8 +430,8 @@ fn fix_composite_condition(stmt: &Stmt, locator: &Locator, stylist: &Stylist) ->
}
Expression::BooleanOperation(op) => {
if matches!(op.operator, BooleanOp::And { .. }) {
conditions.push(*op.left.clone());
conditions.push(*op.right.clone());
conditions.push(parenthesize(*op.left.clone(), &assert_statement.test));
conditions.push(parenthesize(*op.right.clone(), &assert_statement.test));
} else {
bail!("Expected assert statement to be a composite condition");
}

241
crates/ruff/src/rules/flake8_pytest_style/snapshots/ruff__rules__flake8_pytest_style__tests__PT018.snap
View file

@ -163,8 +163,8 @@ PT018.py:21:5: PT018 [*] Assertion should be broken down into multiple parts
22 | | message
23 | | """
| |_______^ PT018
24 |
25 | # recursive case
24 | assert (
25 | something
|
= help: Break down assertion into multiple parts
@ -177,131 +177,144 @@ PT018.py:21:5: PT018 [*] Assertion should be broken down into multiple parts
22 |+ assert something_else == """error
22 23 | message
23 24 | """
24 25 |
24 25 | assert (
PT018.py:26:5: PT018 [*] Assertion should be broken down into multiple parts
PT018.py:24:5: PT018 [*] Assertion should be broken down into multiple parts
|
25 | # recursive case
26 | assert not (a or not (b or c))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018
27 | assert not (a or not (b and c))
|
= help: Break down assertion into multiple parts
ℹ Suggested fix
23 23 | """
24 24 |
25 25 | # recursive case
26 |- assert not (a or not (b or c))
26 |+ assert not a
27 |+ assert (b or c)
27 28 | assert not (a or not (b and c))
28 29 |
29 30 | # detected, but no autofix for messages
PT018.py:27:5: PT018 [*] Assertion should be broken down into multiple parts
|
25 | # recursive case
26 | assert not (a or not (b or c))
27 | assert not (a or not (b and c))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018
28 |
29 | # detected, but no autofix for messages
|
= help: Break down assertion into multiple parts
ℹ Suggested fix
24 24 |
25 25 | # recursive case
26 26 | assert not (a or not (b or c))
27 |- assert not (a or not (b and c))
27 |+ assert not a
28 |+ assert (b and c)
28 29 |
29 30 | # detected, but no autofix for messages
30 31 | assert something and something_else, "error message"
PT018.py:30:5: PT018 Assertion should be broken down into multiple parts
|
29 | # detected, but no autofix for messages
30 | assert something and something_else, "error message"
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018
31 | assert not (something or something_else and something_third), "with message"
32 | # detected, but no autofix for mixed conditions (e.g. `a or b and c`)
|
= help: Break down assertion into multiple parts
PT018.py:31:5: PT018 Assertion should be broken down into multiple parts
|
29 | # detected, but no autofix for messages
30 | assert something and something_else, "error message"
31 | assert not (something or something_else and something_third), "with message"
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018
32 | # detected, but no autofix for mixed conditions (e.g. `a or b and c`)
33 | assert not (something or something_else and something_third)
|
= help: Break down assertion into multiple parts
PT018.py:33:5: PT018 Assertion should be broken down into multiple parts
|
31 | assert not (something or something_else and something_third), "with message"
32 | # detected, but no autofix for mixed conditions (e.g. `a or b and c`)
33 | assert not (something or something_else and something_third)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018
34 | # detected, but no autofix for parenthesized conditions
35 | assert (
|
= help: Break down assertion into multiple parts
PT018.py:35:5: PT018 Assertion should be broken down into multiple parts
|
33 | assert not (something or something_else and something_third)
34 | # detected, but no autofix for parenthesized conditions
35 | assert (
22 | message
23 | """
24 | assert (
| _____^
36 | | something
37 | | and something_else
38 | | == """error
39 | | message
40 | | """
41 | | )
25 | | something
26 | | and something_else
27 | | == """error
28 | | message
29 | | """
30 | | )
| |_____^ PT018
31 |
32 | # recursive case
|
= help: Break down assertion into multiple parts
ℹ Suggested fix
21 21 | assert something and something_else == """error
22 22 | message
23 23 | """
24 |+ assert something
24 25 | assert (
25 |- something
26 |- and something_else
26 |+ something_else
27 27 | == """error
28 28 | message
29 29 | """
PT018.py:33:5: PT018 [*] Assertion should be broken down into multiple parts
|
32 | # recursive case
33 | assert not (a or not (b or c))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018
34 | assert not (a or not (b and c))
|
= help: Break down assertion into multiple parts
ℹ Suggested fix
30 30 | )
31 31 |
32 32 | # recursive case
33 |- assert not (a or not (b or c))
33 |+ assert not a
34 |+ assert (b or c)
34 35 | assert not (a or not (b and c))
35 36 |
36 37 | # detected, but no autofix for messages
PT018.py:34:5: PT018 [*] Assertion should be broken down into multiple parts
|
32 | # recursive case
33 | assert not (a or not (b or c))
34 | assert not (a or not (b and c))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018
35 |
36 | # detected, but no autofix for messages
|
= help: Break down assertion into multiple parts
ℹ Suggested fix
31 31 |
32 32 | # recursive case
33 33 | assert not (a or not (b or c))
34 |- assert not (a or not (b and c))
34 |+ assert not a
35 |+ assert (b and c)
35 36 |
36 37 | # detected, but no autofix for messages
37 38 | assert something and something_else, "error message"
PT018.py:37:5: PT018 Assertion should be broken down into multiple parts
|
36 | # detected, but no autofix for messages
37 | assert something and something_else, "error message"
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018
38 | assert not (something or something_else and something_third), "with message"
39 | # detected, but no autofix for mixed conditions (e.g. `a or b and c`)
|
= help: Break down assertion into multiple parts
PT018.py:38:5: PT018 Assertion should be broken down into multiple parts
|
36 | # detected, but no autofix for messages
37 | assert something and something_else, "error message"
38 | assert not (something or something_else and something_third), "with message"
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018
39 | # detected, but no autofix for mixed conditions (e.g. `a or b and c`)
40 | assert not (something or something_else and something_third)
|
= help: Break down assertion into multiple parts
PT018.py:40:5: PT018 Assertion should be broken down into multiple parts
|
38 | assert not (something or something_else and something_third), "with message"
39 | # detected, but no autofix for mixed conditions (e.g. `a or b and c`)
40 | assert not (something or something_else and something_third)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018
|
= help: Break down assertion into multiple parts
PT018.py:44:1: PT018 [*] Assertion should be broken down into multiple parts
|
43 | assert something # OK
44 | assert something and something_else # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018
45 | assert something and something_else and something_third # Error
|
= help: Break down assertion into multiple parts
ℹ Suggested fix
41 41 |
42 42 |
43 43 | assert something # OK
44 |-assert something and something_else # Error
44 |+assert something
45 |+assert something_else
45 46 | assert something and something_else and something_third # Error
PT018.py:45:1: PT018 [*] Assertion should be broken down into multiple parts
|
44 | assert something # OK
45 | assert something and something_else # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018
46 | assert something and something_else and something_third # Error
|
= help: Break down assertion into multiple parts
ℹ Suggested fix
42 42 |
43 43 |
44 44 | assert something # OK
45 |-assert something and something_else # Error
45 |+assert something
46 |+assert something_else
46 47 | assert something and something_else and something_third # Error
PT018.py:46:1: PT018 [*] Assertion should be broken down into multiple parts
|
44 | assert something # OK
45 | assert something and something_else # Error
46 | assert something and something_else and something_third # Error
43 | assert something # OK
44 | assert something and something_else # Error
45 | assert something and something_else and something_third # Error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PT018
|
= help: Break down assertion into multiple parts
ℹ Suggested fix
43 43 |
44 44 | assert something # OK
45 45 | assert something and something_else # Error
46 |-assert something and something_else and something_third # Error
46 |+assert something and something_else
47 |+assert something_third
42 42 |
43 43 | assert something # OK
44 44 | assert something and something_else # Error
45 |-assert something and something_else and something_third # Error
45 |+assert something and something_else
46 |+assert something_third