Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 40 additions & 7 deletions libs/openant-core/parsers/python/function_extractor.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,19 @@ def get_docstring(self, node: ast.AST) -> Optional[str]:
"""Extract docstring from a function or class."""
return ast.get_docstring(node)

def _path_has_segment(self, file_path: str, token: str) -> bool:
"""True if `token` equals a whole path segment (a directory name or the filename stem),
case-insensitively -- used instead of a bare ``token in path`` substring test so that, for
example, 'views' classifies ``app/views.py`` and ``app/views/x.py`` but NOT ``interviews/a.py``
or ``app/previews/b.py``."""
p = Path(file_path)
try:
p = p.relative_to(self.repo_path)
except ValueError:
pass
segments = {s.lower() for s in p.with_suffix('').parts}
return token.lower() in segments

def classify_function(self, func_name: str, decorators: List[str],
class_name: Optional[str], file_path: str) -> str:
"""Classify a function by its type/purpose."""
Expand All @@ -207,7 +220,7 @@ def classify_function(self, func_name: str, decorators: List[str],
return 'route_handler'

# Django views
if 'views' in path_lower and class_name is None:
if self._path_has_segment(file_path, 'views') and class_name is None:
return 'view_function'

# Class methods
Expand All @@ -225,10 +238,15 @@ def classify_function(self, func_name: str, decorators: List[str],
return 'method'

# Middleware/decorators
if 'middleware' in func_name.lower() or 'middleware' in path_lower:
if 'middleware' in func_name.lower() or self._path_has_segment(file_path, 'middleware'):
return 'middleware'

# Test functions
# Test functions.
# 'test' is matched as a substring here on purpose: test-file conventions use plural/affixed
# forms ('tests/' dir, 'test_*'/'*_test' files) that a whole-segment match would miss. The
# substring over-match (e.g. 'latest'/'contest') is the is_test_file family handled in its own
# scheduled units -- 'test' is NOT an entry-point type, so unlike 'views' it seeds no false
# reachability and is intentionally left as a substring test in this fix.
if func_name.startswith('test_') or 'test' in path_lower:
return 'test'

Expand Down Expand Up @@ -261,9 +279,21 @@ def extract_imports(self, tree: ast.AST, file_path: str) -> Dict[str, str]:
imports[name] = alias.name
elif isinstance(node, ast.ImportFrom):
module = node.module or ''
level = node.level or 0
if level > 0:
# Relative import: reconstruct the absolute package anchor from the importing
# file's location so the dotted path resolves to a real module. file_path is
# repo-relative (e.g. 'pkg/sub/mod.py'); its package is the directory parts.
# level=1 -> the file's own package, level=2 -> the parent package, etc.
pkg_parts = list(Path(file_path).parts[:-1])
keep = max(0, len(pkg_parts) - (level - 1))
anchor = pkg_parts[:keep]
base_parts = anchor + ([module] if module else [])
else:
base_parts = [module] if module else []
for alias in node.names:
name = alias.asname or alias.name
full_path = f"{module}.{alias.name}" if module else alias.name
full_path = '.'.join(base_parts + [alias.name]) if base_parts else alias.name
imports[name] = full_path

return imports
Expand Down Expand Up @@ -552,9 +582,12 @@ def extract_all(self, files: Optional[List[str]] = None) -> Dict:
else:
# Scan all .py files
for file_path in self.repo_path.rglob('*.py'):
# Skip common exclude patterns
path_str = str(file_path)
if any(excl in path_str for excl in ['__pycache__', '.git', 'venv', '.venv', 'node_modules']):
# Skip common excluded directories. Match whole path SEGMENTS (not a substring of the
# full path) so e.g. 'venv' excludes a real venv/ dir but not 'myvenv/keep.py', and an
# ancestor dir whose name contains a token cannot poison the whole scan. rglob yields
# paths under repo_path, so relative_to never raises.
excluded = {'__pycache__', '.git', 'venv', '.venv', 'node_modules'}
if excluded & set(file_path.relative_to(self.repo_path).parts):
continue
self.process_file(file_path)

Expand Down
81 changes: 81 additions & 0 deletions libs/openant-core/tests/test_python_function_extractor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
"""Regression tests for three independent defects in parsers/python/function_extractor.py.

Segment-vs-substring exclusion: extract_all() no-args branch excludes files via an UNANCHORED
substring test (`any(excl in str(file_path) ...)`), so a file whose path merely contains a token
('myvenv/keep.py' contains 'venv') is wrongly skipped. Fix: match whole path SEGMENTS.
Entry-point classification: classify_function uses `'<token>' in path_lower` substring tests to
assign ENTRY-POINT unit_types, so 'interviews/api.py' is classified 'view_function' (a false
reachability seed). Fix: match the 'views'/'middleware' tokens as whole path segments. ('test' is
intentionally left as a substring -- see the in-code note; it is not an entry-point type.)
Relative-import anchor: extract_imports ignores ast.ImportFrom.level, so relative imports lose their
package anchor ('from . import X' -> bare 'X'). Fix: reconstruct the absolute anchor from the
importing file's location.

Loads function_extractor under a UNIQUE module name (not the bare 'function_extractor', which the c/
go/php/ruby/zig parsers also ship) so a bare import cannot pollute sys.modules for sibling tests.
"""
import ast
import importlib.util
import sys
from pathlib import Path

CORE = Path(__file__).resolve().parents[1] # libs/openant-core
if str(CORE) not in sys.path:
sys.path.insert(0, str(CORE)) # for utilities.* if imported

_spec = importlib.util.spec_from_file_location(
"py_function_extractor_isolated", str(CORE / "parsers" / "python" / "function_extractor.py"))
_mod = importlib.util.module_from_spec(_spec)
_spec.loader.exec_module(_mod)
FunctionExtractor = _mod.FunctionExtractor


# ---- extract_all excludes by path segment, not substring ----
def test_extract_all_excludes_on_path_segments_not_substring(tmp_path, monkeypatch):
repo = tmp_path / "repo"
for d in ("myvenv", "venv", ".git", "pkg/__pycache__", "src"):
(repo / d).mkdir(parents=True, exist_ok=True)
(repo / "myvenv" / "keep.py").write_text("def f(): pass\n") # 'venv' substring -> wrongly skipped pre-fix
(repo / "src" / "clean.py").write_text("def g(): pass\n")
(repo / "venv" / "skip.py").write_text("def h(): pass\n") # real venv/ -> excluded
(repo / ".git" / "hook.py").write_text("def i(): pass\n") # .git -> excluded
(repo / "pkg" / "__pycache__" / "c.py").write_text("def j(): pass\n") # __pycache__ -> excluded

ex = FunctionExtractor(str(repo))
processed = []
monkeypatch.setattr(ex, "process_file",
lambda fp: processed.append(Path(fp).relative_to(ex.repo_path).as_posix()))
ex.extract_all()
seen = set(processed)

assert "myvenv/keep.py" in seen, f"'myvenv' wrongly excluded by 'venv' substring: {sorted(seen)}"
assert "src/clean.py" in seen
assert "venv/skip.py" not in seen, "a real venv/ directory must stay excluded"
assert ".git/hook.py" not in seen, ".git must stay excluded"
assert "pkg/__pycache__/c.py" not in seen, "__pycache__ must stay excluded"


# ---- classify_function matches entry-point tokens by segment, not substring ----
def test_classify_function_entrypoint_tokens_match_segments_not_substring(tmp_path):
ex = FunctionExtractor(str(tmp_path))
c = lambda path: ex.classify_function("handler", [], None, path)

# genuine segments still classify (no regression)
assert c("app/views.py") == "view_function"
assert c("app/views/handlers.py") == "view_function"
assert c("app/middleware/auth.py") == "middleware"
# substring over-matches must NOT seed entry-point types
assert c("interviews/api.py") != "view_function", "'interviews' wrongly matched 'views'"
assert c("app/previews/x.py") != "view_function", "'previews' wrongly matched 'views'"
assert c("app/previewmiddleware.py") != "middleware", "'previewmiddleware' wrongly matched 'middleware'"


# ---- extract_imports preserves the relative-import package anchor (node.level) ----
def test_extract_imports_preserves_relative_package_anchor(tmp_path):
ex = FunctionExtractor(str(tmp_path))
src = "from . import helpers\nfrom ..util import U\nfrom os import path\n"
imports = ex.extract_imports(ast.parse(src), "pkg/sub/mod.py")

assert imports["helpers"] == "pkg.sub.helpers", f"relative 'from . import' lost anchor: {imports}"
assert imports["U"] == "pkg.util.U", f"relative 'from ..util import' lost anchor: {imports}"
assert imports["path"] == "os.path", f"absolute import must be unchanged: {imports}"
Loading