Fix panic when formatting comments in unary expressions

crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unary.py

@@ -193,3 +193,19 @@ def foo():
         not (aaaaaaaaaaaaaaaaaaaaa[bbbbbbbb, ccccccc]) and dddddddddd < eeeeeeeeeeeeeee
     ):
         pass
+
+# Regression tests for https://github.com/astral-sh/ruff/issues/19226
+if '' and (not #
+0):
+    pass
+
+if '' and (not #
+(0)
+):
+    pass
+
+if '' and (not
+	(  #
+	0
+)):
+    pass

crates/ruff_python_formatter/src/comments/placement.rs

@@ -1831,7 +1831,7 @@ fn handle_lambda_comment<'a>(
     CommentPlacement::Default(comment)
 }
 
-/// Move comment between a unary op and its operand before the unary op by marking them as trailing.
+/// Move comment between a unary op and its operand before the unary op by marking them as leading.
 ///
 /// For example, given:
 /// ```python
@@ -1841,8 +1841,13 @@ fn handle_lambda_comment<'a>(
 /// )
 /// ```
 ///
-/// The `# comment` will be attached as a dangling comment on the enclosing node, to ensure that
-/// it remains on the same line as the operator.
+/// The `# comment` will be attached as a leading comment on the unary op, to ensure that
+/// it doesn't fall between the operator and its operand:
+/// ```python
+/// (  # comment
+///     not True
+/// )
+/// ```
 fn handle_unary_op_comment<'a>(
     comment: DecoratedComment<'a>,
     unary_op: &'a ast::ExprUnaryOp,

crates/ruff_python_formatter/src/expression/binary_like.rs

@@ -835,21 +835,9 @@ impl<'a> Operand<'a> {
             Operand::Middle { expression } | Operand::Right { expression, .. } => {
                 let leading = comments.leading(*expression);
                 if is_expression_parenthesized((*expression).into(), comments.ranges(), source) {
-                    leading.iter().any(|comment| {
-                        !comment.is_formatted()
-                            && matches!(
-                                SimpleTokenizer::new(
-                                    source,
-                                    TextRange::new(comment.end(), expression.start()),
-                                )
-                                .skip_trivia()
-                                .next(),
-                                Some(SimpleToken {
-                                    kind: SimpleTokenKind::LParen,
-                                    ..
-                                })
-                            )
-                    })
+                    leading
+                        .iter()
+                        .any(|comment| has_parenthesis_between(comment, expression, source))
                 } else {
                     !leading.is_empty()
                 }
@@ -877,6 +865,53 @@ impl<'a> Operand<'a> {
     }
 }
 
+/// Returns `true` if a left parenthesis is the first non-trivia token found between `comment` and
+/// `expression`.
+///
+/// This can be used to detect unparenthesized leading comments, for example:
+///
+/// ```py
+/// # leading comment
+/// (
+///     expression
+/// )
+/// ```
+fn has_parenthesis_between(comment: &SourceComment, expression: &Expr, source: &str) -> bool {
+    // We adjust the comment placement for unary operators to avoid splitting the operator and its
+    // operand as in:
+    //
+    // ```py
+    // (
+    //     not
+    //     # comment
+    //     True
+    // )
+    // ```
+    //
+    // but making these leading comments means that the comment range falls after the start of the
+    // expression.
+    //
+    // This case can only occur when the comment is after the operator, so it's safe to return
+    // `false` here. The comment will definitely be parenthesized if the the operator is.
+    //
+    // Unary operators are the only known case of this, so `debug_assert!` that it stays that way.
+    debug_assert!(
+        expression.is_unary_op_expr() || comment.end() <= expression.start(),
+        "Expected leading comment to come before its expression",
+    );
+    comment.end() <= expression.start()
+        && !comment.is_formatted()
+        && matches!(
+            SimpleTokenizer::new(source, TextRange::new(comment.end(), expression.start()),)
+                .skip_trivia()
+                .next(),
+            Some(SimpleToken {
+                kind: SimpleTokenKind::LParen,
+                ..
+            })
+        )
+}
+
 impl Format<PyFormatContext<'_>> for Operand<'_> {
     fn fmt(&self, f: &mut Formatter<PyFormatContext<'_>>) -> FormatResult<()> {
         let expression = self.expression();
@@ -922,19 +957,7 @@ impl Format<PyFormatContext<'_>> for Operand<'_> {
             let leading_before_parentheses_end = leading
                 .iter()
                 .rposition(|comment| {
-                    comment.is_unformatted()
-                        && matches!(
-                            SimpleTokenizer::new(
-                                f.context().source(),
-                                TextRange::new(comment.end(), expression.start()),
-                            )
-                            .skip_trivia()
-                            .next(),
-                            Some(SimpleToken {
-                                kind: SimpleTokenKind::LParen,
-                                ..
-                            })
-                        )
+                    has_parenthesis_between(comment, expression, f.context().source())
                 })
                 .map_or(0, |position| position + 1);
 

crates/ruff_python_formatter/tests/snapshots/format@expression__unary.py.snap

@@ -1,7 +1,6 @@
 ---
 source: crates/ruff_python_formatter/tests/fixtures.rs
 input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unary.py
-snapshot_kind: text
 ---
 ## Input
 ```python
@@ -200,6 +199,22 @@ def foo():
         not (aaaaaaaaaaaaaaaaaaaaa[bbbbbbbb, ccccccc]) and dddddddddd < eeeeeeeeeeeeeee
     ):
         pass
+
+# Regression tests for https://github.com/astral-sh/ruff/issues/19226
+if '' and (not #
+0):
+    pass
+
+if '' and (not #
+(0)
+):
+    pass
+
+if '' and (not
+	(  #
+	0
+)):
+    pass
 ```
 
 ## Output
@@ -415,4 +430,23 @@ def foo():
         not (aaaaaaaaaaaaaaaaaaaaa[bbbbbbbb, ccccccc]) and dddddddddd < eeeeeeeeeeeeeee
     ):
         pass
+
+
+# Regression tests for https://github.com/astral-sh/ruff/issues/19226
+if "" and (  #
+    not 0
+):
+    pass
+
+if "" and (  #
+    not (0)
+):
+    pass
+
+if "" and (
+    not (  #
+        0
+    )
+):
+    pass
 ```