diff --git a/scripts/audit_exception_handling.py b/scripts/audit_exception_handling.py index e47d8216..4c536e79 100644 --- a/scripts/audit_exception_handling.py +++ b/scripts/audit_exception_handling.py @@ -373,6 +373,16 @@ class ExceptionVisitor(ast.NodeVisitor): # ----- Classification logic ----- + # 0. Heuristic A: Result-returning recovery — the canonical data-oriented pattern. + # If the except body returns `Result(data=..., errors=[ErrorInfo(...)])`, + # the function is following the convention. Classify as INTERNAL_COMPLIANT + # BEFORE the BOUNDARY_CONVERSION check (which also fires for ErrorInfo creation). + if self._returns_result(body): + return ( + "INTERNAL_COMPLIANT", + "Compliant: `try: ...; except: return Result(data=..., errors=[...])` is the canonical Result-recovery pattern. The convention requires Result[T] for try/except sites that can fail; this pattern satisfies the requirement. The function-name-not-ending-in-`_result` is a smell (rename to `xxx_result`); the pattern itself is compliant. (per result_migration_small_files_20260617 Phase 11.2, Heuristic A)", + ) + # 1. ErrorInfo conversion = canonical boundary pattern if creates_errorinfo: return ( @@ -591,6 +601,13 @@ class ExceptionVisitor(ast.NodeVisitor): f"Compliant: `try: ...; except Exception: return ` in a `-> str` tool function is the canonical MCP tool boundary pattern (per result_migration_review_pass_20260617).", ) + # A. Result-returning recovery (canonical Result pattern) — Phase 11.2 + if len(except_body) > 0 and self._returns_result(except_body): + return ( + "INTERNAL_COMPLIANT", + f"Compliant: `try: ...; except ({', '.join(sorted(exc_set))}): return Result(data=..., errors=[...])` is the canonical Result-recovery pattern. The function-name-not-ending-in-`_result` is a smell (rename to `xxx_result`); the pattern itself is the data-oriented convention. (per result_migration_small_files_20260617 Phase 11.2)", + ) + return None def _has_string_return(self, stmts: list[ast.stmt]) -> bool: @@ -610,6 +627,30 @@ class ExceptionVisitor(ast.NodeVisitor): return True return False + def _returns_result(self, stmts: list[ast.stmt]) -> bool: + """True if the body returns a `Result(...)` call (canonical Result-recovery pattern). + + Detects `return Result(data=..., errors=[...])` — the canonical + data-oriented error handling pattern. Matches any call to `Result(...)` + with at least a `data=` keyword argument. The pattern is compliant + when used in a try/except: it satisfies the convention that every + try/except site that can fail must return `Result[T]` with structured + `ErrorInfo`. The function-name-not-ending-in-`_result` is a smell + (the function should be renamed to `xxx_result`), but the pattern + itself is compliant (heuristic A from Phase 11.2). + """ + for s in stmts: + if not isinstance(s, ast.Return) or s.value is None: + continue + if not isinstance(s.value, ast.Call): + continue + f = s.value.func + if isinstance(f, ast.Name) and f.id == "Result": + return True + if isinstance(f, ast.Attribute) and f.attr == "Result": + return True + return False + def _uses_exception_inline(self, stmts: list[ast.stmt]) -> bool: """True if the body uses `e`/`exc` in a non-pass way (Name reference).""" for s in stmts: diff --git a/scripts/tier2/artifacts/result_migration_small_files_20260617/add_helper.py b/scripts/tier2/artifacts/result_migration_small_files_20260617/add_helper.py new file mode 100644 index 00000000..261981b3 --- /dev/null +++ b/scripts/tier2/artifacts/result_migration_small_files_20260617/add_helper.py @@ -0,0 +1,48 @@ +"""Add _returns_result helper to audit_exception_handling.py.""" +from __future__ import annotations +from pathlib import Path + +p = Path("scripts/audit_exception_handling.py") +content = p.read_text(encoding="utf-8") + +needle = " def _has_simple_return(self, stmts: list[ast.stmt]) -> bool:\n \"\"\"True if the body contains a `return ` statement (any value type).\"\"\"\n for s in stmts:\n if isinstance(s, ast.Return) and s.value is not None:\n return True\n return False\n\n def _uses_exception_inline(self, stmts: list[ast.stmt]) -> bool:" + +replacement = """ def _has_simple_return(self, stmts: list[ast.stmt]) -> bool: + \"\"\"True if the body contains a `return ` statement (any value type).\"\"\" + for s in stmts: + if isinstance(s, ast.Return) and s.value is not None: + return True + return False + + def _returns_result(self, stmts: list[ast.stmt]) -> bool: + \"\"\"True if the body returns a `Result(...)` call (canonical Result-recovery pattern). + + Detects `return Result(data=..., errors=[...])` — the canonical + data-oriented error handling pattern. Matches any call to `Result(...)` + with at least a `data=` keyword argument. The pattern is compliant + when used in a try/except: it satisfies the convention that every + try/except site that can fail must return `Result[T]` with structured + `ErrorInfo`. The function-name-not-ending-in-`_result` is a smell + (the function should be renamed to `xxx_result`), but the pattern + itself is compliant (heuristic A from Phase 11.2). + \"\"\" + for s in stmts: + if not isinstance(s, ast.Return) or s.value is None: + continue + if not isinstance(s.value, ast.Call): + continue + f = s.value.func + if isinstance(f, ast.Name) and f.id == "Result": + return True + if isinstance(f, ast.Attribute) and f.attr == "Result": + return True + return False + + def _uses_exception_inline(self, stmts: list[ast.stmt]) -> bool:""" + +if needle not in content: + print("ERROR: needle not found") + raise SystemExit(1) +content = content.replace(needle, replacement) +p.write_text(content, encoding="utf-8", newline="") +print("ok") diff --git a/scripts/tier2/artifacts/result_migration_small_files_20260617/append_heuristic_a_test.py b/scripts/tier2/artifacts/result_migration_small_files_20260617/append_heuristic_a_test.py new file mode 100644 index 00000000..04bf5782 --- /dev/null +++ b/scripts/tier2/artifacts/result_migration_small_files_20260617/append_heuristic_a_test.py @@ -0,0 +1,71 @@ +"""Append 2 failing tests for Heuristic A (Result-returning recovery).""" +from __future__ import annotations +from pathlib import Path + +p = Path("tests/test_audit_exception_handling_heuristics.py") +with p.open("r", encoding="utf-8", newline="") as f: + content = f.read() + +append = ''' + +# --------------------------------------------------------------------------- +# Heuristic A: Result-returning recovery in non-*_result function (Phase 11.2) +# --------------------------------------------------------------------------- +def test_result_returning_recovery_in_non_result_named_function_is_compliant(): + """try: ...; except SpecificError: return Result(data=..., errors=[ErrorInfo(...)]) is compliant. + + The function returns a Result with errors= on failure (the canonical Result + recovery pattern). The convention requires Result[T] for try/except sites + that can fail; this pattern satisfies the requirement. The function name + not ending in '_result' is a smell (the function should be renamed to + 'xxx_result') but the pattern itself is compliant. + This is the pattern used by src/hot_reloader.py:reload(), + src/warmup.py:on_complete/_record_success/_record_failure, and the + other 17 sites migrated in Phase 11.3. + """ + src = \\'\\'\\' + from src.result_types import Result, ErrorInfo, ErrorKind + + def reload(module_name): + try: + importlib.reload(sys.modules[module_name]) + return Result(data=True) + except (ImportError, ModuleNotFoundError) as e: + return Result(data=False, errors=[ErrorInfo(kind=ErrorKind.INTERNAL, message=str(e), source="hot_reloader.reload", original=e)]) + \\'\\'\\' + data = _run_audit_on_fixture(src) + findings = _classifications_for_file(data, "audit_heuristic_fixture.py") + excepts = [f for f in findings if f["kind"] == "EXCEPT"] + assert len(excepts) == 1 + assert excepts[0]["category"] == "INTERNAL_COMPLIANT", ( + f"Result-returning recovery in non-*_result function should be INTERNAL_COMPLIANT, got {excepts[0]['category']}" + ) + + +def test_result_returning_recovery_in_result_named_function_is_compliant(): + """Same pattern but with a function name ending in '_result' is also compliant (and ideal). + + This is the canonical naming: functions that return Result should end in '_result'. + """ + src = \\'\\'\\' + from src.result_types import Result, ErrorInfo, ErrorKind + + def reload_result(module_name): + try: + importlib.reload(sys.modules[module_name]) + return Result(data=True) + except (ImportError, ModuleNotFoundError) as e: + return Result(data=False, errors=[ErrorInfo(kind=ErrorKind.INTERNAL, message=str(e), source="hot_reloader.reload_result", original=e)]) + \\'\\'\\' + data = _run_audit_on_fixture(src) + findings = _classifications_for_file(data, "audit_heuristic_fixture.py") + excepts = [f for f in findings if f["kind"] == "EXCEPT"] + assert len(excepts) == 1 + assert excepts[0]["category"] == "INTERNAL_COMPLIANT", ( + f"Result-returning recovery in *_result function should be INTERNAL_COMPLIANT, got {excepts[0]['category']}" + ) +''' + +with p.open("a", encoding="utf-8", newline="") as f: + f.write(append) +print("ok") diff --git a/scripts/tier2/artifacts/result_migration_small_files_20260617/fix_escape.py b/scripts/tier2/artifacts/result_migration_small_files_20260617/fix_escape.py new file mode 100644 index 00000000..ea4c376f --- /dev/null +++ b/scripts/tier2/artifacts/result_migration_small_files_20260617/fix_escape.py @@ -0,0 +1,12 @@ +"""Fix the bad backslash escape in heuristic A tests.""" +from __future__ import annotations +from pathlib import Path + +p = Path("tests/test_audit_exception_handling_heuristics.py") +content = p.read_text(encoding="utf-8") + +# Replace bad backslash-escaped triple-quotes +content = content.replace(r"\'\'\'", "'''") + +p.write_text(content, encoding="utf-8") +print("ok") diff --git a/scripts/tier2/artifacts/result_migration_small_files_20260617/insert_heuristic_a.py b/scripts/tier2/artifacts/result_migration_small_files_20260617/insert_heuristic_a.py new file mode 100644 index 00000000..da2d7526 --- /dev/null +++ b/scripts/tier2/artifacts/result_migration_small_files_20260617/insert_heuristic_a.py @@ -0,0 +1,32 @@ +"""Add Heuristic A check at the START of _classify_except, before BOUNDARY_CONVERSION.""" +from __future__ import annotations +from pathlib import Path + +p = Path("scripts/audit_exception_handling.py") +content = p.read_text(encoding="utf-8") + +needle = " # ----- Classification logic -----\n\n # 1. ErrorInfo conversion = canonical boundary pattern\n if creates_errorinfo:\n return (\n \"BOUNDARY_CONVERSION\"," + +replacement = """ # ----- Classification logic ----- + + # 0. Heuristic A: Result-returning recovery — the canonical data-oriented pattern. + # If the except body returns `Result(data=..., errors=[ErrorInfo(...)])`, + # the function is following the convention. Classify as INTERNAL_COMPLIANT + # BEFORE the BOUNDARY_CONVERSION check (which also fires for ErrorInfo creation). + if self._returns_result(body): + return ( + "INTERNAL_COMPLIANT", + "Compliant: `try: ...; except: return Result(data=..., errors=[...])` is the canonical Result-recovery pattern. The convention requires Result[T] for try/except sites that can fail; this pattern satisfies the requirement. The function-name-not-ending-in-`_result` is a smell (rename to `xxx_result`); the pattern itself is compliant. (per result_migration_small_files_20260617 Phase 11.2, Heuristic A)", + ) + + # 1. ErrorInfo conversion = canonical boundary pattern + if creates_errorinfo: + return ( + "BOUNDARY_CONVERSION",""" + +if needle not in content: + print("ERROR: needle not found") + raise SystemExit(1) +content = content.replace(needle, replacement) +p.write_text(content, encoding="utf-8", newline="") +print("ok") diff --git a/tests/test_audit_exception_handling_heuristics.py b/tests/test_audit_exception_handling_heuristics.py index e7ad46ae..df5029b0 100644 --- a/tests/test_audit_exception_handling_heuristics.py +++ b/tests/test_audit_exception_handling_heuristics.py @@ -337,3 +337,61 @@ def test_narrow_except_uses_error_inline_is_compliant(): assert excepts[0]["category"] == "INTERNAL_COMPLIANT", ( f"narrow except using error inline should be INTERNAL_COMPLIANT, got {excepts[0]['category']}" ) + + +# --------------------------------------------------------------------------- +# Heuristic A: Result-returning recovery in non-*_result function (Phase 11.2) +# --------------------------------------------------------------------------- +def test_result_returning_recovery_in_non_result_named_function_is_compliant(): + """try: ...; except SpecificError: return Result(data=..., errors=[ErrorInfo(...)]) is compliant. + + The function returns a Result with errors= on failure (the canonical Result + recovery pattern). The convention requires Result[T] for try/except sites + that can fail; this pattern satisfies the requirement. The function name + not ending in '_result' is a smell (the function should be renamed to + 'xxx_result') but the pattern itself is compliant. + This is the pattern used by src/hot_reloader.py:reload(), + src/warmup.py:on_complete/_record_success/_record_failure, and the + other 17 sites migrated in Phase 11.3. + """ + src = ''' + from src.result_types import Result, ErrorInfo, ErrorKind + + def reload(module_name): + try: + importlib.reload(sys.modules[module_name]) + return Result(data=True) + except (ImportError, ModuleNotFoundError) as e: + return Result(data=False, errors=[ErrorInfo(kind=ErrorKind.INTERNAL, message=str(e), source="hot_reloader.reload", original=e)]) + ''' + data = _run_audit_on_fixture(src) + findings = _classifications_for_file(data, "audit_heuristic_fixture.py") + excepts = [f for f in findings if f["kind"] == "EXCEPT"] + assert len(excepts) == 1 + assert excepts[0]["category"] == "INTERNAL_COMPLIANT", ( + f"Result-returning recovery in non-*_result function should be INTERNAL_COMPLIANT, got {excepts[0]['category']}" + ) + + +def test_result_returning_recovery_in_result_named_function_is_compliant(): + """Same pattern but with a function name ending in '_result' is also compliant (and ideal). + + This is the canonical naming: functions that return Result should end in '_result'. + """ + src = ''' + from src.result_types import Result, ErrorInfo, ErrorKind + + def reload_result(module_name): + try: + importlib.reload(sys.modules[module_name]) + return Result(data=True) + except (ImportError, ModuleNotFoundError) as e: + return Result(data=False, errors=[ErrorInfo(kind=ErrorKind.INTERNAL, message=str(e), source="hot_reloader.reload_result", original=e)]) + ''' + data = _run_audit_on_fixture(src) + findings = _classifications_for_file(data, "audit_heuristic_fixture.py") + excepts = [f for f in findings if f["kind"] == "EXCEPT"] + assert len(excepts) == 1 + assert excepts[0]["category"] == "INTERNAL_COMPLIANT", ( + f"Result-returning recovery in *_result function should be INTERNAL_COMPLIANT, got {excepts[0]['category']}" + )