From f7b494a350d79ee37eca50416636b65513f926fb Mon Sep 17 00:00:00 2001 From: Gustavo Adolfo Moennich Date: Wed, 8 Jul 2026 18:06:37 -0300 Subject: [PATCH 1/2] fix(extract): scope Pascal/Delphi call resolution to class + inherits chain Both extract_pascal (tree-sitter) and _extract_pascal_regex (fallback) resolved every call via a single file-wide {method_name_lower: node_id} dict with no class scoping. Two unrelated classes declaring a same-named method (a common Pascal/Delphi pattern -- property accessors, generated COM/TLB wrapper classes) silently collapsed onto whichever declaration was inserted last, producing wrong cross-class `calls` edges. Add _resolve_pascal_callee_factory, shared by both extractors, which resolves a call in this order: (1) a method on the caller's own class, (2) a method on an ancestor class via the already-resolved `inherits` edges, (3) a file-level free function, (4) an unambiguous global match (exactly one procedure with that name in the file). Ambiguous at every level -> no edge, rather than guessing wrong (mirrors the god-node guard already used by resolve_ruby_member_calls for the analogous Ruby problem). Also fixes a related bug found while testing the ancestor-chain case: _extract_pascal_regex's base-class resolution always went through the cross-file, one-class-per-file convention lookup, creating a duplicate stub node for a base class that is declared in the same file as its subclass. It now reuses the real same-file node when present, matching what extract_pascal already did correctly. Verified against a real MTM/Delphi codebase (COM/TLB import unit with 3 near-identical wrapper classes sharing method names like Create/ Set_RaiseExceptions): 26 of 86 `calls` edges were cross-class false positives before this fix; all are now correctly suppressed while same-class and inherited-method calls (e.g. TKernel.ConnectTo -> TKernel.DisConnect) resolve precisely per class. Adds tests/fixtures/sample_scoped_calls.pas and tests/test_pascal_call_scoping.py: 8 tests (4 scenarios x 2 extractors) covering own-class resolution, the cross-class collision regression (both directions), and ancestor-chain resolution. Confirmed these fail on the pre-fix code (5/8, including an order-dependent case that happened to pass by luck) and pass after the fix. No regressions in the existing Pascal/Delphi/Lazarus suite (38 tests) or the full test suite (2845 passed; the 25 unrelated failures pre-exist in Terraform/Ollama/ image-vision/install areas untouched by this change). --- graphify/extract.py | 147 ++++++++++++++++++++----- tests/fixtures/sample_scoped_calls.pas | 60 ++++++++++ tests/test_pascal_call_scoping.py | 102 +++++++++++++++++ 3 files changed, 281 insertions(+), 28 deletions(-) create mode 100644 tests/fixtures/sample_scoped_calls.pas create mode 100644 tests/test_pascal_call_scoping.py diff --git a/graphify/extract.py b/graphify/extract.py index 16a3f753a..22231af24 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -13241,6 +13241,83 @@ def _pascal_find_body(text: str, start: int) -> tuple[int, int]: return (body_start, len(text)) +def _resolve_pascal_callee_factory( + records: list[tuple], + edges: list[dict], + module_nid: str, +) -> Callable[[str, str], str | None]: + """Build a scoped call resolver for a single Pascal/Delphi file. + + ``records`` is the list of raw per-procedure tuples produced by either + Pascal extractor; only the trailing ``(..., container, name_lower)`` + fields and the leading ``proc_nid`` are used, so both extractors' tuple + shapes work unmodified (regex: proc_nid, line, body_text, container, + name_lower; tree-sitter: proc_nid, body_node, container, name_lower). + + Resolution order for a call to ``name_lower`` from ``caller_nid``: + 1. A method declared on the caller's own class. + 2. A method declared on an ancestor class (BFS up ``inherits`` edges, + which are already resolved by this point). + 3. A file-level free function (declared directly under the module). + 4. A global by-name match, but only when unambiguous (exactly one + procedure with that name anywhere in the file). + + Returns None (no edge emitted) when the name is ambiguous at every + level -- guessing at a same-named method on an unrelated class is worse + than omitting the edge. Same-named methods on unrelated classes are a + common Pascal/Delphi pattern (property accessors, generated wrapper + classes such as TLB import units): without this scoping, a flat + file-wide by-name lookup silently collapses onto whichever declaration + happens to be inserted last, producing wrong cross-class edges. Mirrors + the "god-node guard" already used by ``resolve_ruby_member_calls`` for + the analogous Ruby ambiguous-method-name problem. + """ + class_bases: dict[str, list[str]] = {} + for e in edges: + if e.get("relation") == "inherits": + class_bases.setdefault(e["source"], []).append(e["target"]) + + class_procs: dict[str, dict[str, list[str]]] = {} + module_procs: dict[str, list[str]] = {} + global_procs: dict[str, list[str]] = {} + proc_owner: dict[str, str] = {} + for rec in records: + proc_nid, container, name_lower = rec[0], rec[-2], rec[-1] + proc_owner[proc_nid] = container + global_procs.setdefault(name_lower, []).append(proc_nid) + if container == module_nid: + module_procs.setdefault(name_lower, []).append(proc_nid) + else: + class_procs.setdefault(container, {}).setdefault(name_lower, []).append(proc_nid) + + def _resolve(caller_nid: str, name_lower: str) -> str | None: + owner = proc_owner.get(caller_nid) + if owner is not None: + candidates = class_procs.get(owner, {}).get(name_lower) + if candidates: + return candidates[0] if len(candidates) == 1 else None + seen_bases: set[str] = set() + queue = list(class_bases.get(owner, [])) + while queue: + base = queue.pop(0) + if base in seen_bases: + continue + seen_bases.add(base) + candidates = class_procs.get(base, {}).get(name_lower) + if candidates: + return candidates[0] if len(candidates) == 1 else None + queue.extend(class_bases.get(base, [])) + candidates = module_procs.get(name_lower) + if candidates: + return candidates[0] if len(candidates) == 1 else None + candidates = global_procs.get(name_lower) + if candidates and len(candidates) == 1: + return candidates[0] + return None + + return _resolve + + def _extract_pascal_regex(path: Path) -> dict: """Regex fallback for Pascal/Delphi extraction when tree-sitter-pascal is unavailable. Produces the same node/edge schema as the tree-sitter pass. @@ -13325,10 +13402,18 @@ def _lineno(text: str, offset: int) -> int: _add_edge(module_nid, cls_nid, "contains", line) for base_name in _pascal_split_bases(bases_raw): - resolved = _pascal_resolve_class(path, base_name) - base_nid = resolved if resolved else _make_id(base_name) - if base_nid not in seen_ids: - _add_node(base_nid, base_name, line) + same_file_nid = _make_id(stem, base_name) + if same_file_nid in seen_ids: + # Base class already declared earlier in this same file -- + # reuse its real node instead of the cross-file/stub lookup + # below (which assumes one-class-per-file and would create a + # duplicate node for a base class that shares this file). + base_nid = same_file_nid + else: + resolved = _pascal_resolve_class(path, base_name) + base_nid = resolved if resolved else _make_id(base_name) + if base_nid not in seen_ids: + _add_node(base_nid, base_name, line) _add_edge(cls_nid, base_nid, "inherits", line) # Find class body (up to next end;) @@ -13347,7 +13432,8 @@ def _lineno(text: str, offset: int) -> int: pos = end_m.end() if end_m else len(search_text) # Implementation headers (procedure/function/constructor/destructor) - impl_records: list[tuple[str, int, str]] = [] + impl_records: list[tuple[str, int, str, str, str]] = [] + # (proc_nid, line, body_text, container, name_lower) for fm in _PAS_IMPL_HEADER_RE.finditer(impl_text): qualified = fm.group("qual") line = _lineno(stripped, impl_off + fm.start()) @@ -13357,37 +13443,41 @@ def _lineno(text: str, offset: int) -> int: container = cls_nid if cls_nid in seen_ids else module_nid relation = "method" if cls_nid in seen_ids else "contains" label = f"{method_part}()" + name_lower = method_part.lower() else: container, relation = module_nid, "contains" label = f"{qualified}()" + name_lower = qualified.lower() proc_nid = _make_id(stem, qualified) _add_node(proc_nid, label, line) _add_edge(container, proc_nid, relation, line) body_start, body_end = _pascal_find_body(impl_text, fm.end()) body_text = impl_text[body_start:body_end] if body_start else "" - impl_records.append((proc_nid, line, body_text)) - - # Intra-file call edges - all_procs: dict[str, str] = { - n["label"].removesuffix("()").lower(): n["id"] - for n in nodes - if n["id"] != file_nid and n["label"].endswith("()") - } - for caller_nid, caller_line, body_text in impl_records: + impl_records.append((proc_nid, line, body_text, container, name_lower)) + + # Intra-file call edges, scoped by the caller's own class, then its + # ancestor chain (via `inherits` edges already emitted above), then + # file-level free functions; fall back to a global by-name match only + # when it is unambiguous (single owner across the file). Prevents + # same-named methods on unrelated classes (property accessors, generated + # wrapper classes such as TLB import units, etc. -- a common Pascal/Delphi + # pattern) from collapsing into an arbitrary cross-class edge. + callee_nid = _resolve_pascal_callee_factory(impl_records, edges, module_nid) + for caller_nid, caller_line, body_text, _container, _name_lower in impl_records: for cm in _PAS_CALL_RE.finditer(body_text): callee_name = cm.group(1).split(".")[-1].lower() if callee_name in _PAS_KEYWORDS: continue - callee_nid = all_procs.get(callee_name) - if not callee_nid or callee_nid == caller_nid: + target_nid = callee_nid(caller_nid, callee_name) + if not target_nid or target_nid == caller_nid: continue - pair = (caller_nid, callee_nid) + pair = (caller_nid, target_nid) if pair in seen_call_pairs: continue seen_call_pairs.add(pair) call_line = caller_line + body_text.count("\n", 0, cm.start()) - _add_edge(caller_nid, callee_nid, "calls", call_line, context="call") + _add_edge(caller_nid, target_nid, "calls", call_line, context="call") return {"nodes": nodes, "edges": edges, "input_tokens": 0, "output_tokens": 0} @@ -13433,7 +13523,8 @@ def extract_pascal(path: Path) -> dict: nodes: list[dict] = [] edges: list[dict] = [] seen_ids: set[str] = set() - proc_bodies: list[tuple[str, Any]] = [] + proc_bodies: list[tuple[str, Any, str, str]] = [] + # (proc_nid, body_node, container, name_lower) def _read(node) -> str: # type: ignore[no-untyped-def] return source[node.start_byte:node.end_byte].decode("utf-8", errors="replace") @@ -13567,7 +13658,7 @@ def walk(node, parent_nid: str) -> None: # type: ignore[no-untyped-def] line, ) if body_node: - proc_bodies.append((proc_nid, body_node)) + proc_bodies.append((proc_nid, body_node, container, label.removesuffix("()").lower())) return for child in node.children: @@ -13575,11 +13666,11 @@ def walk(node, parent_nid: str) -> None: # type: ignore[no-untyped-def] walk(root, file_nid) - # Second pass: resolve calls inside procedure/function bodies - all_procs: dict[str, str] = { - n["label"].removesuffix("()").lower(): n["id"] - for n in nodes if n["id"] != file_nid - } + # Second pass: resolve calls inside procedure/function bodies, scoped by + # the caller's own class, then its ancestor chain, then file-level free + # functions, falling back to an unambiguous global match (see + # _resolve_pascal_callee_factory). + resolve_callee = _resolve_pascal_callee_factory(proc_bodies, edges, module_nid) seen_call_pairs: set[tuple[str, str]] = set() def walk_calls(node, caller_nid: str) -> None: # type: ignore[no-untyped-def] @@ -13590,7 +13681,7 @@ def walk_calls(node, caller_nid: str) -> None: # type: ignore[no-untyped-def] callee_text = _read(child).split(".")[-1] break if callee_text: - callee_nid = all_procs.get(callee_text.lower()) + callee_nid = resolve_callee(caller_nid, callee_text.lower()) if callee_nid and callee_nid != caller_nid: pair = (caller_nid, callee_nid) if pair not in seen_call_pairs: @@ -13605,7 +13696,7 @@ def walk_calls(node, caller_nid: str) -> None: # type: ignore[no-untyped-def] named = [c for c in node.children if c.is_named] if len(named) == 1 and named[0].type == "identifier": callee_text = _read(named[0]) - callee_nid = all_procs.get(callee_text.lower()) + callee_nid = resolve_callee(caller_nid, callee_text.lower()) if callee_nid and callee_nid != caller_nid: pair = (caller_nid, callee_nid) if pair not in seen_call_pairs: @@ -13617,7 +13708,7 @@ def walk_calls(node, caller_nid: str) -> None: # type: ignore[no-untyped-def] for child in node.children: walk_calls(child, caller_nid) - for proc_nid, body_node in proc_bodies: + for proc_nid, body_node, _container, _name_lower in proc_bodies: walk_calls(body_node, proc_nid) return {"nodes": nodes, "edges": edges, "input_tokens": 0, "output_tokens": 0} diff --git a/tests/fixtures/sample_scoped_calls.pas b/tests/fixtures/sample_scoped_calls.pas new file mode 100644 index 000000000..66ebc53f4 --- /dev/null +++ b/tests/fixtures/sample_scoped_calls.pas @@ -0,0 +1,60 @@ +unit ScopedCallsUnit; + +interface + +type + TFirstWidget = class(TObject) + public + procedure Configure; + procedure Reset; + end; + + TSecondWidget = class(TObject) + public + procedure Configure; + procedure Reset; + end; + + TBaseWidget = class(TObject) + public + procedure Prepare; + end; + + TDerivedWidget = class(TBaseWidget) + public + procedure Run; + end; + +implementation + +procedure TFirstWidget.Configure; +begin + Reset; +end; + +procedure TFirstWidget.Reset; +begin + { first reset } +end; + +procedure TSecondWidget.Configure; +begin + Reset; +end; + +procedure TSecondWidget.Reset; +begin + { second reset } +end; + +procedure TBaseWidget.Prepare; +begin + { base prepare } +end; + +procedure TDerivedWidget.Run; +begin + Prepare; +end; + +end. diff --git a/tests/test_pascal_call_scoping.py b/tests/test_pascal_call_scoping.py new file mode 100644 index 000000000..3d47669a4 --- /dev/null +++ b/tests/test_pascal_call_scoping.py @@ -0,0 +1,102 @@ +"""Regression tests for scoped call resolution in the Pascal/Delphi extractor. + +Before this fix, both `extract_pascal` (tree-sitter path) and +`_extract_pascal_regex` (fallback path) resolved every call by a single +file-wide ``{method_name_lower: node_id}`` dict with no class scoping. Two +unrelated classes declaring a same-named method (a common Pascal/Delphi +pattern -- property accessors, generated wrapper classes such as TLB import +units) silently collapsed onto whichever declaration was inserted last, +producing wrong cross-class `calls` edges. See `sample_scoped_calls.pas`. +""" +from __future__ import annotations + +import pytest +from pathlib import Path + +FIXTURES = Path(__file__).parent / "fixtures" +FIXTURE_PATH = FIXTURES / "sample_scoped_calls.pas" + + +def _extractors(): + from graphify.extract import extract_pascal, _extract_pascal_regex + return [extract_pascal, _extract_pascal_regex] + + +def _class_node_id(r, class_label): + matches = [n["id"] for n in r["nodes"] if n["label"] == class_label] + assert len(matches) == 1, f"expected exactly one node labeled {class_label!r}, got {matches}" + return matches[0] + + +def _method_node_id(r, class_label, method_label): + class_id = _class_node_id(r, class_label) + node_by_id = {n["id"]: n for n in r["nodes"]} + for e in r["edges"]: + if e["relation"] == "method" and e["source"] == class_id: + node = node_by_id.get(e["target"]) + if node and node["label"] == method_label: + return node["id"] + raise AssertionError(f"no method edge {class_label}.{method_label} found") + + +def _has_call(r, src_id, tgt_id): + return any( + e["relation"] == "calls" and e["source"] == src_id and e["target"] == tgt_id + for e in r["edges"] + ) + + +@pytest.mark.parametrize("extract", [ + pytest.param(0, id="tree-sitter"), + pytest.param(1, id="regex-fallback"), +]) +def test_calls_scoped_to_own_class(extract): + r = _extractors()[extract](FIXTURE_PATH) + first_configure = _method_node_id(r, "TFirstWidget", "Configure()") + first_reset = _method_node_id(r, "TFirstWidget", "Reset()") + assert _has_call(r, first_configure, first_reset) + + +@pytest.mark.parametrize("extract", [ + pytest.param(0, id="tree-sitter"), + pytest.param(1, id="regex-fallback"), +]) +def test_calls_do_not_cross_unrelated_classes(extract): + r = _extractors()[extract](FIXTURE_PATH) + first_configure = _method_node_id(r, "TFirstWidget", "Configure()") + second_reset = _method_node_id(r, "TSecondWidget", "Reset()") + assert not _has_call(r, first_configure, second_reset), ( + "TFirstWidget.Configure must not resolve Reset() to the unrelated " + "TSecondWidget.Reset -- same-named methods on unrelated classes must " + "not collapse into a cross-class edge" + ) + + +@pytest.mark.parametrize("extract", [ + pytest.param(0, id="tree-sitter"), + pytest.param(1, id="regex-fallback"), +]) +def test_calls_scoped_other_direction(extract): + r = _extractors()[extract](FIXTURE_PATH) + second_configure = _method_node_id(r, "TSecondWidget", "Configure()") + second_reset = _method_node_id(r, "TSecondWidget", "Reset()") + first_reset = _method_node_id(r, "TFirstWidget", "Reset()") + assert _has_call(r, second_configure, second_reset) + assert not _has_call(r, second_configure, first_reset), ( + "TSecondWidget.Configure must not resolve Reset() to the unrelated " + "TFirstWidget.Reset" + ) + + +@pytest.mark.parametrize("extract", [ + pytest.param(0, id="tree-sitter"), + pytest.param(1, id="regex-fallback"), +]) +def test_calls_resolve_via_ancestor_chain(extract): + r = _extractors()[extract](FIXTURE_PATH) + derived_run = _method_node_id(r, "TDerivedWidget", "Run()") + base_prepare = _method_node_id(r, "TBaseWidget", "Prepare()") + assert _has_call(r, derived_run, base_prepare), ( + "TDerivedWidget.Run should resolve the inherited Prepare() to " + "TBaseWidget.Prepare via the inherits chain" + ) From 162e476b04c62792a6ff3cacc23916f4aa66a8f5 Mon Sep 17 00:00:00 2001 From: Gustavo Adolfo Moennich Date: Wed, 8 Jul 2026 18:41:59 -0300 Subject: [PATCH 2/2] fix(extract): resolve Pascal/Delphi calls to methods inherited across files The scoped call resolution added in the previous commit only sees a single file at a time (each Pascal/Delphi file is extracted independently), so a call from a manual descendant class to a method it inherits from an ancestor declared in a DIFFERENT file -- the common code-generator-base + manual-descendant split (e.g. Sistec's Th0Xxx/Th5Xxx) -- fell outside any one extraction's own scope and was silently dropped. Add graphify/pascal_resolution.py: a corpus-wide, post-extraction resolver (registered via resolver_registry, same mechanism as resolve_ruby_member_ calls) that walks the `inherits` chain across file boundaries using the full merged node/edge graph. It intentionally does NOT fall back to a global by-name match the way the per-file pass's last tier does -- walking `inherits` mirrors Delphi's actual method-lookup semantics, so it is a structurally justified resolution; guessing by name across an entire multi-thousand-file corpus is a different bet and stays out of scope here. extract_pascal and _extract_pascal_regex now report locally-unresolved calls via a `raw_calls` list (source_file, source_location, caller_nid, callee) instead of dropping them, following the same convention other languages already use for their own cross-file resolvers. Ownership (which class a raw call's caller belongs to) is looked up from the `method` edges in the final merged graph rather than carried as a separate field on the raw call -- a field the generic id-remap machinery that runs before resolvers would not know to keep in sync with the corpus's finalized ids. cache.py: raw_calls now goes through the same source_file relativize/ absolutize treatment as nodes/edges/hyperedges, so cached entries carrying unresolved calls stay portable across machines/checkout roots. Also fixes a related bug surfaced while testing the cross-file case: both Pascal extractors, on resolving a base class to another file via _pascal_resolve_class, still added a duplicate stub node for it (comment: "Stub for RTL/external/cross-file base classes") carrying the REFERENCING file's source_file instead of the base class's own. That duplicate collided with the base class's real node under cross-file id disambiguation, producing two different salted ids for what should be one class -- the `inherits` edge pointed at one id, the class's real `method` edges lived under the other, so no ancestor-chain walk could ever connect them. Stubs are now only created for the true external/unresolvable case (RTL classes etc.); a successfully resolved cross-file base reuses its real node's id. Tests: tests/test_pascal_resolution.py (4 tests) exercise the full extract() pipeline over static fixtures in tests/fixtures/pascal_cross_file/ (own-class call still resolves, cross-file inherited call resolves, an unrelated same-named method in a third file is not crossed into, and the resolver is registered). Verified these fail without this commit's changes (including a recurrence check confirming the duplicate-stub bug independently breaks the cross-file case even with the resolver present) and pass with them. Static fixtures are used instead of pytest's tmp_path because _pascal_project_root walks up looking for the highest ancestor with 2+ .pas files to find a "project root" for cross-file lookups; tmp_path lives under the shared system temp directory, which can already contain unrelated .pas files at some ancestor level on a real dev machine, escalating the search past the test's own directory. Re-validated against a real MTM/Delphi client-customization module (____M3_Customizacoes/M3FmPneus/H, 191 files): the generated/manual split (uh5*.pas extends uh0*.pas in _Gerados/) resolves across files as expected; 7 previously-invisible cross-file calls now resolve, all correctly (spot- checked). Lower than the file count might suggest -- this MTM-style framework leans more on override/dispatch semantics (already captured by the existing inherits+method edges) than on direct unqualified calls to inherited methods, so the biggest win here is precision and correctness on the calls that DO exist this way, not raw edge volume. No regressions: existing Pascal/Delphi/Lazarus suite (46 tests) and the full test suite (2849 passed, same 25 pre-existing unrelated failures in Terraform/Ollama/image-vision/install areas as before this change) both pass unchanged. --- graphify/cache.py | 10 +- graphify/extract.py | 119 ++++++++++++----- graphify/pascal_resolution.py | 120 ++++++++++++++++++ .../fixtures/pascal_cross_file/BaseGadget.pas | 18 +++ .../pascal_cross_file/DerivedGadget.pas | 21 +++ .../pascal_cross_file/OtherGadget.pas | 18 +++ tests/test_pascal_resolution.py | 95 ++++++++++++++ 7 files changed, 369 insertions(+), 32 deletions(-) create mode 100644 graphify/pascal_resolution.py create mode 100644 tests/fixtures/pascal_cross_file/BaseGadget.pas create mode 100644 tests/fixtures/pascal_cross_file/DerivedGadget.pas create mode 100644 tests/fixtures/pascal_cross_file/OtherGadget.pas create mode 100644 tests/test_pascal_resolution.py diff --git a/graphify/cache.py b/graphify/cache.py index 35dc7cc11..a71f07b18 100644 --- a/graphify/cache.py +++ b/graphify/cache.py @@ -276,7 +276,11 @@ def _relativize_source_files_in(payload: dict, root: Path) -> None: root_resolved = Path(root).resolve() except OSError: return - for bucket in ("nodes", "edges", "hyperedges"): + # raw_calls (#: Pascal/Delphi cross-file inherited-call resolution) carries + # source_file the same way nodes/edges/hyperedges do, so it needs the same + # portable-path treatment for cache entries to round-trip correctly across + # machines/checkout directories. + for bucket in ("nodes", "edges", "hyperedges", "raw_calls"): for item in payload.get(bucket, []): if not isinstance(item, dict): continue @@ -307,7 +311,7 @@ def _absolutize_source_files_in(payload: dict, root: Path) -> None: root_resolved = Path(root).resolve() except OSError: return - for bucket in ("nodes", "edges", "hyperedges"): + for bucket in ("nodes", "edges", "hyperedges", "raw_calls"): for item in payload.get(bucket, []): if not isinstance(item, dict): continue @@ -400,7 +404,7 @@ def save_cached(path: Path, result: dict, root: Path = Path("."), kind: str = "a # source_file field's original absolute form. Mutating the input here would # silently break those remaps on the first extraction pass. on_disk = result - if isinstance(result, dict) and any(result.get(k) for k in ("nodes", "edges", "hyperedges")): + if isinstance(result, dict) and any(result.get(k) for k in ("nodes", "edges", "hyperedges", "raw_calls")): import copy as _copy on_disk = _copy.deepcopy(result) _relativize_source_files_in(on_disk, root) diff --git a/graphify/extract.py b/graphify/extract.py index 22231af24..ad62e6810 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -20,6 +20,7 @@ run_language_resolvers, ) from .ruby_resolution import resolve_ruby_member_calls +from .pascal_resolution import resolve_pascal_inherited_calls # --- migrated to graphify/extractors/ (see graphify/extractors/MIGRATION.md) --- from graphify.extractors.base import ( # noqa: F401 @@ -12448,6 +12449,19 @@ def _key(label: str) -> str: register_language_resolver( LanguageResolver("csharp_member_calls", frozenset({".cs"}), _resolve_csharp_member_calls) ) +# Pascal/Delphi cross-file inherited-method-call resolution: a call from a +# manual descendant class to a method it inherits from an ancestor declared +# in a DIFFERENT file (the common generated-base/manual-descendant split, +# e.g. Sistec's Th0Xxx/Th5Xxx) falls outside the per-file extractor's own +# scope. Lives in graphify.pascal_resolution; registered here as a consumer +# of the framework, same as the Ruby resolver above. +register_language_resolver( + LanguageResolver( + "pascal_inherited_calls", + frozenset({".pas", ".pp", ".dpr", ".dpk", ".inc"}), + resolve_pascal_inherited_calls, + ) +) def extract_objc(path: Path) -> dict: @@ -13411,9 +13425,20 @@ def _lineno(text: str, offset: int) -> int: base_nid = same_file_nid else: resolved = _pascal_resolve_class(path, base_name) - base_nid = resolved if resolved else _make_id(base_name) - if base_nid not in seen_ids: - _add_node(base_nid, base_name, line) + if resolved: + # Cross-file base class found on disk -- its real node + # arrives via THAT file's own extraction. Do not add a + # duplicate stub here: it would carry this file's + # source_file (wrong -- it belongs to the base class's + # own file) and collide with the real node under + # cross-file id disambiguation, producing two different + # salted ids for what should be one class (breaks + # cross-file `inherits`-chain resolution downstream). + base_nid = resolved + else: + base_nid = _make_id(base_name) + if base_nid not in seen_ids: + _add_node(base_nid, base_name, line) _add_edge(cls_nid, base_nid, "inherits", line) # Find class body (up to next end;) @@ -13464,22 +13489,38 @@ def _lineno(text: str, offset: int) -> int: # wrapper classes such as TLB import units, etc. -- a common Pascal/Delphi # pattern) from collapsing into an arbitrary cross-class edge. callee_nid = _resolve_pascal_callee_factory(impl_records, edges, module_nid) + raw_calls: list[dict] = [] for caller_nid, caller_line, body_text, _container, _name_lower in impl_records: for cm in _PAS_CALL_RE.finditer(body_text): callee_name = cm.group(1).split(".")[-1].lower() if callee_name in _PAS_KEYWORDS: continue + call_line = caller_line + body_text.count("\n", 0, cm.start()) target_nid = callee_nid(caller_nid, callee_name) - if not target_nid or target_nid == caller_nid: + if target_nid == caller_nid: + continue + if not target_nid: + # Not resolvable within this file (e.g. inherited from a base + # class declared in another file) -- report for the + # cross-file resolver (graphify.pascal_resolution) instead of + # guessing or dropping it silently. + raw_calls.append({ + "source_file": str_path, + "source_location": f"L{call_line}", + "caller_nid": caller_nid, + "callee": callee_name, + }) continue pair = (caller_nid, target_nid) if pair in seen_call_pairs: continue seen_call_pairs.add(pair) - call_line = caller_line + body_text.count("\n", 0, cm.start()) _add_edge(caller_nid, target_nid, "calls", call_line, context="call") - return {"nodes": nodes, "edges": edges, "input_tokens": 0, "output_tokens": 0} + return { + "nodes": nodes, "edges": edges, "input_tokens": 0, "output_tokens": 0, + "raw_calls": raw_calls, + } def extract_pascal(path: Path) -> dict: @@ -13607,10 +13648,21 @@ def walk(node, parent_nid: str) -> None: # type: ignore[no-untyped-def] if base_nid not in seen_ids: # Try cross-file resolution (TFooBar → FooBar.pas) resolved = _pascal_resolve_class(path, base_name) - base_nid = resolved if resolved else _make_id(base_name) - if base_nid not in seen_ids: - # Stub for RTL/external/cross-file base classes - add_node(base_nid, base_name, line) + if resolved: + # Cross-file base class found on disk -- its + # real node arrives via THAT file's own + # extraction. Do not add a duplicate stub + # here: it would carry this file's + # source_file (wrong) and collide with the + # real node under cross-file id + # disambiguation, producing two different + # salted ids for what should be one class. + base_nid = resolved + else: + base_nid = _make_id(base_name) + if base_nid not in seen_ids: + # Stub for RTL/external base classes. + add_node(base_nid, base_name, line) add_edge(cls_nid, base_nid, "inherits", line) for child in kind_node.children: walk(child, cls_nid) @@ -13672,6 +13724,28 @@ def walk(node, parent_nid: str) -> None: # type: ignore[no-untyped-def] # _resolve_pascal_callee_factory). resolve_callee = _resolve_pascal_callee_factory(proc_bodies, edges, module_nid) seen_call_pairs: set[tuple[str, str]] = set() + raw_calls: list[dict] = [] + + def _emit_or_report(caller_nid: str, name_lower: str, line: int) -> None: + target = resolve_callee(caller_nid, name_lower) + if target == caller_nid: + return + if not target: + # Not resolvable within this file (e.g. inherited from a base + # class declared in another file) -- report for the cross-file + # resolver (graphify.pascal_resolution) instead of guessing or + # dropping it silently. + raw_calls.append({ + "source_file": str_path, + "source_location": f"L{line}", + "caller_nid": caller_nid, + "callee": name_lower, + }) + return + pair = (caller_nid, target) + if pair not in seen_call_pairs: + seen_call_pairs.add(pair) + add_edge(caller_nid, target, "calls", line, context="call") def walk_calls(node, caller_nid: str) -> None: # type: ignore[no-untyped-def] if node.type == "exprCall": @@ -13681,37 +13755,24 @@ def walk_calls(node, caller_nid: str) -> None: # type: ignore[no-untyped-def] callee_text = _read(child).split(".")[-1] break if callee_text: - callee_nid = resolve_callee(caller_nid, callee_text.lower()) - if callee_nid and callee_nid != caller_nid: - pair = (caller_nid, callee_nid) - if pair not in seen_call_pairs: - seen_call_pairs.add(pair) - add_edge( - caller_nid, callee_nid, "calls", - node.start_point[0] + 1, context="call", - ) + _emit_or_report(caller_nid, callee_text.lower(), node.start_point[0] + 1) elif node.type == "statement": # Pascal bare procedure calls with no args: `Reset;` # tree-sitter represents these as statement → identifier (no exprCall wrapper) named = [c for c in node.children if c.is_named] if len(named) == 1 and named[0].type == "identifier": callee_text = _read(named[0]) - callee_nid = resolve_callee(caller_nid, callee_text.lower()) - if callee_nid and callee_nid != caller_nid: - pair = (caller_nid, callee_nid) - if pair not in seen_call_pairs: - seen_call_pairs.add(pair) - add_edge( - caller_nid, callee_nid, "calls", - node.start_point[0] + 1, context="call", - ) + _emit_or_report(caller_nid, callee_text.lower(), node.start_point[0] + 1) for child in node.children: walk_calls(child, caller_nid) for proc_nid, body_node, _container, _name_lower in proc_bodies: walk_calls(body_node, proc_nid) - return {"nodes": nodes, "edges": edges, "input_tokens": 0, "output_tokens": 0} + return { + "nodes": nodes, "edges": edges, "input_tokens": 0, "output_tokens": 0, + "raw_calls": raw_calls, + } def extract_lazarus_form(path: Path) -> dict: diff --git a/graphify/pascal_resolution.py b/graphify/pascal_resolution.py new file mode 100644 index 000000000..8112db9fe --- /dev/null +++ b/graphify/pascal_resolution.py @@ -0,0 +1,120 @@ +"""Cross-file resolution for Pascal/Delphi calls to inherited methods. + +The per-file Pascal/Delphi extractors (``extract_pascal``, +``_extract_pascal_regex``) resolve calls to a method on the caller's own +class, its ancestor chain, or a file-level free function -- but only within +the single file being extracted (each file is extracted independently; see +``_resolve_pascal_callee_factory`` in ``extract.py``). Real Delphi/MTM-style +codebases very commonly split a class across two files -- a code-generator +base class (e.g. Sistec's ``Th0Xxx``) and a manual descendant that extends it +in a separate unit (``Th5Xxx``) -- so a call from the manual descendant to a +method it inherits from the generated base falls outside any one file's own +scope. The per-file pass reports these as ``raw_calls`` instead of guessing. + +This resolver runs after all files are extracted (registered in +``graphify.resolver_registry``), with the full merged node/edge corpus +available, so it can walk an ``inherits`` chain across file boundaries. It +intentionally does NOT fall back to a global by-name match the way the +per-file pass's final tier does -- an unqualified call resolving to a +specific ancestor mirrors Delphi's actual method-lookup semantics (nearest +ancestor in the chain), so walking `inherits` is a structurally justified +resolution, not a heuristic guess; guessing by name across an entire +multi-thousand-file corpus is not the same bet. +""" +from __future__ import annotations + +_PASCAL_SUFFIXES = (".pas", ".pp", ".dpr", ".dpk", ".inc") + + +def _pascal_raw_calls(per_file: list[dict]) -> list[dict]: + calls: list[dict] = [] + for result in per_file: + if not isinstance(result, dict): + continue + for rc in result.get("raw_calls", []): + if not isinstance(rc, dict): + continue + if str(rc.get("source_file", "")).endswith(_PASCAL_SUFFIXES): + calls.append(rc) + return calls + + +def resolve_pascal_inherited_calls( + per_file: list[dict], + all_nodes: list[dict], + all_edges: list[dict], +) -> None: + """Resolve Pascal/Delphi calls to a method inherited across file boundaries. + + Purely additive: only emits edges for raw calls the per-file pass could + not resolve locally (see module docstring). Each emission requires a + single owning class at the nearest matching level of the caller's + ``inherits`` chain (god-node guard, same principle as + ``resolve_ruby_member_calls``) -- an ambiguous or unresolved name + produces no edge rather than a guess. + """ + node_by_id = {n.get("id"): n for n in all_nodes} + + class_bases: dict[str, list[str]] = {} + # method_nid -> its owning class nid, so a raw call's caller_nid (a method + # or free-function nid) can be mapped to the CLASS whose inherits chain + # should be walked. Derived from `all_edges` (already remapped/finalized + # by the id-disambiguation passes that run before resolvers, same as + # caller_nid itself) rather than carried as a separate field on the raw + # call -- a field the generic id-remap machinery would not know to update. + owner_of: dict[str, str] = {} + class_procs: dict[str, dict[str, list[str]]] = {} + for e in all_edges: + if e.get("relation") == "inherits": + class_bases.setdefault(e["source"], []).append(e["target"]) + elif e.get("relation") == "method": + owner, method_nid = e.get("source"), e.get("target") + owner_of[method_nid] = owner + mnode = node_by_id.get(method_nid) + if mnode is None: + continue + name_lower = str(mnode.get("label", "")).removesuffix("()").lower() + class_procs.setdefault(owner, {}).setdefault(name_lower, []).append(method_nid) + + existing_pairs = {(e.get("source"), e.get("target")) for e in all_edges} + + def _resolve(owner: str, name_lower: str) -> str | None: + seen_bases: set[str] = set() + queue = list(class_bases.get(owner, [])) + while queue: + base = queue.pop(0) + if base in seen_bases: + continue + seen_bases.add(base) + candidates = class_procs.get(base, {}).get(name_lower) + if candidates: + return candidates[0] if len(candidates) == 1 else None + queue.extend(class_bases.get(base, [])) + return None + + for rc in _pascal_raw_calls(per_file): + caller = rc.get("caller_nid") + name_lower = rc.get("callee") + if not caller or not name_lower: + continue + owner = owner_of.get(caller) + if not owner: + continue + target = _resolve(str(owner), str(name_lower)) + if not target or target == caller: + continue + pair = (caller, target) + if pair in existing_pairs: + continue + existing_pairs.add(pair) + all_edges.append({ + "source": caller, + "target": target, + "relation": "calls", + "context": "call", + "confidence": "EXTRACTED", + "confidence_score": 1.0, + "source_file": rc.get("source_file", ""), + "source_location": rc.get("source_location"), + "weight": 1.0, + }) diff --git a/tests/fixtures/pascal_cross_file/BaseGadget.pas b/tests/fixtures/pascal_cross_file/BaseGadget.pas new file mode 100644 index 000000000..66116e6ab --- /dev/null +++ b/tests/fixtures/pascal_cross_file/BaseGadget.pas @@ -0,0 +1,18 @@ +unit BaseGadget; + +interface + +type + TBaseGadget = class(TObject) + public + procedure Prepare; + end; + +implementation + +procedure TBaseGadget.Prepare; +begin + { base prepare } +end; + +end. diff --git a/tests/fixtures/pascal_cross_file/DerivedGadget.pas b/tests/fixtures/pascal_cross_file/DerivedGadget.pas new file mode 100644 index 000000000..863870ced --- /dev/null +++ b/tests/fixtures/pascal_cross_file/DerivedGadget.pas @@ -0,0 +1,21 @@ +unit DerivedGadget; + +interface + +uses + BaseGadget; + +type + TDerivedGadget = class(TBaseGadget) + public + procedure Run; + end; + +implementation + +procedure TDerivedGadget.Run; +begin + Prepare; +end; + +end. diff --git a/tests/fixtures/pascal_cross_file/OtherGadget.pas b/tests/fixtures/pascal_cross_file/OtherGadget.pas new file mode 100644 index 000000000..d13dd0693 --- /dev/null +++ b/tests/fixtures/pascal_cross_file/OtherGadget.pas @@ -0,0 +1,18 @@ +unit OtherGadget; + +interface + +type + TOtherGadget = class(TObject) + public + procedure Prepare; + end; + +implementation + +procedure TOtherGadget.Prepare; +begin + { unrelated prepare } +end; + +end. diff --git a/tests/test_pascal_resolution.py b/tests/test_pascal_resolution.py new file mode 100644 index 000000000..f48401050 --- /dev/null +++ b/tests/test_pascal_resolution.py @@ -0,0 +1,95 @@ +"""Tests for cross-file Pascal/Delphi inherited-method-call resolution. + +The per-file Pascal/Delphi extractors resolve a call to the caller's own +class, its ancestor chain, or a file-level free function -- but only within +the single file being extracted. Real Delphi/MTM-style code very commonly +splits a class across two files (a generated base class + a manual +descendant that extends it in a separate unit), so a call from the +descendant to a method it inherits from the base falls outside any one +file's own scope. graphify.pascal_resolution closes that gap as a +corpus-wide, post-extraction pass. See its module docstring for the full +rationale. + +Uses static fixtures under tests/fixtures/pascal_cross_file/ rather than +pytest's tmp_path: the Pascal extractor's cross-file class lookup +(_pascal_project_root) walks UP the directory tree looking for the highest +ancestor with 2+ .pas files, to find the project root. tmp_path lives under +the shared system temp directory, which on a dev machine can easily already +contain 2+ stray .pas files at some ancestor level (other tools' scratch +files, other tests' leftover fixtures) -- the walk-up then escalates past the +test's own directory and picks up unrelated files. tests/fixtures/ has no +such siblings above it, so it is a stable project root for these tests. +""" +from __future__ import annotations + +from pathlib import Path + +from graphify.extract import extract, extract_pascal + +FIXTURES = Path(__file__).parent / "fixtures" / "pascal_cross_file" +BASE = FIXTURES / "BaseGadget.pas" +OTHER = FIXTURES / "OtherGadget.pas" +DERIVED = FIXTURES / "DerivedGadget.pas" + + +def _find_raw_call(result: dict, callee: str) -> dict | None: + for rc in result.get("raw_calls", []): + if rc.get("callee") == callee: + return rc + return None + + +def _labels(nodes: list[dict]) -> dict[str, str]: + return {n["id"]: str(n.get("label", "")) for n in nodes} + + +def _call_edge(graph: dict, src_label: str, tgt_label: str): + labels = _labels(graph["nodes"]) + for e in graph["edges"]: + if e.get("relation") != "calls": + continue + if labels.get(e.get("source")) == src_label and labels.get(e.get("target")) == tgt_label: + return e + return None + + +def test_single_file_extraction_reports_unresolved_inherited_call(): + """Sanity check for the gap this resolver closes: the per-file extractor + alone cannot see BaseGadget.pas while extracting DerivedGadget.pas, so it + must NOT emit a `calls` edge for Run -> Prepare, and must report it via + raw_calls instead of silently dropping it.""" + r = extract_pascal(DERIVED) + assert _call_edge(r, "Run()", "Prepare()") is None + rc = _find_raw_call(r, "prepare") + assert rc is not None + assert rc["caller_nid"] + + +def test_calls_resolve_across_files_via_inherits_chain(tmp_path): + # cache_root only controls where graphify-out/cache/ is written -- it has + # no bearing on the Pascal cross-file class lookup, which is keyed off + # each source path's own project root (see module docstring). Using + # tmp_path here just keeps cache artifacts out of the repo. + graph = extract([BASE, DERIVED], cache_root=tmp_path, parallel=False) + edge = _call_edge(graph, "Run()", "Prepare()") + assert edge is not None + assert edge.get("confidence") == "EXTRACTED" + + +def test_cross_file_calls_do_not_cross_unrelated_classes(tmp_path): + """TDerivedGadget inherits only from TBaseGadget. TOtherGadget declares an + unrelated same-named Prepare in a third file -- Run() must resolve to + TBaseGadget.Prepare, never to TOtherGadget.Prepare.""" + graph = extract([BASE, OTHER, DERIVED], cache_root=tmp_path, parallel=False) + edge = _call_edge(graph, "Run()", "Prepare()") + assert edge is not None + node_by_id = {n["id"]: n for n in graph["nodes"]} + target = node_by_id[edge["target"]] + assert "BaseGadget.pas" in target.get("source_file", "") + assert "OtherGadget.pas" not in target.get("source_file", "") + + +def test_pascal_resolver_registered(): + from graphify.resolver_registry import registered_resolvers + names = {r.name for r in registered_resolvers()} + assert "pascal_inherited_calls" in names