diff --git a/features/steps/tdd_indentation_library_try_except_steps.py b/features/steps/tdd_indentation_library_try_except_steps.py new file mode 100644 index 000000000..af6830e00 --- /dev/null +++ b/features/steps/tdd_indentation_library_try_except_steps.py @@ -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, "", "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)}" + ) diff --git a/features/tdd_indentation_library_try_except.feature b/features/tdd_indentation_library_try_except.feature new file mode 100644 index 000000000..057109069 --- /dev/null +++ b/features/tdd_indentation_library_try_except.feature @@ -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 diff --git a/robot/indentation_library.py b/robot/indentation_library.py index e229a2086..a6815a6af 100644 --- a/robot/indentation_library.py +++ b/robot/indentation_library.py @@ -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