diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C413.py b/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C413.py index 29f381e0d71b8..28b3e1ee3f19f 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C413.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_comprehensions/C413.py @@ -14,9 +14,6 @@ reversed(sorted(i for i in range(42))) reversed(sorted((i for i in range(42)), reverse=True)) - -def reversed(*args, **kwargs): - return None - - -reversed(sorted(x, reverse=True)) +# Regression test for: https://github.com/astral-sh/ruff/issues/10335 +reversed(sorted([1, 2, 3], reverse=False or True)) +reversed(sorted([1, 2, 3], reverse=(False or True))) diff --git a/crates/ruff_linter/src/cst/helpers.rs b/crates/ruff_linter/src/cst/helpers.rs index 731ff8a56d2ac..b41d6348f8605 100644 --- a/crates/ruff_linter/src/cst/helpers.rs +++ b/crates/ruff_linter/src/cst/helpers.rs @@ -1,5 +1,6 @@ use libcst_native::{ - Expression, Name, ParenthesizableWhitespace, SimpleWhitespace, UnaryOperation, + Expression, LeftParen, Name, ParenthesizableWhitespace, ParenthesizedNode, RightParen, + SimpleWhitespace, UnaryOperation, }; /// Return a [`ParenthesizableWhitespace`] containing a single space. @@ -24,6 +25,7 @@ pub(crate) fn negate<'a>(expression: &Expression<'a>) -> Expression<'a> { } } + // If the expression is `True` or `False`, return the opposite. if let Expression::Name(ref expression) = expression { match expression.value { "True" => { @@ -44,11 +46,32 @@ pub(crate) fn negate<'a>(expression: &Expression<'a>) -> Expression<'a> { } } + // If the expression is higher precedence than the unary `not`, we need to wrap it in + // parentheses. + // + // For example: given `a and b`, we need to return `not (a and b)`, rather than `not a and b`. + // + // See: + let needs_parens = matches!( + expression, + Expression::BooleanOperation(_) + | Expression::IfExp(_) + | Expression::Lambda(_) + | Expression::NamedExpr(_) + ); + let has_parens = !expression.lpar().is_empty() && !expression.rpar().is_empty(); + // Otherwise, wrap in a `not` operator. Expression::UnaryOperation(Box::new(UnaryOperation { operator: libcst_native::UnaryOp::Not { whitespace_after: space(), }, - expression: Box::new(expression.clone()), + expression: Box::new(if needs_parens && !has_parens { + expression + .clone() + .with_parens(LeftParen::default(), RightParen::default()) + } else { + expression.clone() + }), lpar: vec![], rpar: vec![], })) diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C413_C413.py.snap b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C413_C413.py.snap index 4b7cd7a5641ec..def04f25aea8f 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C413_C413.py.snap +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C413_C413.py.snap @@ -205,7 +205,7 @@ C413.py:14:1: C413 [*] Unnecessary `reversed` call around `sorted()` 14 |+sorted((i for i in range(42)), reverse=True) 15 15 | reversed(sorted((i for i in range(42)), reverse=True)) 16 16 | -17 17 | +17 17 | # Regression test for: https://github.com/astral-sh/ruff/issues/10335 C413.py:15:1: C413 [*] Unnecessary `reversed` call around `sorted()` | @@ -213,6 +213,8 @@ C413.py:15:1: C413 [*] Unnecessary `reversed` call around `sorted()` 14 | reversed(sorted(i for i in range(42))) 15 | reversed(sorted((i for i in range(42)), reverse=True)) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C413 +16 | +17 | # Regression test for: https://github.com/astral-sh/ruff/issues/10335 | = help: Remove unnecessary `reversed` call @@ -223,7 +225,38 @@ C413.py:15:1: C413 [*] Unnecessary `reversed` call around `sorted()` 15 |-reversed(sorted((i for i in range(42)), reverse=True)) 15 |+sorted((i for i in range(42)), reverse=False) 16 16 | -17 17 | -18 18 | def reversed(*args, **kwargs): +17 17 | # Regression test for: https://github.com/astral-sh/ruff/issues/10335 +18 18 | reversed(sorted([1, 2, 3], reverse=False or True)) +C413.py:18:1: C413 [*] Unnecessary `reversed` call around `sorted()` + | +17 | # Regression test for: https://github.com/astral-sh/ruff/issues/10335 +18 | reversed(sorted([1, 2, 3], reverse=False or True)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C413 +19 | reversed(sorted([1, 2, 3], reverse=(False or True))) + | + = help: Remove unnecessary `reversed` call + +ℹ Unsafe fix +15 15 | reversed(sorted((i for i in range(42)), reverse=True)) +16 16 | +17 17 | # Regression test for: https://github.com/astral-sh/ruff/issues/10335 +18 |-reversed(sorted([1, 2, 3], reverse=False or True)) + 18 |+sorted([1, 2, 3], reverse=not (False or True)) +19 19 | reversed(sorted([1, 2, 3], reverse=(False or True))) +C413.py:19:1: C413 [*] Unnecessary `reversed` call around `sorted()` + | +17 | # Regression test for: https://github.com/astral-sh/ruff/issues/10335 +18 | reversed(sorted([1, 2, 3], reverse=False or True)) +19 | reversed(sorted([1, 2, 3], reverse=(False or True))) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C413 + | + = help: Remove unnecessary `reversed` call + +ℹ Unsafe fix +16 16 | +17 17 | # Regression test for: https://github.com/astral-sh/ruff/issues/10335 +18 18 | reversed(sorted([1, 2, 3], reverse=False or True)) +19 |-reversed(sorted([1, 2, 3], reverse=(False or True))) + 19 |+sorted([1, 2, 3], reverse=not (False or True)) diff --git a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT018.snap b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT018.snap index cc8b68bc3d556..cf96ebfbb180e 100644 --- a/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT018.snap +++ b/crates/ruff_linter/src/rules/flake8_pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT018.snap @@ -413,5 +413,3 @@ PT018.py:65:5: PT018 [*] Assertion should be broken down into multiple parts 70 72 | 71 73 | assert (not self.find_graph_output(node.output[0]) or 72 74 | self.find_graph_input(node.input[0])) - -