From 4c7765877df50a8cfe9159706ce8f321af125c84 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Mon, 22 Sep 2025 02:04:32 +0900 Subject: [PATCH] Fix panic when comments appear between unary operators and operands --- .../test/fixtures/ruff/expression/unary.py | 17 +++++ .../src/expression/binary_like.rs | 66 ++++++++++++------- .../format@expression__unary.py.snap | 39 +++++++++++ 3 files changed, 100 insertions(+), 22 deletions(-) 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 5c8ab8d8b3..1076763c63 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,20 @@ 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 ( + # unary comment + not + # operand comment + ( + # comment + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + ) +): + pass diff --git a/crates/ruff_python_formatter/src/expression/binary_like.rs b/crates/ruff_python_formatter/src/expression/binary_like.rs index 5d86608452..134e8dbf1e 100644 --- a/crates/ruff_python_formatter/src/expression/binary_like.rs +++ b/crates/ruff_python_formatter/src/expression/binary_like.rs @@ -515,6 +515,44 @@ const fn is_simple_power_operand(expr: &Expr) -> bool { } } +/// Safely checks if there's an `LParen` token between a comment and expression start, +/// with special handling for unary operations to avoid `TextRange` panics. +fn has_lparen_between_comment_and_expression( + comment: &SourceComment, + expression: &Expr, + source: &str, +) -> bool { + // For unary operations, we need to use the operand's start position instead of the + // expression's start position. This prevents panics when comments appear between + // the unary operator and its operand. + // + // Example that would panic without this fix: + // if '' and ( + // not + // # comment + // 0): + // pass + // + // In this case: + // - expression.start() points to 'not' + // - comment.end() points after '#' + // - expression.start() < comment.end(), so TextRange::new would panic + // - comment.end() <= operand.start(), so using operand works correctly + let operand_start = match expression { + Expr::UnaryOp(unary_op) => unary_op.operand.start(), + _ => expression.start(), + }; + let tokenizer = SimpleTokenizer::new(source, TextRange::new(comment.end(), operand_start)); + + matches!( + tokenizer.skip_trivia().next(), + Some(SimpleToken { + kind: SimpleTokenKind::LParen, + .. + }) + ) +} + /// Owned [`FlatBinaryExpressionSlice`]. Read the [`FlatBinaryExpressionSlice`] documentation for more details about the data structure. #[derive(Debug)] struct FlatBinaryExpression<'a>(SmallVec<[OperandOrOperator<'a>; 8]>); @@ -837,17 +875,8 @@ impl<'a> Operand<'a> { 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, - .. - }) + && has_lparen_between_comment_and_expression( + comment, expression, source, ) }) } else { @@ -923,17 +952,10 @@ impl Format> for Operand<'_> { .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_lparen_between_comment_and_expression( + 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 67f54f9da9..208e0a2efb 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 @@ -200,6 +200,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 ( + # unary comment + not + # operand comment + ( + # comment + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + ) +): + pass ``` ## Output @@ -415,4 +432,26 @@ 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 + # unary comment + # operand comment + ( + not ( + # comment + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + ) + ) +): + pass ```