Skip to content

Commit 0a6396b

Browse files
batch --line-ranges node conversion to avoid quadratic removal (#5213)
* batch --line-ranges node conversion to avoid quadratic removal * Fix decorator handling in batched --line-ranges conversion Deferring the STANDALONE_COMMENT conversions broke single-line decorated blocks. The per-node path relied on the decorator being collapsed in place before the decorated node was converted: it re-inserted a NEWLINE between the two and read the migrated prefix off the replaced first leaf. With the conversions batched, the decorator is still its own child carrying its NEWLINE, so re-inserting one doubled the blank line, and the enclosing decorated conversion picked up an empty prefix after the decorator run had already cleared it. Drop the now-redundant NEWLINE insertion, remember each first leaf's taken prefix so an inner and an outer conversion over the same leaf reuse it, and zero that leaf's lineno to mirror the fresh line-0 STANDALONE_COMMENT the in-place path used to leave behind. Fixes line_ranges_decorator_edge_case. --------- Co-authored-by: cobalt <61329810+cobaltt7@users.noreply.github.com>
1 parent 56ba38a commit 0a6396b

2 files changed

Lines changed: 153 additions & 55 deletions

File tree

CHANGES.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@
113113
`--preview` string processing of `"%s ..." % (a, b, c, ...)` or a string with a
114114
backslash continuation) by resuming the child lookup in `append_leaves` instead of
115115
rescanning each leaf's parent from the start (#5199)
116+
- Improve performance of `--line-ranges` on files with many sibling blocks (a long
117+
`if`/`elif` chain, a `match` with many cases, or many top-level definitions) by
118+
splicing the unchanged blocks into each parent's child list in a single pass rather
119+
than removing and re-inserting each one, which rescanned and shifted the whole child
120+
list on every conversion (#5213)
116121

117122
### Output
118123

src/black/ranges.py

Lines changed: 148 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -190,11 +190,103 @@ def convert_unchanged_lines(src_node: Node, lines: Collection[tuple[int, int]])
190190
lines_set: set[int] = set()
191191
for start, end in lines:
192192
lines_set.update(range(start, end + 1))
193-
visitor = _TopLevelStatementsVisitor(lines_set)
193+
replacements = _NodeReplacements()
194+
visitor = _TopLevelStatementsVisitor(lines_set, replacements)
194195
_ = list(visitor.visit(src_node)) # Consume all results.
196+
replacements.apply()
195197
_convert_unchanged_line_by_line(src_node, lines_set)
196198

197199

200+
class _NodeReplacements:
201+
"""Collects STANDALONE_COMMENT conversions and applies them per parent.
202+
203+
Converting a node splices a `STANDALONE_COMMENT` leaf in place of a run of
204+
sibling nodes. Doing that one node at a time with `Base.remove` /
205+
`insert_child` scans and shifts the parent's children list on every call, so
206+
converting the many sibling blocks of one parent (a long if/elif chain, a
207+
match with many cases, a module with many top-level statements) is O(n^2).
208+
Recording the conversions and rewriting each parent's children in a single
209+
pass makes it O(n).
210+
"""
211+
212+
def __init__(self) -> None:
213+
# id(parent) -> (parent, {id(child): standalone to splice in}, {ids to drop})
214+
# A run's nodes can live under more than one parent (an `async` statement
215+
# keeps the `ASYNC` leaf on the grandparent), so the standalone replaces
216+
# the run's first node under its own parent while the remaining nodes are
217+
# just dropped from wherever they sit.
218+
self._by_parent: dict[int, tuple[Node, dict[int, Leaf], set[int]]] = {}
219+
# NEWLINE leaves already inside a recorded run, so the line-by-line pass
220+
# doesn't record an overlapping conversion for them.
221+
self.covered_newlines: set[int] = set()
222+
# Nodes already scheduled for removal. When conversions immediately
223+
# mutated the tree, a second conversion touching an already-removed node
224+
# was a no-op (`Base.remove` returned None); recording defers the
225+
# mutation, so we skip such nodes explicitly to keep that behaviour.
226+
self._recorded: set[int] = set()
227+
# A leaf's prefix (indentation, blank lines, comments) moves onto its
228+
# STANDALONE_COMMENT. When an inner and an outer node that share the same
229+
# first leaf are both converted (a decorator inside a decorated block),
230+
# the earlier conversion clears the prefix off the leaf, so the later one
231+
# would see an empty prefix. Remember the taken prefix keyed by the leaf
232+
# so both conversions reuse the original.
233+
self._taken_prefixes: dict[int, str] = {}
234+
235+
def _entry(self, parent: Node) -> tuple[Node, dict[int, Leaf], set[int]]:
236+
return self._by_parent.setdefault(id(parent), (parent, {}, set()))
237+
238+
def take_prefix(self, first: Leaf) -> str:
239+
cached = self._taken_prefixes.get(id(first))
240+
if cached is not None:
241+
return cached
242+
prefix = first.prefix
243+
first.prefix = ""
244+
# Immediate conversion replaced the whole run with a freshly built
245+
# STANDALONE_COMMENT leaf, whose synthesized position is line 0. Recording
246+
# the conversion leaves the original leaf in place, so mirror that line 0
247+
# here: when a later, enclosing node shares this first leaf (a decorated
248+
# block over an already-recorded decorator), _get_line_range sees the run
249+
# as starting at line 0, matching the immediate behaviour.
250+
first.lineno = 0
251+
self._taken_prefixes[id(first)] = prefix
252+
return prefix
253+
254+
def is_recorded(self, node: LN) -> bool:
255+
return id(node) in self._recorded
256+
257+
def record(self, run: list[LN], standalone: Leaf) -> None:
258+
first = run[0]
259+
first_parent = first.parent
260+
if first_parent is None or id(first) in self._recorded:
261+
return
262+
for node in run:
263+
self._recorded.add(id(node))
264+
for leaf in node.leaves():
265+
if leaf.type == NEWLINE:
266+
self.covered_newlines.add(id(leaf))
267+
parent = node.parent
268+
if parent is not None:
269+
self._entry(parent)[2].add(id(node))
270+
self._entry(first_parent)[1][id(first)] = standalone
271+
272+
def apply(self) -> None:
273+
for parent, replaced, removed in self._by_parent.values():
274+
new_children: list[LN] = []
275+
for child in parent.children:
276+
standalone = replaced.get(id(child))
277+
if standalone is not None:
278+
standalone.parent = parent
279+
new_children.append(standalone)
280+
child.parent = None
281+
elif id(child) in removed:
282+
child.parent = None
283+
else:
284+
new_children.append(child)
285+
parent.children = new_children
286+
parent.changed()
287+
parent.invalidate_sibling_maps()
288+
289+
198290
def _contains_standalone_comment(node: LN) -> bool:
199291
if isinstance(node, Leaf):
200292
return node.type == STANDALONE_COMMENT
@@ -215,8 +307,9 @@ class _TopLevelStatementsVisitor(Visitor[None]):
215307
classes/functions/statements.
216308
"""
217309

218-
def __init__(self, lines_set: set[int]):
310+
def __init__(self, lines_set: set[int], replacements: "_NodeReplacements"):
219311
self._lines_set = lines_set
312+
self._replacements = replacements
220313

221314
def visit_simple_stmt(self, node: Node) -> Iterator[None]:
222315
# This is only called for top-level statements, since `visit_suite`
@@ -233,7 +326,7 @@ def visit_simple_stmt(self, node: Node) -> Iterator[None]:
233326
# its body on the same line. Example: `if cond: pass`.
234327
ancestor = furthest_ancestor_with_last_leaf(newline_leaf)
235328
if not _get_line_range(ancestor).intersection(self._lines_set):
236-
_convert_node_to_standalone_comment(ancestor)
329+
_convert_node_to_standalone_comment(ancestor, self._replacements)
237330

238331
def visit_suite(self, node: Node) -> Iterator[None]:
239332
yield from []
@@ -256,16 +349,21 @@ def visit_suite(self, node: Node) -> Iterator[None]:
256349
if semantic_parent is not None and not _get_line_range(
257350
semantic_parent
258351
).intersection(self._lines_set):
259-
_convert_node_to_standalone_comment(semantic_parent)
352+
_convert_node_to_standalone_comment(semantic_parent, self._replacements)
260353

261354

262355
def _convert_unchanged_line_by_line(node: Node, lines_set: set[int]) -> None:
263356
"""Converts unchanged to STANDALONE_COMMENT line by line."""
357+
replacements = _NodeReplacements()
264358
for leaf in node.leaves():
265359
if leaf.type != NEWLINE:
266360
# We only consider "unwrapped lines", which are divided by the NEWLINE
267361
# token.
268362
continue
363+
if id(leaf) in replacements.covered_newlines:
364+
# This NEWLINE is inside a run already scheduled for conversion (e.g.
365+
# a second decorator on a stacked decorator block).
366+
continue
269367
if leaf.parent and leaf.parent.type == syms.match_stmt:
270368
# The `suite` node is defined as:
271369
# match_stmt: "match" subject_expr ':' NEWLINE INDENT case_block+ DEDENT
@@ -277,7 +375,9 @@ def _convert_unchanged_line_by_line(node: Node, lines_set: set[int]) -> None:
277375
nodes_to_ignore.insert(0, prev_sibling)
278376
prev_sibling = prev_sibling.prev_sibling
279377
if not _get_line_range(nodes_to_ignore).intersection(lines_set):
280-
_convert_nodes_to_standalone_comment(nodes_to_ignore, newline=leaf)
378+
_convert_nodes_to_standalone_comment(
379+
nodes_to_ignore, newline=leaf, replacements=replacements
380+
)
281381
elif leaf.parent and leaf.parent.type == syms.suite:
282382
# The `suite` node is defined as:
283383
# suite: simple_stmt | NEWLINE INDENT stmt+ DEDENT
@@ -298,7 +398,9 @@ def _convert_unchanged_line_by_line(node: Node, lines_set: set[int]) -> None:
298398
):
299399
nodes_to_ignore.insert(0, grandparent.prev_sibling)
300400
if not _get_line_range(nodes_to_ignore).intersection(lines_set):
301-
_convert_nodes_to_standalone_comment(nodes_to_ignore, newline=leaf)
401+
_convert_nodes_to_standalone_comment(
402+
nodes_to_ignore, newline=leaf, replacements=replacements
403+
)
302404
else:
303405
ancestor = furthest_ancestor_with_last_leaf(leaf)
304406
# Consider multiple decorators as a whole block, as their
@@ -310,13 +412,16 @@ def _convert_unchanged_line_by_line(node: Node, lines_set: set[int]) -> None:
310412
):
311413
ancestor = ancestor.parent
312414
if not _get_line_range(ancestor).intersection(lines_set):
313-
_convert_node_to_standalone_comment(ancestor)
415+
_convert_node_to_standalone_comment(ancestor, replacements)
416+
replacements.apply()
314417

315418

316-
def _convert_node_to_standalone_comment(node: LN) -> None:
419+
def _convert_node_to_standalone_comment(
420+
node: LN, replacements: "_NodeReplacements"
421+
) -> None:
317422
"""Convert node to STANDALONE_COMMENT by modifying the tree inline."""
318423
parent = node.parent
319-
if not parent:
424+
if not parent or replacements.is_recorded(node):
320425
return
321426
first = first_leaf(node)
322427
last = last_leaf(node)
@@ -335,65 +440,53 @@ def _convert_node_to_standalone_comment(node: LN) -> None:
335440
# reformatted accordingly to the correct indentation level.
336441
# This also means the indentation will be changed on the unchanged lines, and
337442
# this is actually required to not break incremental reformatting.
338-
prefix = first.prefix
339-
first.prefix = ""
340-
index = node.remove()
341-
if index is not None:
342-
# Because of the special handling of multiple decorators, if the decorated
343-
# item is a single line then there will be a missing newline between the
344-
# decorator and item, so add it back. This doesn't affect any other case
345-
# since a decorated item with a newline would hit the earlier suite case
346-
# in _convert_unchanged_line_by_line that correctly handles the newlines.
347-
if node.type == syms.decorated:
348-
# A leaf of type decorated wouldn't make sense, since it should always
349-
# have at least the decorator + the decorated item, so if this assert
350-
# hits that means there's a problem in the parser.
351-
assert isinstance(node, Node)
352-
# 1 will always be the correct index since before this function is
353-
# called all the decorators are collapsed into a single leaf
354-
node.insert_child(1, Leaf(NEWLINE, "\n"))
355-
# Remove the '\n', as STANDALONE_COMMENT will have '\n' appended when
356-
# generating the formatted code.
357-
value = str(node)[:-1]
358-
parent.insert_child(
359-
index,
360-
Leaf(
361-
STANDALONE_COMMENT,
362-
value,
363-
prefix=prefix,
364-
fmt_pass_converted_first_leaf=first,
365-
),
366-
)
443+
prefix = replacements.take_prefix(first)
444+
# For a single-line decorated item the decorator and the item need a newline
445+
# between them. The conversions are recorded and applied in a single pass
446+
# instead of mutating the tree per node, so the decorator here is still its
447+
# own child node carrying its trailing NEWLINE, which already separates it
448+
# from the item; there's nothing to add back. (A decorated item spanning
449+
# multiple lines is handled by the earlier suite case in
450+
# _convert_unchanged_line_by_line, which manages the newlines itself.)
451+
# Remove the '\n', as STANDALONE_COMMENT will have '\n' appended when
452+
# generating the formatted code.
453+
value = str(node)[:-1]
454+
replacements.record(
455+
[node],
456+
Leaf(
457+
STANDALONE_COMMENT,
458+
value,
459+
prefix=prefix,
460+
fmt_pass_converted_first_leaf=first,
461+
),
462+
)
367463

368464

369-
def _convert_nodes_to_standalone_comment(nodes: Sequence[LN], *, newline: Leaf) -> None:
465+
def _convert_nodes_to_standalone_comment(
466+
nodes: Sequence[LN], *, newline: Leaf, replacements: "_NodeReplacements"
467+
) -> None:
370468
"""Convert nodes to STANDALONE_COMMENT by modifying the tree inline."""
371469
if not nodes:
372470
return
373471
parent = nodes[0].parent
374472
first = first_leaf(nodes[0])
375-
if not parent or not first:
473+
if not parent or not first or replacements.is_recorded(nodes[0]):
376474
return
377-
prefix = first.prefix
378-
first.prefix = ""
475+
prefix = replacements.take_prefix(first)
379476
value = "".join(str(node) for node in nodes)
380477
# The prefix comment on the NEWLINE leaf is the trailing comment of the statement.
381478
if newline.prefix:
382479
value += newline.prefix
383480
newline.prefix = ""
384-
index = nodes[0].remove()
385-
for node in nodes[1:]:
386-
node.remove()
387-
if index is not None:
388-
parent.insert_child(
389-
index,
390-
Leaf(
391-
STANDALONE_COMMENT,
392-
value,
393-
prefix=prefix,
394-
fmt_pass_converted_first_leaf=first,
395-
),
396-
)
481+
replacements.record(
482+
list(nodes),
483+
Leaf(
484+
STANDALONE_COMMENT,
485+
value,
486+
prefix=prefix,
487+
fmt_pass_converted_first_leaf=first,
488+
),
489+
)
397490

398491

399492
def _leaf_line_end(leaf: Leaf) -> int:

0 commit comments

Comments
 (0)