refactor(src): file_cache.py Phase 11.3.5 - extract _get_mtime_safe
Phase 11.3.5. The original try/except (OSError, ValueError): mtime = 0.0 in get_cached_tree is now extracted to a Result-returning helper. The helper returns Result[float]; the caller uses .data (0.0 fallback) and can inspect .errors. The convention requires Result[T] for try/except sites that can fail; the helper satisfies this requirement. Audit post-migration: - _get_mtime_safe L48 = INTERNAL_COMPLIANT (Heuristic A) ✓ - get_cached_tree L92 = no try/except for mtime (extracted) Tests: 24/24 pass (test_ast_parser, test_file_cache_no_top_level_tree_sitter).
This commit is contained in:
@@ -0,0 +1,70 @@
|
||||
"""Migrate file_cache.py: extract Result-returning _get_mtime_safe helper."""
|
||||
from __future__ import annotations
|
||||
from pathlib import Path
|
||||
|
||||
p = Path("src/file_cache.py")
|
||||
content = p.read_text(encoding="utf-8")
|
||||
|
||||
# Add Result imports
|
||||
old_imports = "from typing import Any, Dict, List, Optional, Tuple"
|
||||
new_imports = (
|
||||
"from typing import Any, Dict, List, Optional, Tuple\n"
|
||||
"\n"
|
||||
"from src.result_types import ErrorInfo, ErrorKind, Result"
|
||||
)
|
||||
if old_imports not in content:
|
||||
print("ERROR: imports not found")
|
||||
raise SystemExit(1)
|
||||
content = content.replace(old_imports, new_imports)
|
||||
|
||||
# Replace the try/except in get_cached_tree with helper call
|
||||
old_block = (
|
||||
" try:\n"
|
||||
" p = Path(path)\n"
|
||||
" mtime = p.stat().st_mtime if p.exists() else 0.0\n"
|
||||
" except (OSError, ValueError):\n"
|
||||
" mtime = 0.0"
|
||||
)
|
||||
new_block = (
|
||||
" mtime_result = _get_mtime_safe(path)\n"
|
||||
" mtime = mtime_result.data # 0.0 on error (Result.errors has the details)"
|
||||
)
|
||||
if old_block not in content:
|
||||
print("ERROR: mtime block not found")
|
||||
raise SystemExit(1)
|
||||
content = content.replace(old_block, new_block)
|
||||
|
||||
# Add helper after _ast_cache definition, before class ASTParser
|
||||
helper = '''
|
||||
|
||||
def _get_mtime_safe(path: Optional[str]) -> Result[float]:
|
||||
"""Get file mtime, returning Result[float] with errors on OSError/ValueError.
|
||||
|
||||
The convention requires Result[T] for try/except sites that can fail. Used
|
||||
by ASTParser.get_cached_tree to abstract the mtime computation; the caller
|
||||
uses `.data` (0.0 fallback) and can inspect `.errors` if needed.
|
||||
"""
|
||||
if path is None:
|
||||
return Result(data=0.0)
|
||||
try:
|
||||
p = Path(path)
|
||||
mtime = p.stat().st_mtime if p.exists() else 0.0
|
||||
return Result(data=mtime)
|
||||
except (OSError, ValueError) as e:
|
||||
return Result(data=0.0, errors=[ErrorInfo(
|
||||
kind=ErrorKind.INTERNAL,
|
||||
message=f"failed to get mtime for {path}: {e}",
|
||||
source="file_cache._get_mtime_safe",
|
||||
original=e,
|
||||
)])
|
||||
'''
|
||||
|
||||
old_class_marker = "\n\nclass ASTParser:"
|
||||
new_class_marker = helper + "\n\nclass ASTParser:"
|
||||
if old_class_marker not in content:
|
||||
print("ERROR: class marker not found")
|
||||
raise SystemExit(1)
|
||||
content = content.replace(old_class_marker, new_class_marker)
|
||||
|
||||
p.write_text(content, encoding="utf-8", newline="")
|
||||
print("ok")
|
||||
+26
-5
@@ -40,9 +40,33 @@ import re
|
||||
from pathlib import Path
|
||||
from typing import Any, Dict, List, Optional, Tuple
|
||||
|
||||
from src.result_types import ErrorInfo, ErrorKind, Result
|
||||
|
||||
|
||||
_ast_cache: Dict[str, Tuple[float, tree_sitter.Tree]] = {}
|
||||
|
||||
def _get_mtime_safe(path: Optional[str]) -> Result[float]:
|
||||
"""Get file mtime, returning Result[float] with errors on OSError/ValueError.
|
||||
|
||||
The convention requires Result[T] for try/except sites that can fail. Used
|
||||
by ASTParser.get_cached_tree to abstract the mtime computation; the caller
|
||||
uses `.data` (0.0 fallback) and can inspect `.errors` if needed.
|
||||
"""
|
||||
if path is None:
|
||||
return Result(data=0.0)
|
||||
try:
|
||||
p = Path(path)
|
||||
mtime = p.stat().st_mtime if p.exists() else 0.0
|
||||
return Result(data=mtime)
|
||||
except (OSError, ValueError) as e:
|
||||
return Result(data=0.0, errors=[ErrorInfo(
|
||||
kind=ErrorKind.INTERNAL,
|
||||
message=f"failed to get mtime for {path}: {e}",
|
||||
source="file_cache._get_mtime_safe",
|
||||
original=e,
|
||||
)])
|
||||
|
||||
|
||||
class ASTParser:
|
||||
"""
|
||||
Parser for extracting AST-based views of source code.
|
||||
@@ -78,11 +102,8 @@ class ASTParser:
|
||||
if not path:
|
||||
return self.parse(code)
|
||||
|
||||
try:
|
||||
p = Path(path)
|
||||
mtime = p.stat().st_mtime if p.exists() else 0.0
|
||||
except (OSError, ValueError):
|
||||
mtime = 0.0
|
||||
mtime_result = _get_mtime_safe(path)
|
||||
mtime = mtime_result.data # 0.0 on error (Result.errors has the details)
|
||||
|
||||
if path in _ast_cache:
|
||||
cached_mtime, tree = _ast_cache[path]
|
||||
|
||||
Reference in New Issue
Block a user