Skip to content

Commit f03fa74

Browse files
Python: Unwrap ExpressionStatement around block-level template placeholders (#6838)
When a template has a placeholder in statement position (e.g. the body of a `with` block), the parser wraps it as ExpressionStatement(Identifier). If the replacement is a non-Expression statement (return, if, for, etc.), the visitor must substitute it directly in the block before the default visitor runs, so visit_expression_statement never tries to cast the replacement to Expression. This mirrors the JS template engine's visitBlock logic.
1 parent 998e8c3 commit f03fa74

2 files changed

Lines changed: 72 additions & 0 deletions

File tree

rewrite-python/rewrite/src/rewrite/python/template/replacement.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,42 @@ def visit_identifier(self, ident: j.Identifier, p: None) -> J:
193193
# Not a placeholder or no value provided, continue normally
194194
return super().visit_identifier(ident, p)
195195

196+
def visit_block(self, block: j.Block, p: None) -> J:
197+
"""Override visit_block to unwrap ExpressionStatement around placeholders.
198+
199+
When a template has a placeholder in statement position (e.g., the body
200+
of a ``with`` block), the parser wraps it as ``ExpressionStatement(Identifier(...))``
201+
since it looks like a bare expression. If the replacement value is a
202+
non-Expression statement (``return``, ``if``, ``for``, etc.), we must
203+
substitute it directly into the statements list, bypassing the
204+
``ExpressionStatement`` wrapper. This mirrors the JS template engine's
205+
``visitBlock`` logic.
206+
"""
207+
# Substitute statement-position placeholders BEFORE the default
208+
# visitor runs, so visit_expression_statement never sees them.
209+
new_stmts: List[JRightPadded] = []
210+
changed = False
211+
for rp in block.padding.statements:
212+
stmt = rp.element
213+
if isinstance(stmt, py.ExpressionStatement):
214+
expr = stmt.expression
215+
if isinstance(expr, j.Identifier):
216+
capture_name = from_placeholder(expr.simple_name)
217+
if capture_name is not None and capture_name in self._values:
218+
replacement = self._values[capture_name]
219+
# Preserve the placeholder's whitespace prefix
220+
if hasattr(replacement, '_prefix'):
221+
replacement = replacement.replace(_prefix=expr.prefix)
222+
new_stmts.append(rp.replace(element=replacement))
223+
changed = True
224+
continue
225+
new_stmts.append(rp)
226+
227+
if changed:
228+
block = block.padding.replace(statements=new_stmts)
229+
230+
return super().visit_block(block, p)
231+
196232
def visit_binary(self, binary: j.Binary, p: None) -> J:
197233
"""Visit a Java Binary and auto-parenthesize substituted operands if needed."""
198234
binary = super().visit_binary(binary, p)

rewrite-python/rewrite/tests/python/template/test_replacement.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,3 +341,39 @@ def test_comparison_under_not_no_parens(self):
341341
assert isinstance(result, j.Unary)
342342
# Comparisons have precedence 4 which is >= 3 (not threshold), so no parens
343343
assert isinstance(result.expression, j.Binary)
344+
345+
346+
class TestBlockStatementPlaceholder:
347+
"""Tests for placeholder replacement in statement position inside blocks.
348+
349+
When a template has {body} inside a block (e.g. ``if True:\\n {body}``),
350+
the parser wraps it as ``ExpressionStatement(Identifier(placeholder))``.
351+
If the replacement is a non-Expression statement (Return, If, etc.),
352+
the visitor must unwrap the ExpressionStatement so the block directly
353+
contains the replacement statement.
354+
"""
355+
356+
def setup_method(self):
357+
TemplateEngine.clear_cache()
358+
359+
def test_return_in_block_placeholder(self):
360+
"""Substituting a Return for a block placeholder should unwrap ExpressionStatement."""
361+
tree = TemplateEngine.get_template_tree(
362+
"if True:\n {body}", {'body': capture('body')}
363+
)
364+
return_stmt = j.Return(
365+
uuid4(), Space([], ' '), Markers.EMPTY, _ident('result')
366+
)
367+
visitor = PlaceholderReplacementVisitor({'body': return_stmt})
368+
result = visitor.visit(tree, None)
369+
370+
# The result should be an If statement
371+
assert isinstance(result, j.If)
372+
# The then-part block should contain a Return, NOT an ExpressionStatement
373+
then_block = result.then_part
374+
assert isinstance(then_block, j.Block)
375+
stmts = then_block.statements
376+
assert len(stmts) == 1
377+
assert isinstance(stmts[0], j.Return), (
378+
f"Expected Return in block, got {type(stmts[0]).__name__}"
379+
)

0 commit comments

Comments
 (0)