Skip to content

Commit e804713

Browse files
Python: Fix template apply losing indentation in nested contexts (#6844)
The previous fix (82d6e55) removed the pre_visit setup call in TabsAndIndentsVisitor.visit() to fix doubled indentation when auto_format was called on sub-trees. However, the root cause was that the test recipe was passing self.cursor (the cursor AT the node) instead of self.cursor.parent (the parent cursor). The visit(tree, p, parent) contract requires parent to be the cursor of the parent element — the visitor then creates Cursor(parent, tree) for the node being visited. The pre_visit setup call is needed to establish indent_type on the parent cursor so that child indentation is computed correctly. Restore the pre_visit setup and fix auto_format callers to pass the parent cursor. Add tests for template-based compound statement replacement inside function and class method bodies.
1 parent c32d382 commit e804713

3 files changed

Lines changed: 167 additions & 7 deletions

File tree

rewrite-python/rewrite/src/rewrite/python/format/tabs_and_indents_visitor.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ def visit(self, tree: Optional[Tree], p: P, parent: Optional[Cursor] = None) ->
6363
if indent != 0:
6464
c.put_message("last_indent", indent)
6565

66+
for next_parent in parent.get_path():
67+
if isinstance(next_parent, J):
68+
self.pre_visit(next_parent, p)
69+
break
70+
6671
return super().visit(tree, p)
6772

6873
def pre_visit(self, tree: T, p: P) -> Optional[T]:

rewrite-python/rewrite/tests/python/all/format/auto_format_subtree_test.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,14 @@
1414

1515
"""Tests for auto_format when applied to sub-trees with a cursor.
1616
17-
When a recipe calls auto_format(node, p, cursor=self.cursor) on a node
18-
that is not the root CompilationUnit, the formatter must preserve the
19-
node's own indentation level and only adjust its children. Previously,
20-
TabsAndIndentsVisitor.visit() called pre_visit() on the root element
21-
during setup, which set indent_type=INDENT on the cursor and caused an
22-
extra indentation level to be added to the node's own prefix.
17+
When a recipe calls auto_format(node, p, cursor=self.cursor.parent) on a
18+
node that is not the root CompilationUnit, the formatter must preserve the
19+
node's own indentation level and only adjust its children.
20+
21+
The cursor parameter to auto_format (and visit() in general) is the
22+
*parent* cursor. The visitor creates a child cursor for the tree being
23+
visited: Cursor(parent, tree). Passing self.cursor.parent provides the
24+
correct parent context for the node being formatted.
2325
"""
2426

2527
from typing import Any, Optional
@@ -60,7 +62,7 @@ def visit_if(
6062
if isinstance(if_stmt.then_part, Block):
6163
stmts = if_stmt.then_part.statements
6264
if len(stmts) == 1 and isinstance(stmts[0], py_tree.Pass):
63-
return auto_format(if_stmt, p, cursor=self.cursor)
65+
return auto_format(if_stmt, p, cursor=self.cursor.parent)
6466
return if_stmt
6567

6668
return Visitor()
Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
# Copyright 2025 the original author or authors.
2+
# <p>
3+
# Licensed under the Moderne Source Available License (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
# <p>
7+
# https://docs.moderne.io/licensing/moderne-source-available-license
8+
# <p>
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Tests for template apply preserving indentation in nested contexts.
16+
17+
When a template replaces a compound statement (e.g., try -> with) inside
18+
a function body, the replacement must inherit the original statement's
19+
indentation. The template engine's apply_coordinates() calls auto_format
20+
which must receive the correct cursor context to compute indentation.
21+
"""
22+
23+
from typing import Any, Optional
24+
25+
from rewrite import ExecutionContext, Recipe, TreeVisitor
26+
from rewrite.java import tree as j
27+
from rewrite.python import tree as py_tree
28+
from rewrite.python.template import template, capture
29+
from rewrite.python.visitor import PythonVisitor
30+
from rewrite.test import RecipeSpec, python
31+
32+
33+
# Template: replace a try/except:pass with a with-statement
34+
_exc = capture('exc')
35+
_body = capture('body')
36+
_with_template = template(
37+
'with suppress({exc}):\n {body}',
38+
exc=_exc,
39+
body=_body,
40+
)
41+
42+
43+
class _ReplaceTryWithWith(Recipe):
44+
"""Replaces try/except <Exc>: pass with: with suppress(<Exc>): <body>."""
45+
46+
@property
47+
def name(self) -> str:
48+
return "test.ReplaceTryWithWith"
49+
50+
@property
51+
def display_name(self) -> str:
52+
return "Test replace try with with-statement"
53+
54+
@property
55+
def description(self) -> str:
56+
return "Replaces single-except try/pass with a with-statement."
57+
58+
def editor(self) -> TreeVisitor[Any, ExecutionContext]:
59+
class Visitor(PythonVisitor[ExecutionContext]):
60+
def visit_try(
61+
self, try_stmt: j.Try, p: ExecutionContext
62+
) -> Optional[j.Try]:
63+
try_stmt = super().visit_try(try_stmt, p)
64+
65+
if len(try_stmt.catches) != 1:
66+
return try_stmt
67+
catch = try_stmt.catches[0]
68+
if len(catch.body.statements) != 1:
69+
return try_stmt
70+
if not isinstance(catch.body.statements[0], py_tree.Pass):
71+
return try_stmt
72+
73+
body_stmts = try_stmt.body.statements
74+
if len(body_stmts) != 1:
75+
return try_stmt
76+
77+
vd = catch.parameter.tree
78+
te = vd.type_expression
79+
if te is None:
80+
return try_stmt
81+
if isinstance(te, py_tree.ExceptionType):
82+
exc_expr = te.expression
83+
else:
84+
exc_expr = te
85+
86+
values = {
87+
'exc': exc_expr,
88+
'body': body_stmts[0],
89+
}
90+
return _with_template.apply(self.cursor, values=values)
91+
92+
return Visitor()
93+
94+
95+
def test_template_replace_try_at_top_level():
96+
"""Template replacement at top level preserves zero indentation."""
97+
spec = RecipeSpec(recipe=_ReplaceTryWithWith())
98+
spec.rewrite_run(
99+
python(
100+
"try:\n"
101+
" do_stuff()\n"
102+
"except KeyError:\n"
103+
" pass",
104+
#
105+
"with suppress(KeyError):\n"
106+
" do_stuff()",
107+
)
108+
)
109+
110+
111+
def test_template_replace_try_inside_function():
112+
"""Template replacement inside a function must preserve indentation.
113+
114+
The with-statement should be at column 4 (inside the function body),
115+
and its body at column 8.
116+
"""
117+
spec = RecipeSpec(recipe=_ReplaceTryWithWith())
118+
spec.rewrite_run(
119+
python(
120+
"def f():\n"
121+
" try:\n"
122+
" do_stuff()\n"
123+
" except KeyError:\n"
124+
" pass",
125+
#
126+
"def f():\n"
127+
" with suppress(KeyError):\n"
128+
" do_stuff()",
129+
)
130+
)
131+
132+
133+
def test_template_replace_try_inside_class_method():
134+
"""Template replacement inside a class method must preserve indentation.
135+
136+
The with-statement should be at column 8 (inside the method body).
137+
"""
138+
spec = RecipeSpec(recipe=_ReplaceTryWithWith())
139+
spec.rewrite_run(
140+
python(
141+
"class Foo:\n"
142+
" def bar(self):\n"
143+
" try:\n"
144+
" do_stuff()\n"
145+
" except KeyError:\n"
146+
" pass",
147+
#
148+
"class Foo:\n"
149+
" def bar(self):\n"
150+
" with suppress(KeyError):\n"
151+
" do_stuff()",
152+
)
153+
)

0 commit comments

Comments
 (0)