diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unary.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unary.py index 5c8ab8d8b31ab..ea17e4a66f8c0 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unary.py +++ b/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 diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 234752ba5319b..39393b2e6372d 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/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, diff --git a/crates/ruff_python_formatter/src/expression/binary_like.rs b/crates/ruff_python_formatter/src/expression/binary_like.rs index 5d8660845224c..7607cf21ca6fa 100644 --- a/crates/ruff_python_formatter/src/expression/binary_like.rs +++ b/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> for Operand<'_> { fn fmt(&self, f: &mut Formatter>) -> FormatResult<()> { let expression = self.expression(); @@ -922,19 +957,7 @@ impl Format> 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); diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__unary.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__unary.py.snap index 67f54f9da90a7..697fe3747c4eb 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__unary.py.snap +++ b/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 ```