Skip to content

Commit 90ba40c

Browse files
authored
Fix zulip unstable formatting with end-of-line comments (#6386)
## Bug Given ```python x = () - (# ) ``` the comment is a dangling comment of the empty tuple. This is an end-of-line comment so it may move after the expression. It still expands the parent, so the operator breaks: ```python x = ( () - () # ) ``` In the next formatting pass, the comment is not a trailing tuple but a trailing bin op comment, so the bin op doesn't break anymore. The comment again expands the parent, so we still add the superfluous parentheses ```python x = ( () - () # ) ``` ## Fix The new formatting is to keep the comment on the empty tuple. This is a log uglier and again has additional outer parentheses, but it's stable: ```python x = ( () - ( # ) ) ``` ## Alternatives Black formats all the examples above as ```python x = () - () # ``` which i find better. I would be happy about any suggestions for better solutions than the current one. I'd mainly need a workaround for expand parent having an effect on the bin op instead of first moving the comment to the end and then applying expand parent to the assign statement.
1 parent 2bd3453 commit 90ba40c

11 files changed

+73
-8
lines changed

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,3 +211,14 @@
211211
log(self.price / self.strike)
212212
+ (self.risk_free - self.div_cont + 0.5 * (self.sigma**2)) * self.exp_time
213213
) / self.sigmaT
214+
215+
# Stability with end-of-line comments between empty tuples and bin op
216+
x = () - (#
217+
)
218+
x = (
219+
()
220+
- () #
221+
)
222+
x = (
223+
() - () #
224+
)

crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/delete.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,3 +73,7 @@
7373

7474
del ( # dangling end of line comment
7575
)
76+
77+
del ( # dangling end of line comment
78+
# dangling own line comment
79+
) # trailing statement comment

crates/ruff_python_formatter/src/builders.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,16 @@ impl<'ast> Format<PyFormatContext<'ast>> for EmptyWithDanglingComments<'_> {
253253
self.opening,
254254
// end-of-line comments
255255
trailing_comments(&self.comments[..end_of_line_split]),
256+
// Avoid unstable formatting with
257+
// ```python
258+
// x = () - (#
259+
// )
260+
// ```
261+
// Without this the comment would go after the empty tuple first, but still expand
262+
// the bin op. In the second formatting pass they are trailing bin op comments
263+
// so the bin op collapse. Suboptimally we keep parentheses around the bin op in
264+
// either case.
265+
(!self.comments[..end_of_line_split].is_empty()).then_some(hard_line_break()),
256266
// own line comments, which need to be indented
257267
soft_block_indent(&dangling_comments(&self.comments[end_of_line_split..])),
258268
self.closing

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,17 @@ for user_id in set(target_user_ids) - {u.user_id for u in updates}:
217217
log(self.price / self.strike)
218218
+ (self.risk_free - self.div_cont + 0.5 * (self.sigma**2)) * self.exp_time
219219
) / self.sigmaT
220+
221+
# Stability with end-of-line comments between empty tuples and bin op
222+
x = () - (#
223+
)
224+
x = (
225+
()
226+
- () #
227+
)
228+
x = (
229+
() - () #
230+
)
220231
```
221232

222233
## Output
@@ -488,6 +499,19 @@ for user_id in set(target_user_ids) - {u.user_id for u in updates}:
488499
log(self.price / self.strike)
489500
+ (self.risk_free - self.div_cont + 0.5 * (self.sigma**2)) * self.exp_time
490501
) / self.sigmaT
502+
503+
# Stability with end-of-line comments between empty tuples and bin op
504+
x = (
505+
()
506+
- ( #
507+
)
508+
)
509+
x = (
510+
() - () #
511+
)
512+
x = (
513+
() - () #
514+
)
491515
```
492516

493517

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,8 @@ f(
213213
a.very_long_function_function_that_is_so_long_that_it_expands_the_parent_but_its_only_a_single_argument()
214214
)
215215
216-
f() # abc
216+
f( # abc
217+
)
217218
218219
f( # abc
219220
# abc

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,8 @@ a = {
129129
3: True,
130130
}
131131
132-
x = {} # dangling end of line comment
132+
x = { # dangling end of line comment
133+
}
133134
```
134135

135136

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,8 @@ a = (
165165
# Regression test: lambda empty arguments ranges were too long, leading to unstable
166166
# formatting
167167
(
168-
lambda: (), #
168+
lambda: ( #
169+
),
169170
)
170171
171172

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ c1 = [ # trailing open bracket
6262
```py
6363
# Dangling comment placement in empty lists
6464
# Regression test for https://github.com/python/cpython/blob/03160630319ca26dcbbad65225da4248e54c45ec/Tools/c-analyzer/c_analyzer/datafiles.py#L14-L16
65-
a1 = [] # a
65+
a1 = [ # a
66+
]
6667
a2 = [ # a
6768
# b
6869
]
@@ -93,7 +94,8 @@ c1 = [ # trailing open bracket
9394
] # trailing close bracket
9495
9596
96-
[] # end-of-line comment
97+
[ # end-of-line comment
98+
]
9799
98100
[ # end-of-line comment
99101
# own-line comment

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ del (
7979
8080
del ( # dangling end of line comment
8181
)
82+
83+
del ( # dangling end of line comment
84+
# dangling own line comment
85+
) # trailing statement comment
8286
```
8387

8488
## Output
@@ -215,7 +219,12 @@ del (
215219
) # Completed
216220
# Done
217221
218-
del () # dangling end of line comment
222+
del ( # dangling end of line comment
223+
)
224+
225+
del ( # dangling end of line comment
226+
# dangling own line comment
227+
) # trailing statement comment
219228
```
220229

221230

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -845,7 +845,8 @@ def f( # first
845845
...
846846

847847

848-
def f(): # first # second
848+
def f( # first
849+
): # second
849850
...
850851

851852

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,8 @@ raise aksjdhflsakhdflkjsadlfajkslhfdkjsaldajlahflashdfljahlfk < (
192192
) # the other end
193193
# sneaky comment
194194
195-
raise () # another comment
195+
raise ( # another comment
196+
)
196197
197198
raise () # what now
198199

0 commit comments

Comments
 (0)