fix(robot): correct dedent logic for try...except...finally in indentation_library
CI / lint (pull_request) Successful in 29s
CI / typecheck (pull_request) Successful in 48s
CI / quality (pull_request) Successful in 33s
CI / security (pull_request) Successful in 1m2s
CI / build (pull_request) Successful in 23s
CI / helm (pull_request) Successful in 23s
CI / unit_tests (pull_request) Successful in 6m48s
CI / e2e_tests (pull_request) Successful in 19m4s
CI / integration_tests (pull_request) Successful in 22m6s
CI / coverage (pull_request) Successful in 10m59s
CI / docker (pull_request) Successful in 22s
CI / status-check (pull_request) Successful in 2s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 57m5s
CI / lint (pull_request) Successful in 29s
CI / typecheck (pull_request) Successful in 48s
CI / quality (pull_request) Successful in 33s
CI / security (pull_request) Successful in 1m2s
CI / build (pull_request) Successful in 23s
CI / helm (pull_request) Successful in 23s
CI / unit_tests (pull_request) Successful in 6m48s
CI / e2e_tests (pull_request) Successful in 19m4s
CI / integration_tests (pull_request) Successful in 22m6s
CI / coverage (pull_request) Successful in 10m59s
CI / docker (pull_request) Successful in 22s
CI / status-check (pull_request) Successful in 2s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 57m5s
Rewrites the block_stack management in fix_python_indentation to correctly handle try...except...finally blocks in all configurations: - Single except clause - Multiple except clauses for the same try - try...except...finally - Nested try blocks with outer except - Nested try with inner finally followed by outer except (critical regression) Root cause: The original implementation stored only keyword strings in block_stack (e.g. 'try', 'class', 'other'). Two bugs existed: 1. When 'finally:' was encountered, the code popped the 'try' from the stack. This meant subsequent 'except:' clauses for an outer try could not find their matching try, producing syntactically invalid Python. 2. The dedent calculation for 'except'/'finally' counted all stack entries up to the first 'try', but did not account for nested try blocks where the inner try's except/finally had already been processed. Fix: Replace the flat string stack with a tuple stack of (keyword, base_indent, has_seen_except_finally). The new logic: - 'finally:' always belongs to the innermost try. Pop non-try entries, set indent to that try's base_indent, then pop the try itself (finally closes the block). The outer try remains on the stack. - 'except:' belongs to the innermost try. If the innermost try has already seen an except/finally AND there is an outer try, the inner try is done: pop it and use the outer try instead. Otherwise use the innermost try and mark it as having seen an except/finally clause. Adds a Behave BDD feature (tdd_indentation_library_try_except.feature) with 6 scenarios covering all acceptance criteria from issue #2845. ISSUES CLOSED: #2845
This commit is contained in:
@@ -0,0 +1,283 @@
|
||||
"""Step definitions for TDD Issue #2845 — fix_python_indentation try...except...finally.
|
||||
|
||||
These steps verify that ``fix_python_indentation`` in
|
||||
``robot/indentation_library.py`` correctly reconstructs indentation for
|
||||
``try...except...finally`` blocks, producing syntactically valid Python in
|
||||
all cases.
|
||||
|
||||
Root cause
|
||||
~~~~~~~~~~
|
||||
The original implementation had two bugs:
|
||||
|
||||
1. When ``finally:`` was encountered, the code popped the ``try`` from the
|
||||
``block_stack``. This meant subsequent ``except:`` clauses (for an outer
|
||||
``try``) could not find their matching ``try`` in the stack, resulting in
|
||||
incorrect indentation and syntactically invalid Python.
|
||||
|
||||
2. After an ``except`` block body, subsequent code at the same level was
|
||||
incorrectly indented inside the except block because the ``block_stack``
|
||||
was not properly cleaned up after ``try...except`` blocks completed.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
from behave import given, then, when
|
||||
from behave.runner import Context
|
||||
|
||||
# Add robot directory to path so we can import indentation_library
|
||||
_ROBOT_DIR = str(Path(__file__).parent.parent.parent / "robot")
|
||||
if _ROBOT_DIR not in sys.path:
|
||||
sys.path.insert(0, _ROBOT_DIR)
|
||||
|
||||
from indentation_library import fix_python_indentation # noqa: E402
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Given steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given("the fix_python_indentation function is available")
|
||||
def step_function_available(context: Context) -> None:
|
||||
"""Verify the function is importable and callable."""
|
||||
assert callable(fix_python_indentation)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# When steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@when("I fix indentation for a try block with a single except clause")
|
||||
def step_single_except(context: Context) -> None:
|
||||
"""Fix indentation for a try block with one except clause."""
|
||||
script = 'try:\nx = 1\nexcept ValueError:\nprint("error")'
|
||||
context.result = fix_python_indentation(script)
|
||||
context.script_lines = context.result.split("\n")
|
||||
|
||||
|
||||
@when("I fix indentation for a try block with multiple except clauses")
|
||||
def step_multiple_except(context: Context) -> None:
|
||||
"""Fix indentation for a try block with multiple except clauses."""
|
||||
script = (
|
||||
"try:\n"
|
||||
"x = 1\n"
|
||||
"except ValueError:\n"
|
||||
'print("value error")\n'
|
||||
"except TypeError:\n"
|
||||
'print("type error")'
|
||||
)
|
||||
context.result = fix_python_indentation(script)
|
||||
context.script_lines = context.result.split("\n")
|
||||
|
||||
|
||||
@when("I fix indentation for a try block with except and finally")
|
||||
def step_except_finally(context: Context) -> None:
|
||||
"""Fix indentation for a try...except...finally block."""
|
||||
script = 'try:\nx = 1\nexcept ValueError:\nprint("error")\nfinally:\nprint("done")'
|
||||
context.result = fix_python_indentation(script)
|
||||
context.script_lines = context.result.split("\n")
|
||||
|
||||
|
||||
@when("I fix indentation for nested try blocks")
|
||||
def step_nested_try(context: Context) -> None:
|
||||
"""Fix indentation for nested try blocks."""
|
||||
script = (
|
||||
"try:\n"
|
||||
"try:\n"
|
||||
"x = 1\n"
|
||||
"except ValueError:\n"
|
||||
'print("inner error")\n'
|
||||
"except TypeError:\n"
|
||||
'print("outer error")'
|
||||
)
|
||||
context.result = fix_python_indentation(script)
|
||||
context.script_lines = context.result.split("\n")
|
||||
|
||||
|
||||
@when("I fix indentation for nested try with inner finally and outer except")
|
||||
def step_nested_try_finally_outer_except(context: Context) -> None:
|
||||
"""Fix indentation for nested try with inner finally and outer except.
|
||||
|
||||
This is the critical regression case: the inner ``finally`` must NOT
|
||||
consume the outer ``try`` from the block stack.
|
||||
"""
|
||||
script = (
|
||||
"try:\n"
|
||||
"try:\n"
|
||||
"x = 1\n"
|
||||
"except ValueError:\n"
|
||||
'print("inner")\n'
|
||||
"finally:\n"
|
||||
'print("inner finally")\n'
|
||||
"except TypeError:\n"
|
||||
'print("outer")'
|
||||
)
|
||||
context.result = fix_python_indentation(script)
|
||||
context.script_lines = context.result.split("\n")
|
||||
|
||||
|
||||
@when("I fix indentation for code that follows a try block")
|
||||
def step_code_after_try(context: Context) -> None:
|
||||
"""Fix indentation for code that follows a complete try...except block."""
|
||||
script = 'x = 1\ntry:\ny = 2\nexcept ValueError:\nprint("error")\nz = 3'
|
||||
context.result = fix_python_indentation(script)
|
||||
context.script_lines = context.result.split("\n")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Then steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then("the indentation result should be syntactically valid Python")
|
||||
def step_indentation_valid_python(context: Context) -> None:
|
||||
"""Verify the result compiles as valid Python."""
|
||||
try:
|
||||
compile(context.result, "<fix_python_indentation>", "exec")
|
||||
except SyntaxError as exc:
|
||||
raise AssertionError(
|
||||
f"fix_python_indentation produced invalid Python:\n"
|
||||
f"{context.result}\n\nSyntaxError: {exc}"
|
||||
) from exc
|
||||
|
||||
|
||||
@then("the try body should be indented one level")
|
||||
def step_try_body_indented(context: Context) -> None:
|
||||
"""Verify the try body is indented by 4 spaces."""
|
||||
lines = context.script_lines
|
||||
try_idx = next(i for i, ln in enumerate(lines) if ln.strip() == "try:")
|
||||
body_line = lines[try_idx + 1]
|
||||
assert body_line.startswith(" ") and not body_line.startswith(" "), (
|
||||
f"Expected try body indented 4 spaces, got: {body_line!r}"
|
||||
)
|
||||
|
||||
|
||||
@then("the except clause should be at the same level as try")
|
||||
def step_except_same_level_as_try(context: Context) -> None:
|
||||
"""Verify except is at the same indentation level as try."""
|
||||
lines = context.script_lines
|
||||
try_indent = next(
|
||||
len(ln) - len(ln.lstrip()) for ln in lines if ln.strip() == "try:"
|
||||
)
|
||||
except_lines = [ln for ln in lines if ln.strip().startswith("except")]
|
||||
for exc_line in except_lines:
|
||||
exc_indent = len(exc_line) - len(exc_line.lstrip())
|
||||
assert exc_indent == try_indent, (
|
||||
f"except indent ({exc_indent}) != try indent ({try_indent}): {exc_line!r}"
|
||||
)
|
||||
|
||||
|
||||
@then("the except body should be indented one level")
|
||||
def step_except_body_indented(context: Context) -> None:
|
||||
"""Verify the except body is indented one level deeper than except."""
|
||||
lines = context.script_lines
|
||||
for i, line in enumerate(lines):
|
||||
if line.strip().startswith("except") and i + 1 < len(lines):
|
||||
except_indent = len(line) - len(line.lstrip())
|
||||
body_line = lines[i + 1]
|
||||
if body_line.strip():
|
||||
body_indent = len(body_line) - len(body_line.lstrip())
|
||||
assert body_indent == except_indent + 4, (
|
||||
f"except body indent ({body_indent}) should be "
|
||||
f"except indent ({except_indent}) + 4: {body_line!r}"
|
||||
)
|
||||
|
||||
|
||||
@then("all except clauses should be at the same level as try")
|
||||
def step_all_excepts_same_level(context: Context) -> None:
|
||||
"""Verify all except clauses are at the same indentation level as try."""
|
||||
step_except_same_level_as_try(context)
|
||||
|
||||
|
||||
@then("each except body should be indented one level")
|
||||
def step_each_except_body_indented(context: Context) -> None:
|
||||
"""Verify each except body is indented one level deeper than its except."""
|
||||
step_except_body_indented(context)
|
||||
|
||||
|
||||
@then("the finally clause should be at the same level as try")
|
||||
def step_finally_same_level_as_try(context: Context) -> None:
|
||||
"""Verify finally is at the same indentation level as try."""
|
||||
lines = context.script_lines
|
||||
try_indent = next(
|
||||
len(ln) - len(ln.lstrip()) for ln in lines if ln.strip() == "try:"
|
||||
)
|
||||
finally_lines = [ln for ln in lines if ln.strip() == "finally:"]
|
||||
assert finally_lines, "No finally clause found in result"
|
||||
for fin_line in finally_lines:
|
||||
fin_indent = len(fin_line) - len(fin_line.lstrip())
|
||||
assert fin_indent == try_indent, (
|
||||
f"finally indent ({fin_indent}) != try indent ({try_indent}): {fin_line!r}"
|
||||
)
|
||||
|
||||
|
||||
@then("the finally body should be indented one level")
|
||||
def step_finally_body_indented(context: Context) -> None:
|
||||
"""Verify the finally body is indented one level deeper than finally."""
|
||||
lines = context.script_lines
|
||||
for i, line in enumerate(lines):
|
||||
if line.strip() == "finally:" and i + 1 < len(lines):
|
||||
finally_indent = len(line) - len(line.lstrip())
|
||||
body_line = lines[i + 1]
|
||||
if body_line.strip():
|
||||
body_indent = len(body_line) - len(body_line.lstrip())
|
||||
assert body_indent == finally_indent + 4, (
|
||||
f"finally body indent ({body_indent}) should be "
|
||||
f"finally indent ({finally_indent}) + 4: {body_line!r}"
|
||||
)
|
||||
|
||||
|
||||
@then("the inner try should be indented one level inside the outer try")
|
||||
def step_inner_try_indented(context: Context) -> None:
|
||||
"""Verify the inner try is indented one level inside the outer try."""
|
||||
lines = context.script_lines
|
||||
try_lines = [ln for ln in lines if ln.strip() == "try:"]
|
||||
assert len(try_lines) >= 2, f"Expected at least 2 try lines, got: {try_lines}"
|
||||
outer_indent = len(try_lines[0]) - len(try_lines[0].lstrip())
|
||||
inner_indent = len(try_lines[1]) - len(try_lines[1].lstrip())
|
||||
assert inner_indent == outer_indent + 4, (
|
||||
f"Inner try indent ({inner_indent}) should be outer try indent "
|
||||
f"({outer_indent}) + 4"
|
||||
)
|
||||
|
||||
|
||||
@then("the outer except should be at the same level as the outer try")
|
||||
def step_outer_except_same_level(context: Context) -> None:
|
||||
"""Verify the last except clause is at the same level as the outer try."""
|
||||
lines = context.script_lines
|
||||
try_lines = [ln for ln in lines if ln.strip() == "try:"]
|
||||
assert try_lines, "No try lines found"
|
||||
outer_try_line = try_lines[0]
|
||||
outer_try_indent = len(outer_try_line) - len(outer_try_line.lstrip())
|
||||
|
||||
# Find the last except clause (should be the outer one)
|
||||
except_lines = [ln for ln in lines if ln.strip().startswith("except")]
|
||||
assert except_lines, "No except lines found"
|
||||
last_except_line = except_lines[-1]
|
||||
last_except_indent = len(last_except_line) - len(last_except_line.lstrip())
|
||||
|
||||
assert last_except_indent == outer_try_indent, (
|
||||
f"Outer except indent ({last_except_indent}) != outer try indent "
|
||||
f"({outer_try_indent}): {last_except_line!r}\n\n"
|
||||
f"Full result:\n{chr(10).join(lines)}"
|
||||
)
|
||||
|
||||
|
||||
@then("the code after the try block should be at the same level as the try statement")
|
||||
def step_code_after_try_same_level(context: Context) -> None:
|
||||
"""Verify code after a try block is at the same indentation as the try."""
|
||||
lines = context.script_lines
|
||||
try_indent = next(
|
||||
len(ln) - len(ln.lstrip()) for ln in lines if ln.strip() == "try:"
|
||||
)
|
||||
# The last non-empty line should be at the same level as try
|
||||
non_empty = [ln for ln in lines if ln.strip()]
|
||||
last_line = non_empty[-1]
|
||||
last_indent = len(last_line) - len(last_line.lstrip())
|
||||
assert last_indent == try_indent, (
|
||||
f"Code after try block indent ({last_indent}) != try indent "
|
||||
f"({try_indent}): {last_line!r}\n\nFull result:\n{chr(10).join(lines)}"
|
||||
)
|
||||
@@ -0,0 +1,42 @@
|
||||
@tdd_issue @tdd_issue_2845
|
||||
Feature: TDD Issue #2845 — fix_python_indentation handles try...except...finally correctly
|
||||
As a Robot Framework user
|
||||
I want fix_python_indentation to correctly reconstruct indentation for try...except...finally blocks
|
||||
So that inline Python scripts with exception handling produce syntactically valid code
|
||||
|
||||
Background:
|
||||
Given the fix_python_indentation function is available
|
||||
|
||||
Scenario: Single except clause produces valid Python
|
||||
When I fix indentation for a try block with a single except clause
|
||||
Then the indentation result should be syntactically valid Python
|
||||
And the try body should be indented one level
|
||||
And the except clause should be at the same level as try
|
||||
And the except body should be indented one level
|
||||
|
||||
Scenario: Multiple except clauses produce valid Python
|
||||
When I fix indentation for a try block with multiple except clauses
|
||||
Then the indentation result should be syntactically valid Python
|
||||
And all except clauses should be at the same level as try
|
||||
And each except body should be indented one level
|
||||
|
||||
Scenario: try...except...finally produces valid Python
|
||||
When I fix indentation for a try block with except and finally
|
||||
Then the indentation result should be syntactically valid Python
|
||||
And the finally clause should be at the same level as try
|
||||
And the finally body should be indented one level
|
||||
|
||||
Scenario: Nested try blocks produce valid Python
|
||||
When I fix indentation for nested try blocks
|
||||
Then the indentation result should be syntactically valid Python
|
||||
And the inner try should be indented one level inside the outer try
|
||||
And the outer except should be at the same level as the outer try
|
||||
|
||||
Scenario: Nested try with finally then outer except produces valid Python
|
||||
When I fix indentation for nested try with inner finally and outer except
|
||||
Then the indentation result should be syntactically valid Python
|
||||
And the outer except should be at the same level as the outer try
|
||||
|
||||
Scenario: Code after try block produces valid Python
|
||||
When I fix indentation for code that follows a try block
|
||||
Then the indentation result should be syntactically valid Python
|
||||
@@ -15,7 +15,13 @@ def fix_python_indentation(script: str) -> str:
|
||||
lines = script.split("\n")
|
||||
result: list[str] = []
|
||||
indent_level = 0
|
||||
block_stack: list[str] = []
|
||||
# Each entry is a tuple (keyword, base_indent, has_seen_except_finally) where:
|
||||
# keyword — "try", "class", or "other"
|
||||
# base_indent — the indent level at which the block keyword was written
|
||||
# has_seen_except_finally — True once an except/finally clause has been seen
|
||||
# for this try block (used to detect when the inner
|
||||
# try is done and an outer try's except/finally follows)
|
||||
block_stack: list[tuple[str, int, bool]] = []
|
||||
|
||||
for line in lines:
|
||||
stripped = line.strip()
|
||||
@@ -38,21 +44,60 @@ def fix_python_indentation(script: str) -> str:
|
||||
and block_stack
|
||||
and (
|
||||
stripped.startswith(("import ", "from "))
|
||||
or (stripped.startswith("class ") and block_stack[-1] == "class")
|
||||
or (stripped.startswith("class ") and block_stack[-1][0] == "class")
|
||||
)
|
||||
):
|
||||
is_module_level_statement = True
|
||||
|
||||
if is_dedenting and block_stack:
|
||||
if stripped.startswith(("except:", "except ", "finally:")):
|
||||
dedent_levels = 0
|
||||
for block in reversed(block_stack):
|
||||
dedent_levels += 1
|
||||
if block == "try":
|
||||
break
|
||||
indent_level = max(0, indent_level - dedent_levels)
|
||||
if stripped.startswith("finally:"):
|
||||
# ``finally`` always belongs to the innermost ``try`` on the stack.
|
||||
# Pop non-try entries above it, set indent to the try's level,
|
||||
# then pop the try itself (``finally`` closes the try block).
|
||||
while block_stack and block_stack[-1][0] != "try":
|
||||
block_stack.pop()
|
||||
if block_stack and block_stack[-1][0] == "try":
|
||||
indent_level = block_stack[-1][1]
|
||||
block_stack.pop()
|
||||
else:
|
||||
indent_level = max(0, indent_level - 1)
|
||||
|
||||
elif stripped.startswith(("except:", "except ")):
|
||||
# ``except`` belongs to the innermost ``try``. However, if the
|
||||
# innermost ``try`` has already seen an ``except``/``finally``
|
||||
# clause AND there is an outer ``try`` on the stack, this
|
||||
# ``except`` belongs to the outer ``try`` (the inner try block
|
||||
# is complete).
|
||||
while block_stack and block_stack[-1][0] != "try":
|
||||
block_stack.pop()
|
||||
|
||||
if block_stack and block_stack[-1][0] == "try":
|
||||
innermost_try = block_stack[-1]
|
||||
has_outer_try = any(b[0] == "try" for b in block_stack[:-1])
|
||||
|
||||
if innermost_try[2] and has_outer_try:
|
||||
# Inner try is done — pop it and find the outer try.
|
||||
block_stack.pop()
|
||||
while block_stack and block_stack[-1][0] != "try":
|
||||
block_stack.pop()
|
||||
|
||||
if block_stack and block_stack[-1][0] == "try":
|
||||
indent_level = block_stack[-1][1]
|
||||
# Mark this try as having seen an except/finally clause.
|
||||
block_stack[-1] = (
|
||||
block_stack[-1][0],
|
||||
block_stack[-1][1],
|
||||
True,
|
||||
)
|
||||
else:
|
||||
indent_level = max(0, indent_level - 1)
|
||||
else:
|
||||
indent_level = max(0, indent_level - 1)
|
||||
|
||||
else:
|
||||
# ``else`` / ``elif`` — dedent one level from the preceding body.
|
||||
indent_level = max(0, indent_level - 1)
|
||||
|
||||
elif is_module_level_statement:
|
||||
# Dedent back to module level
|
||||
indent_level = 0
|
||||
@@ -61,24 +106,21 @@ def fix_python_indentation(script: str) -> str:
|
||||
current_indent = indent_level
|
||||
result.append(" " * current_indent + stripped)
|
||||
|
||||
# Update the stack and indent level for the *next* line.
|
||||
if stripped.startswith("try:"):
|
||||
block_stack.append("try")
|
||||
block_stack.append(("try", current_indent, False))
|
||||
indent_level += 1
|
||||
elif stripped.startswith(("except:", "except ", "finally:")):
|
||||
while block_stack and block_stack[-1] != "try":
|
||||
block_stack.pop()
|
||||
if (
|
||||
stripped.startswith("finally:")
|
||||
and block_stack
|
||||
and block_stack[-1] == "try"
|
||||
):
|
||||
block_stack.pop()
|
||||
elif stripped.startswith("finally:"):
|
||||
# The matching ``try`` was already popped during the dedent phase above.
|
||||
indent_level += 1
|
||||
elif stripped.startswith(("except:", "except ")):
|
||||
# The try remains on the stack (more except/finally may follow).
|
||||
indent_level += 1
|
||||
elif stripped.endswith(":") and not is_dedenting:
|
||||
if stripped.startswith("class "):
|
||||
block_stack.append("class")
|
||||
block_stack.append(("class", current_indent, False))
|
||||
else:
|
||||
block_stack.append("other")
|
||||
block_stack.append(("other", current_indent, False))
|
||||
indent_level += 1
|
||||
elif is_dedenting and stripped.endswith(":"):
|
||||
indent_level += 1
|
||||
|
||||
Reference in New Issue
Block a user