Wrap expressions in parentheses when negating

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)))

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: <https://docs.python.org/3/reference/expressions.html#operator-precedence>
+    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![],
     }))

crates/ruff_linter/src/rules/flake8_comprehensions/snapshots/ruff_linter__rules__flake8_comprehensions__tests__C413_C413.py.snap

@@ -205,14 +205,16 @@ 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()`
    |
 13 | # Regression test for: https://github.com/astral-sh/ruff/issues/7289
 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))

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]))
-
-