From 923b4dd01e4df10bcde7759a7df2581855464840 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 12 Nov 2025 13:10:23 -0500 Subject: [PATCH 01/10] Fix panic when formatting comments in unary expressions Summary -- This is a second attempt at fixing #19226 based on the feedback in https://github.com/astral-sh/ruff/pull/20494#issuecomment-3467920065. We currently mark the comment in an expression like this: ```py if '' and (not # 0): pass ``` as a leading comment on `not`, eventually causing a panic in `Operand::has_unparenthesized_leading_comments` because the end of the comment is greater than the start of the expression. https://github.com/astral-sh/ruff/blob/a1d9cb5830eca9b63e7fb529504fc536e99bca23/crates/ruff_python_formatter/src/expression/binary_like.rs#L843 This PR fixes the issue by instead making such a comment a dangling comment on the unary expression. In the third commit, I instead tried making the comment a leading comment on the operand, which also looks pretty reasonable to me. Making it a dangling comment seems more in line with the docs on `handle_unary_op_comments`, though. I also tried deleting the leading comment logic in favor of the new dangling logic in the fifth commit before reverting in the sixth. This looks okay to me too, but the current state of the PR seems like the least invasive fix. Test Plan -- A new, minimized test case based on the issue. I also checked that the original snippet from the report works now. Co-authored-by: Takayuki Maeda From 6eebc35eec2b84e7ffdae790a28a4a4cdadf7ca5 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 12 Nov 2025 13:16:34 -0500 Subject: [PATCH 02/10] add failing test --- .../resources/test/fixtures/ruff/expression/unary.py | 5 +++++ 1 file changed, 5 insertions(+) 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 5c8ab8d8b31ab5..6240ec76cbe9ec 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,8 @@ def foo(): not (aaaaaaaaaaaaaaaaaaaaa[bbbbbbbb, ccccccc]) and dddddddddd < eeeeeeeeeeeeeee ): pass + +# Regression test for https://github.com/astral-sh/ruff/issues/19226 +if '' and (not # +0): + pass From a303d1aa492dfbfa1358f9f230e9ffc549df9121 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 12 Nov 2025 14:46:33 -0500 Subject: [PATCH 03/10] leading operand comment --- .../src/comments/placement.rs | 2 +- .../format@expression__unary.py.snap | 59 +++++++++++++------ ...s__expression_parentheses_comments.py.snap | 17 +++--- 3 files changed, 51 insertions(+), 27 deletions(-) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 234752ba5319bd..d0906d24b6e3ac 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1865,7 +1865,7 @@ fn handle_unary_op_comment<'a>( .find(|token| token.kind == SimpleTokenKind::LParen) .map_or(unary_op.operand.start(), |lparen| lparen.start()); if comment.end() < up_to { - CommentPlacement::leading(unary_op, comment) + CommentPlacement::leading(&*unary_op.operand, comment) } else { CommentPlacement::Default(comment) } 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 67f54f9da90a78..2cdde4e0d97f68 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,11 @@ def foo(): not (aaaaaaaaaaaaaaaaaaaaa[bbbbbbbb, ccccccc]) and dddddddddd < eeeeeeeeeeeeeee ): pass + +# Regression test for https://github.com/astral-sh/ruff/issues/19226 +if '' and (not # +0): + pass ``` ## Output @@ -250,31 +254,35 @@ if +( pass if ( + not # comment - not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( + ~ # comment - ~aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( + - # comment - -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( + + # comment - +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass @@ -283,8 +291,8 @@ if ( if ( # unary comment - # operand comment - not ( + not # operand comment + ( # comment aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb @@ -318,31 +326,36 @@ if ( ## Trailing operator comments -if ( # comment - not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +if ( + not + # comment + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( + ~ # comment - ~aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( + - # comment - -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( + + # comment - +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass @@ -362,13 +375,16 @@ if ( pass if ( + not # comment - not a + a ): pass -if ( # comment - not a +if ( + not + # comment + a ): pass @@ -385,9 +401,9 @@ if True: # Regression test for: https://github.com/astral-sh/ruff/issues/7448 x = ( # a - # b + not # b # c - not ( # d + ( # d # e True ) @@ -415,4 +431,13 @@ def foo(): not (aaaaaaaaaaaaaaaaaaaaa[bbbbbbbb, ccccccc]) and dddddddddd < eeeeeeeeeeeeeee ): pass + + +# Regression test for https://github.com/astral-sh/ruff/issues/19226 +if "" and ( + not + # + 0 +): + pass ``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap index 5e8e826231f3bc..4a145d5e703d7d 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/parentheses/expression_parentheses_comments.py -snapshot_kind: text --- ## Input ```python @@ -179,13 +178,13 @@ nested_parentheses4 = [ x = ( # unary comment - # in-between comment - not ( + not # in-between comment + ( # leading inner "a" ), - # in-between comment - not ( + not # in-between comment + ( # leading inner "b" ), @@ -194,8 +193,8 @@ x = ( "c" ), # 1 - # 2 - not ( # 3 + not # 2 + ( # 3 # 4 "d" ), @@ -203,8 +202,8 @@ x = ( if ( # unary comment - # in-between comment - not ( + not # in-between comment + ( # leading inner 1 ) From ecccfd2a560a07e3e65136868ddb83a68714613d Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 12 Nov 2025 15:01:39 -0500 Subject: [PATCH 04/10] dangling operator comment fix leading comments --- .../src/comments/placement.rs | 14 ++++-- .../format@expression__unary.py.snap | 49 ++++++------------- ...s__expression_parentheses_comments.py.snap | 17 ++++--- 3 files changed, 34 insertions(+), 46 deletions(-) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index d0906d24b6e3ac..1f763563f04546 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1861,11 +1861,17 @@ fn handle_unary_op_comment<'a>( | SimpleTokenKind::Plus | SimpleTokenKind::Minus ))); - let up_to = tokenizer + let lparen_start = tokenizer .find(|token| token.kind == SimpleTokenKind::LParen) - .map_or(unary_op.operand.start(), |lparen| lparen.start()); - if comment.end() < up_to { - CommentPlacement::leading(&*unary_op.operand, comment) + .map(|lparen| lparen.start()); + let up_to = lparen_start.unwrap_or(unary_op.operand.start()); + if lparen_start.is_none() + && comment.end() < unary_op.operand.start() + && comment.line_position().is_end_of_line() + { + CommentPlacement::dangling(unary_op, comment) + } else if comment.end() < up_to { + CommentPlacement::leading(unary_op, comment) } else { CommentPlacement::Default(comment) } 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 2cdde4e0d97f68..74ba326706750f 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 @@ -254,35 +254,31 @@ if +( pass if ( - not # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( - ~ # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + ~aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( - - # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( - + # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass @@ -291,8 +287,8 @@ if ( if ( # unary comment - not # operand comment - ( + # operand comment + not ( # comment aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb @@ -327,35 +323,27 @@ if ( ## Trailing operator comments if ( - not - # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( - ~ - # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + ~aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( - - - # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( - + - # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass @@ -375,17 +363,12 @@ if ( pass if ( - not # comment - a + not a ): pass -if ( - not - # comment - a -): +if not a: # comment pass # Regression test for: https://github.com/astral-sh/ruff/issues/7423 @@ -401,9 +384,9 @@ if True: # Regression test for: https://github.com/astral-sh/ruff/issues/7448 x = ( # a - not # b + # b # c - ( # d + not ( # d # e True ) @@ -435,9 +418,7 @@ def foo(): # Regression test for https://github.com/astral-sh/ruff/issues/19226 if "" and ( - not - # - 0 + not 0 # ): pass ``` diff --git a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap index 4a145d5e703d7d..5e8e826231f3bc 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap @@ -1,6 +1,7 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/parentheses/expression_parentheses_comments.py +snapshot_kind: text --- ## Input ```python @@ -178,13 +179,13 @@ nested_parentheses4 = [ x = ( # unary comment - not # in-between comment - ( + # in-between comment + not ( # leading inner "a" ), - not # in-between comment - ( + # in-between comment + not ( # leading inner "b" ), @@ -193,8 +194,8 @@ x = ( "c" ), # 1 - not # 2 - ( # 3 + # 2 + not ( # 3 # 4 "d" ), @@ -202,8 +203,8 @@ x = ( if ( # unary comment - not # in-between comment - ( + # in-between comment + not ( # leading inner 1 ) From 1cf5eff9f468398a234506d8691ee8b25de055f2 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 12 Nov 2025 15:31:22 -0500 Subject: [PATCH 05/10] try only dangling --- .../src/comments/placement.rs | 9 ++------ .../format@expression__unary.py.snap | 23 +++++++++++-------- ...s__expression_parentheses_comments.py.snap | 17 +++++++------- 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 1f763563f04546..e1f620845b2b20 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1861,17 +1861,12 @@ fn handle_unary_op_comment<'a>( | SimpleTokenKind::Plus | SimpleTokenKind::Minus ))); - let lparen_start = tokenizer - .find(|token| token.kind == SimpleTokenKind::LParen) - .map(|lparen| lparen.start()); - let up_to = lparen_start.unwrap_or(unary_op.operand.start()); - if lparen_start.is_none() + let lparen = tokenizer.find(|token| token.kind == SimpleTokenKind::LParen); + if lparen.is_none() && comment.end() < unary_op.operand.start() && comment.line_position().is_end_of_line() { CommentPlacement::dangling(unary_op, comment) - } else if comment.end() < up_to { - CommentPlacement::leading(unary_op, comment) } else { CommentPlacement::Default(comment) } 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 74ba326706750f..f2dbce006be512 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 @@ -254,31 +254,35 @@ if +( pass if ( + not # comment - not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( + ~ # comment - ~aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( + - # comment - -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( + + # comment - +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass @@ -287,8 +291,8 @@ if ( if ( # unary comment - # operand comment - not ( + not # operand comment + ( # comment aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb @@ -363,8 +367,9 @@ if ( pass if ( + not # comment - not a + a ): pass @@ -384,9 +389,9 @@ if True: # Regression test for: https://github.com/astral-sh/ruff/issues/7448 x = ( # a - # b + not # b # c - not ( # d + ( # d # e True ) diff --git a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap index 5e8e826231f3bc..4a145d5e703d7d 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap @@ -1,7 +1,6 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/parentheses/expression_parentheses_comments.py -snapshot_kind: text --- ## Input ```python @@ -179,13 +178,13 @@ nested_parentheses4 = [ x = ( # unary comment - # in-between comment - not ( + not # in-between comment + ( # leading inner "a" ), - # in-between comment - not ( + not # in-between comment + ( # leading inner "b" ), @@ -194,8 +193,8 @@ x = ( "c" ), # 1 - # 2 - not ( # 3 + not # 2 + ( # 3 # 4 "d" ), @@ -203,8 +202,8 @@ x = ( if ( # unary comment - # in-between comment - not ( + not # in-between comment + ( # leading inner 1 ) From f49fda317ee440b8785136f049458cbf25bcc7ab Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Wed, 12 Nov 2025 15:31:41 -0500 Subject: [PATCH 06/10] Revert "try only dangling" This reverts commit 4c8540ede798f544a895225b541f46552649fd7b. --- .../src/comments/placement.rs | 9 ++++++-- .../format@expression__unary.py.snap | 23 ++++++++----------- ...s__expression_parentheses_comments.py.snap | 17 +++++++------- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index e1f620845b2b20..1f763563f04546 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1861,12 +1861,17 @@ fn handle_unary_op_comment<'a>( | SimpleTokenKind::Plus | SimpleTokenKind::Minus ))); - let lparen = tokenizer.find(|token| token.kind == SimpleTokenKind::LParen); - if lparen.is_none() + let lparen_start = tokenizer + .find(|token| token.kind == SimpleTokenKind::LParen) + .map(|lparen| lparen.start()); + let up_to = lparen_start.unwrap_or(unary_op.operand.start()); + if lparen_start.is_none() && comment.end() < unary_op.operand.start() && comment.line_position().is_end_of_line() { CommentPlacement::dangling(unary_op, comment) + } else if comment.end() < up_to { + CommentPlacement::leading(unary_op, comment) } else { CommentPlacement::Default(comment) } 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 f2dbce006be512..74ba326706750f 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 @@ -254,35 +254,31 @@ if +( pass if ( - not # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( - ~ # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + ~aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( - - # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( - + # comment - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass @@ -291,8 +287,8 @@ if ( if ( # unary comment - not # operand comment - ( + # operand comment + not ( # comment aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb @@ -367,9 +363,8 @@ if ( pass if ( - not # comment - a + not a ): pass @@ -389,9 +384,9 @@ if True: # Regression test for: https://github.com/astral-sh/ruff/issues/7448 x = ( # a - not # b + # b # c - ( # d + not ( # d # e True ) diff --git a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap index 4a145d5e703d7d..5e8e826231f3bc 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap @@ -1,6 +1,7 @@ --- source: crates/ruff_python_formatter/tests/fixtures.rs input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/parentheses/expression_parentheses_comments.py +snapshot_kind: text --- ## Input ```python @@ -178,13 +179,13 @@ nested_parentheses4 = [ x = ( # unary comment - not # in-between comment - ( + # in-between comment + not ( # leading inner "a" ), - not # in-between comment - ( + # in-between comment + not ( # leading inner "b" ), @@ -193,8 +194,8 @@ x = ( "c" ), # 1 - not # 2 - ( # 3 + # 2 + not ( # 3 # 4 "d" ), @@ -202,8 +203,8 @@ x = ( if ( # unary comment - not # in-between comment - ( + # in-between comment + not ( # leading inner 1 ) From 9bee558c3a376f885e3eeb8267406322b5b5121f Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 13 Nov 2025 09:37:15 -0500 Subject: [PATCH 07/10] add more variations on issue test case --- .../test/fixtures/ruff/expression/unary.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) 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 6240ec76cbe9ec..ea17e4a66f8c07 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 @@ -194,7 +194,18 @@ def foo(): ): pass -# Regression test for https://github.com/astral-sh/ruff/issues/19226 +# 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 From e0b0b54b253244641283973f2d04d7a27f83eb68 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 13 Nov 2025 10:09:37 -0500 Subject: [PATCH 08/10] try a different approach --- .../src/comments/placement.rs | 12 ++--- .../src/expression/binary_like.rs | 6 ++- .../format@expression__unary.py.snap | 46 +++++++++++++++---- 3 files changed, 44 insertions(+), 20 deletions(-) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 1f763563f04546..234752ba5319bd 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1861,16 +1861,10 @@ fn handle_unary_op_comment<'a>( | SimpleTokenKind::Plus | SimpleTokenKind::Minus ))); - let lparen_start = tokenizer + let up_to = tokenizer .find(|token| token.kind == SimpleTokenKind::LParen) - .map(|lparen| lparen.start()); - let up_to = lparen_start.unwrap_or(unary_op.operand.start()); - if lparen_start.is_none() - && comment.end() < unary_op.operand.start() - && comment.line_position().is_end_of_line() - { - CommentPlacement::dangling(unary_op, comment) - } else if comment.end() < up_to { + .map_or(unary_op.operand.start(), |lparen| lparen.start()); + if comment.end() < up_to { CommentPlacement::leading(unary_op, comment) } else { CommentPlacement::Default(comment) diff --git a/crates/ruff_python_formatter/src/expression/binary_like.rs b/crates/ruff_python_formatter/src/expression/binary_like.rs index 5d8660845224c5..bc6df28cd1d853 100644 --- a/crates/ruff_python_formatter/src/expression/binary_like.rs +++ b/crates/ruff_python_formatter/src/expression/binary_like.rs @@ -836,7 +836,8 @@ impl<'a> Operand<'a> { let leading = comments.leading(*expression); if is_expression_parenthesized((*expression).into(), comments.ranges(), source) { leading.iter().any(|comment| { - !comment.is_formatted() + comment.end() <= expression.start() + && !comment.is_formatted() && matches!( SimpleTokenizer::new( source, @@ -922,7 +923,8 @@ impl Format> for Operand<'_> { let leading_before_parentheses_end = leading .iter() .rposition(|comment| { - comment.is_unformatted() + comment.end() <= expression.start() + && comment.is_unformatted() && matches!( SimpleTokenizer::new( f.context().source(), 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 74ba326706750f..697fe3747c4ebd 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,10 +200,21 @@ def foo(): ): pass -# Regression test for https://github.com/astral-sh/ruff/issues/19226 +# 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 @@ -322,28 +333,31 @@ if ( ## Trailing operator comments -if ( - not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment +if ( # comment + not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( - ~aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment + # comment + ~aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( - -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment + # comment + -aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass if ( - +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment + # comment + +aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb ): pass @@ -368,7 +382,9 @@ if ( ): pass -if not a: # comment +if ( # comment + not a +): pass # Regression test for: https://github.com/astral-sh/ruff/issues/7423 @@ -416,9 +432,21 @@ def foo(): pass -# Regression test for https://github.com/astral-sh/ruff/issues/19226 +# 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 # + not ( # + 0 + ) ): pass ``` From ca5f4cc96fd513dd1d777f5a0ff92b23ae335a5c Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 14 Nov 2025 15:50:33 -0500 Subject: [PATCH 09/10] factor out has_parenthesis_between, add some docs --- .../src/expression/binary_like.rs | 81 ++++++++++++------- 1 file changed, 51 insertions(+), 30 deletions(-) diff --git a/crates/ruff_python_formatter/src/expression/binary_like.rs b/crates/ruff_python_formatter/src/expression/binary_like.rs index bc6df28cd1d853..7607cf21ca6fa2 100644 --- a/crates/ruff_python_formatter/src/expression/binary_like.rs +++ b/crates/ruff_python_formatter/src/expression/binary_like.rs @@ -835,22 +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.end() <= expression.start() - && !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() } @@ -878,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(); @@ -923,20 +957,7 @@ impl Format> for Operand<'_> { let leading_before_parentheses_end = leading .iter() .rposition(|comment| { - comment.end() <= expression.start() - && 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); From fc513684d20fe8ea12a0caf9be6cb625f71b4330 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 14 Nov 2025 17:42:47 -0500 Subject: [PATCH 10/10] update handle_unary_op_comment docs --- .../ruff_python_formatter/src/comments/placement.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 234752ba5319bd..39393b2e6372d5 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,