diff --git a/libs/openant-core/parsers/python/function_extractor.py b/libs/openant-core/parsers/python/function_extractor.py index 8714e9dc..37475e57 100644 --- a/libs/openant-core/parsers/python/function_extractor.py +++ b/libs/openant-core/parsers/python/function_extractor.py @@ -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.""" @@ -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 @@ -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' @@ -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 @@ -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) diff --git a/libs/openant-core/tests/test_python_function_extractor.py b/libs/openant-core/tests/test_python_function_extractor.py new file mode 100644 index 00000000..43d2908e --- /dev/null +++ b/libs/openant-core/tests/test_python_function_extractor.py @@ -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 `'' 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}"