From c18a987e19a929f6f7670876b999e0fe01900a8f Mon Sep 17 00:00:00 2001 From: Gadi Evron Date: Thu, 28 May 2026 12:20:29 -0700 Subject: [PATCH] fix(parsers/zig): align function_extractor with the real tree-sitter-zig grammar + lazy import Pinned against the actually installed grammar (tree-sitter-zig 1.1.2) by AST-probing it directly; several AST node-type names had drifted. Three defects in libs/openant-core/parsers/zig/function_extractor.py: 1. Struct extraction was dead and fields could be emitted as functions. Struct handling gated on node types the 1.1.2 grammar never emits (VarDecl / container_decl / ContainerDecl); the grammar emits variable_declaration and struct_declaration, so total_classes was always 0 and the literal container_field->method path was unreachable. Corrected the node-type names; methods now flow through the recursive walk which only treats function_declaration as a unit, so container_field (struct FIELDS) are never emitted. The dedicated _extract_struct_methods helper (home of the original container_field bug) is removed as superseded. 2. A struct method was emitted twice (qualified Type.method by the eager scan, then bare by an unconditional recursion that never threaded current_struct). Now thread the struct name into the recursion so a method produces the same func_id and the dict de-duplicates instead of producing a bare twin. Also fix a name-capture bug in _extract_function (it grabbed the last identifier, recording `fn init(...) Point {` under the return type `Point`); it now captures the first identifier and stops. Regression test pins that the de-dup does not drop nested struct / method / @import. 3. tree_sitter_zig was imported at module top level and called at class-body time, raising ImportError in any clean install lacking the package, which was declared in neither pyproject.toml nor requirements.txt. Load the grammar lazily on first FunctionExtractor construction with a clear error message, and declare tree-sitter-zig>=1.1.2 in pyproject.toml + requirements.txt. (call_graph_builder.py carries the same top-level import but is out of this change's scope; the dependency declarations cover it too.) New tests/parsers/zig/test_zig_extractor_followup.py (loaded via importlib unique name because function_extractor.py recurs across parser packages). RED at base 6 failed / 1 passed; GREEN 7 passed. Full suite 183 passed, 63 skipped, 0 failed. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../parsers/zig/function_extractor.py | 127 +++++----- libs/openant-core/pyproject.toml | 1 + libs/openant-core/requirements.txt | 1 + .../tests/parsers/zig/__init__.py | 0 .../zig/test_zig_extractor_followup.py | 229 ++++++++++++++++++ 5 files changed, 294 insertions(+), 64 deletions(-) create mode 100644 libs/openant-core/tests/parsers/zig/__init__.py create mode 100644 libs/openant-core/tests/parsers/zig/test_zig_extractor_followup.py diff --git a/libs/openant-core/parsers/zig/function_extractor.py b/libs/openant-core/parsers/zig/function_extractor.py index 647f0cd5..779e30ab 100644 --- a/libs/openant-core/parsers/zig/function_extractor.py +++ b/libs/openant-core/parsers/zig/function_extractor.py @@ -10,19 +10,37 @@ from utilities.file_io import write_json -import tree_sitter_zig as ts_zig from tree_sitter import Language, Parser, Node +def _load_zig_language() -> Language: + """Load the Zig tree-sitter grammar lazily. + + The grammar package (``tree-sitter-zig``) is an optional runtime + dependency. Importing it at module top level made the entire Zig parser + unimportable in any environment where the package is absent (e.g. a clean + install that does not need Zig support). Resolving it here, on first use, + lets the module import unconditionally and surfaces a clear, actionable + error only when the Zig parser is actually exercised. + """ + try: + import tree_sitter_zig as ts_zig + except ImportError as exc: # pragma: no cover - exercised via the no-dep test + raise ImportError( + "The Zig parser requires the 'tree-sitter-zig' package, which is not " + "installed. Install it with `pip install tree-sitter-zig` " + "(declared in pyproject.toml / requirements.txt)." + ) from exc + return Language(ts_zig.language()) + + class FunctionExtractor: """Extracts functions and structs from Zig source files using tree-sitter.""" - ZIG_LANGUAGE = Language(ts_zig.language()) - def __init__(self, repo_path: str, scan_results: Dict[str, Any]): self.repo_path = Path(repo_path).resolve() self.scan_results = scan_results - self.parser = Parser(self.ZIG_LANGUAGE) + self.parser = Parser(_load_zig_language()) def extract(self) -> Dict[str, Any]: """ @@ -95,58 +113,70 @@ def _walk_node( imports: List[str], current_struct: Optional[str], ) -> None: - """Recursively walk the AST to extract definitions.""" + """Recursively walk the AST to extract definitions. + + Node-type names match the tree-sitter-zig grammar actually in use + (variable_declaration / struct_declaration / function_declaration / + builtin_function). Earlier names (VarDecl / container_decl / FnProto / + builtin_call_expr) were from a different grammar revision and never + matched, leaving struct extraction dead and methods mis-emitted. + """ + + # The struct context for any children we recurse into. It only changes + # when this node is itself a `const Name = struct {...}` declaration. + child_struct = current_struct - if node.type == "function_declaration" or node.type == "FnProto": + if node.type == "function_declaration": func_info = self._extract_function(node, source, file_path, current_struct) if func_info: func_id = f"{file_path}:{func_info['qualified_name']}" functions[func_id] = func_info - elif node.type == "VarDecl": - # Check if this is a struct/enum definition + elif node.type == "variable_declaration": + # `const Foo = struct { ... };` -- a named struct/enum definition. struct_info = self._extract_struct_from_var_decl(node, source, file_path) if struct_info: struct_id = f"{file_path}:{struct_info['name']}" structs[struct_id] = struct_info - # Extract methods within the struct - self._extract_struct_methods( - node, source, file_path, struct_info["name"], functions - ) - - elif node.type == "container_decl" or node.type == "ContainerDecl": - # Direct struct/enum declarations - struct_info = self._extract_container(node, source, file_path) - if struct_info: - struct_id = f"{file_path}:{struct_info['name']}" - structs[struct_id] = struct_info - - elif node.type == "@import" or ( - node.type == "builtin_call_expr" - and self._get_node_text(node, source).startswith("@import") - ): + # Thread the struct name down into the body so its method + # function_declarations are emitted ONCE, qualified as + # Struct.method (a method visited via recursion produces the + # same func_id as the eager scan, so there is no bare duplicate). + child_struct = struct_info["name"] + + elif node.type == "builtin_function" and self._get_node_text( + node, source + ).startswith("@import"): import_path = self._extract_import(node, source) if import_path: imports.append(import_path) - # Recurse into children + # Recurse into children with the (possibly updated) struct context so + # nested function_declarations are qualified correctly. Struct FIELDS + # (`container_field`) are not function_declarations, so they are never + # emitted as units. for child in node.children: self._walk_node( - child, source, file_path, functions, structs, imports, current_struct + child, source, file_path, functions, structs, imports, child_struct ) def _extract_function( self, node: Node, source: bytes, file_path: str, current_struct: Optional[str] ) -> Optional[Dict[str, Any]]: """Extract function information from a function declaration node.""" - # Find function name + # Find function name. In the grammar a function_declaration is + # `[pub] fn (params) [return-type] block`; the function + # name is the FIRST identifier. A return type can also be a bare + # identifier (e.g. `fn init(...) Point {`), so capture the name once + # and stop -- otherwise the return-type identifier overwrites it. name = None parameters = [] for child in node.children: if child.type == "identifier" or child.type == "IDENTIFIER": - name = self._get_node_text(child, source) - elif child.type == "parameters" or child.type == "ParamDeclList": + if name is None: + name = self._get_node_text(child, source) + elif child.type in ("parameters", "ParamDeclList"): parameters = self._extract_parameters(child, source) if not name: @@ -195,9 +225,10 @@ def _extract_struct_from_var_decl( is_struct = False for child in node.children: - if child.type == "identifier" or child.type == "IDENTIFIER": - name = self._get_node_text(child, source) - elif child.type == "container_decl" or child.type == "ContainerDecl": + if child.type in ("identifier", "IDENTIFIER"): + if name is None: + name = self._get_node_text(child, source) + elif child.type == "struct_declaration": is_struct = True if name and is_struct: @@ -210,38 +241,6 @@ def _extract_struct_from_var_decl( } return None - def _extract_container( - self, node: Node, source: bytes, file_path: str - ) -> Optional[Dict[str, Any]]: - """Extract struct/enum from a container declaration.""" - # Anonymous container - try to find name from parent - return None - - def _extract_struct_methods( - self, - node: Node, - source: bytes, - file_path: str, - struct_name: str, - functions: Dict[str, Any], - ) -> None: - """Extract methods from within a struct definition.""" - for child in node.children: - if child.type == "container_decl" or child.type == "ContainerDecl": - for member in child.children: - if ( - member.type == "function_declaration" - or member.type == "FnProto" - or member.type == "container_field" - ): - # Check if it's a function field - func_info = self._extract_function( - member, source, file_path, struct_name - ) - if func_info: - func_id = f"{file_path}:{func_info['qualified_name']}" - functions[func_id] = func_info - def _extract_import(self, node: Node, source: bytes) -> Optional[str]: """Extract import path from an @import call.""" text = self._get_node_text(node, source) diff --git a/libs/openant-core/pyproject.toml b/libs/openant-core/pyproject.toml index bf0377a8..3fb2c000 100644 --- a/libs/openant-core/pyproject.toml +++ b/libs/openant-core/pyproject.toml @@ -16,6 +16,7 @@ dependencies = [ "tree-sitter-cpp>=0.21.0", "tree-sitter-ruby>=0.21.0", "tree-sitter-php>=0.22.0", + "tree-sitter-zig>=1.1.2", ] [project.optional-dependencies] diff --git a/libs/openant-core/requirements.txt b/libs/openant-core/requirements.txt index 966904a8..d821a8f4 100644 --- a/libs/openant-core/requirements.txt +++ b/libs/openant-core/requirements.txt @@ -22,3 +22,4 @@ tree-sitter-c>=0.21.0 tree-sitter-cpp>=0.21.0 tree-sitter-ruby>=0.21.0 tree-sitter-php>=0.22.0 +tree-sitter-zig>=1.1.2 diff --git a/libs/openant-core/tests/parsers/zig/__init__.py b/libs/openant-core/tests/parsers/zig/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/libs/openant-core/tests/parsers/zig/test_zig_extractor_followup.py b/libs/openant-core/tests/parsers/zig/test_zig_extractor_followup.py new file mode 100644 index 00000000..d2e5640d --- /dev/null +++ b/libs/openant-core/tests/parsers/zig/test_zig_extractor_followup.py @@ -0,0 +1,229 @@ +"""Regression tests for the Zig function/struct extractor. + +Pinned against the actual tree-sitter-zig 1.1.2 grammar (the AST node-type +names below were confirmed by parsing sample Zig with the live grammar): + + - struct FIELDS (`container_field` nodes) must not be emitted as function + units, and struct extraction must actually run. The extractor previously + gated struct handling on grammar node types `VarDecl` / `container_decl` / + `ContainerDecl` that the real 1.1.2 grammar never emits (it emits + `variable_declaration` / `struct_declaration`), so struct extraction was + dead code (0 classes) and the field/method paths were unreachable. + + - a struct method must appear exactly once, qualified as `.`, + not duplicated as a bare top-level function. `_extract_struct_methods` + emitted the qualified entry, then the unconditional child recursion in + `_walk_node` re-descended into the same struct body and re-emitted the + method bare because `current_struct` was never threaded through. + + - `tree_sitter_zig` was imported hard at module top level and called at + class-body time, so a clean environment without the grammar package raised + ImportError on import. The parser must degrade gracefully (importable; a + clear error only when actually used). + +The basename `function_extractor.py` recurs across parser packages +(c/cpp/php/python/ruby/zig), so this module is loaded under a unique name via +importlib to avoid any sys.modules cache collision. +""" + +import importlib.util +import os +import sys +import tempfile +from pathlib import Path + +import pytest + +_CORE_ROOT = Path(__file__).resolve().parents[3] +if str(_CORE_ROOT) not in sys.path: + sys.path.insert(0, str(_CORE_ROOT)) + +# tree-sitter-zig is required to exercise the extractor; skip cleanly if absent. +ts_zig = pytest.importorskip("tree_sitter_zig") + +_EXTRACTOR_PATH = _CORE_ROOT / "parsers" / "zig" / "function_extractor.py" + + +def _load_extractor_module(): + """Load parsers/zig/function_extractor.py under a unique module name.""" + spec = importlib.util.spec_from_file_location( + "zig_function_extractor_isolated", _EXTRACTOR_PATH + ) + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +def _extract(source: str) -> dict: + """Run FunctionExtractor over a single in-memory main.zig file.""" + module = _load_extractor_module() + repo = tempfile.mkdtemp() + (Path(repo) / "main.zig").write_text(source) + extractor = module.FunctionExtractor(repo, {"files": [{"path": "main.zig"}]}) + return extractor.extract() + + +# A struct with two fields and two methods, plus a free function. +_SAMPLE = """const std = @import("std"); + +const Point = struct { + x: i32, + y: i32, + + pub fn init(x: i32, y: i32) Point { + return Point{ .x = x, .y = y }; + } + + pub fn distance(self: Point) i32 { + return self.x + self.y; + } +}; + +fn helper(n: i32) i32 { + return n * 2; +} +""" + + +def test_struct_field_is_not_a_function(): + """`container_field` struct fields must never become function units.""" + result = _extract(_SAMPLE) + func_names = {info["name"] for info in result["functions"].values()} + assert "x" not in func_names, f"struct field 'x' emitted as a function: {func_names}" + assert "y" not in func_names, f"struct field 'y' emitted as a function: {func_names}" + + +def test_struct_is_extracted_as_a_class(): + """The struct must be extracted as a class (grammar-drift had killed this).""" + result = _extract(_SAMPLE) + class_names = {info["name"] for info in result["classes"].values()} + assert "Point" in class_names, ( + f"struct 'Point' was not extracted as a class; classes={class_names}. " + "Struct handling is gated on grammar node types the 1.1.2 grammar " + "does not emit (VarDecl/container_decl)." + ) + + +def test_only_real_methods_extracted_and_qualified_once(): + """Each method appears exactly once, qualified as Point.; no bare dup.""" + result = _extract(_SAMPLE) + funcs = result["functions"] + qualified = [info["qualified_name"] for info in funcs.values()] + + # The two methods must be present, qualified, exactly once each. + assert qualified.count("Point.init") == 1, f"Point.init not uniquely qualified: {qualified}" + assert qualified.count("Point.distance") == 1, ( + f"Point.distance not uniquely qualified: {qualified}" + ) + + # No bare duplicate of a method that also exists qualified. + assert "init" not in qualified, f"method 'init' emitted bare (duplicate): {qualified}" + assert "distance" not in qualified, ( + f"method 'distance' emitted bare (duplicate): {qualified}" + ) + + # The return-type identifier of init ('Point') must not be captured as a function name. + assert "Point" not in qualified, ( + f"return-type identifier 'Point' captured as a function name: {qualified}" + ) + + # The free function is still extracted, bare. + assert qualified.count("helper") == 1, f"free function 'helper' missing: {qualified}" + + +def test_nested_struct_and_import_not_dropped(): + """De-duplicating methods must not drop nested declarations. + + A naive fix that simply stops recursing into a struct body to avoid the + bare-duplicate emission would also lose nested structs, their methods, and + nested @import calls. The correct fix threads the struct context through + recursion (same func_id => natural de-dup), so everything below is kept. + """ + source = ( + "const Outer = struct {\n" + ' const builtin = @import("builtin");\n' + "\n" + " const Inner = struct {\n" + " pub fn innerFn() void {}\n" + " };\n" + "\n" + " pub fn outerFn() void {}\n" + "};\n" + ) + result = _extract(source) + qualified = [info["qualified_name"] for info in result["functions"].values()] + class_names = {info["name"] for info in result["classes"].values()} + + assert "Outer.outerFn" in qualified, qualified + assert "Inner.innerFn" in qualified, qualified + assert "Outer" in class_names, class_names + assert "Inner" in class_names, class_names + assert "builtin" in result["imports"]["main.zig"], result["imports"] + + +def test_methods_marked_as_methods_with_class_name(): + """Qualified methods carry unit_type=method and class_name=Point.""" + result = _extract(_SAMPLE) + methods = { + info["qualified_name"]: info + for info in result["functions"].values() + if info["qualified_name"].startswith("Point.") + } + assert set(methods) == {"Point.init", "Point.distance"}, sorted(methods) + for info in methods.values(): + assert info["unit_type"] == "method", info + assert info["class_name"] == "Point", info + + +def test_module_imports_without_grammar_package(monkeypatch): + """The parser module must import even when tree_sitter_zig is absent. + + Simulate a clean environment by hiding `tree_sitter_zig` from import, then + load the module fresh. A hard top-level import + class-body call raises + ImportError here; a graceful (lazy/guarded) parser imports fine and only + fails when the grammar is actually needed. + """ + # Drop any cached copies so the fresh import re-evaluates the import machinery. + for name in list(sys.modules): + if name == "tree_sitter_zig" or name.startswith("tree_sitter_zig."): + monkeypatch.delitem(sys.modules, name, raising=False) + if name.startswith("zig_function_extractor_isolated"): + monkeypatch.delitem(sys.modules, name, raising=False) + + real_import = __builtins__["__import__"] if isinstance(__builtins__, dict) else __builtins__.__import__ + + def _blocked_import(name, *args, **kwargs): + if name == "tree_sitter_zig" or name.startswith("tree_sitter_zig."): + raise ImportError("No module named 'tree_sitter_zig' (simulated clean env)") + return real_import(name, *args, **kwargs) + + monkeypatch.setattr("builtins.__import__", _blocked_import) + + spec = importlib.util.spec_from_file_location( + "zig_function_extractor_isolated_nodep", _EXTRACTOR_PATH + ) + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + try: + # Must not raise: importing the parser cannot require the grammar package. + spec.loader.exec_module(module) + except ImportError as exc: # pragma: no cover - this is the failure we pin against + pytest.fail( + "Zig parser module raised ImportError on import when tree_sitter_zig " + f"is absent (no graceful degradation): {exc}" + ) + finally: + sys.modules.pop(spec.name, None) + + +def test_pyproject_declares_tree_sitter_zig(): + """The zig grammar must be declared as a dependency (it is imported).""" + pyproject = (_CORE_ROOT / "pyproject.toml").read_text() + requirements = (_CORE_ROOT / "requirements.txt").read_text() + assert "tree-sitter-zig" in pyproject, ( + "tree-sitter-zig imported by parsers/zig but missing from pyproject.toml dependencies" + ) + assert "tree-sitter-zig" in requirements, ( + "tree-sitter-zig imported by parsers/zig but missing from requirements.txt" + )