From 9e2387f3e894b4dfbc3e33c4bc637868f7984007 Mon Sep 17 00:00:00 2001 From: safishamsi Date: Fri, 14 Aug 2026 21:09:32 +0100 Subject: [PATCH] fix(ocaml): don't bind a qualified external call to a same-named local def A qualified call `M.f` where `M` is not a module defined in the same file is an external-library call (e.g. Hardcaml's `Reg_spec.create`, `Scope.create`). Resolving it by bare last name bound it to a same-named local `let f`, producing a false `calls` edge and a `create -> create` self-loop when the caller was that local `f`. Now: track locally-defined module names; a qualified call whose root module is not local and whose bare name collides with a local def is kept as a distinct external target (stub labelled by the full path), so it neither self-loops nor collapses onto the local def. Unqualified calls and calls into a locally-defined module still resolve locally, and cross-file `Geo.area` still collapses onto another file's `area`. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 6 ++++- graphify/extractors/ocaml.py | 52 +++++++++++++++++++++++++++++++----- tests/test_ocaml.py | 38 ++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8188748a..280e951f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,11 @@ Full release notes with details on each version: [GitHub Releases](https://github.com/safishamsi/graphify/releases) -## 0.9.43 (unreleased) +## 0.9.44 (unreleased) + +- Fix: an OCaml qualified call `M.f` to an external module (one not defined in the same file, e.g. Hardcaml's `Reg_spec.create`) no longer binds to a same-named local `let f` — which produced a false `calls` edge and, when the caller was that local `f`, a `f -> f` self-loop. External qualified calls are kept as a distinct target labelled by the full path; unqualified calls and calls into a locally-defined module still resolve locally, and cross-file `Geo.area` still collapses onto another file's `area`. + +## 0.9.43 (2026-08-14) - Feature: OCaml `.ml`/`.mli` extraction via tree-sitter-ocaml (optional `[ocaml]` extra). Extracts modules, top-level and module-level values/functions, types and their variant constructors, `open` imports, and function calls; qualified calls (`Geo.area`) resolve to the value, and cross-file `open`/call targets collapse onto the unique real definition via the corpus stub rewire. - Fix: a cross-file INFERRED `uses` edge now binds to the symbol whose body actually references the imported name (a module-level function is a valid source; a co-located class that never touches the import gets no edge), instead of fanning out from the import line to every class in the importing file (#2652, thanks @ousamabenyounes). A reference at module top level, with no enclosing symbol, emits no edge. diff --git a/graphify/extractors/ocaml.py b/graphify/extractors/ocaml.py index d1de21c9..ce735c13 100644 --- a/graphify/extractors/ocaml.py +++ b/graphify/extractors/ocaml.py @@ -42,9 +42,15 @@ def extract_ocaml(path: Path) -> dict: # once in the file is marked ambiguous and never resolved locally. local_defs: dict[str, str] = {} ambiguous: set[str] = set() - # (caller_nid, callee_name, line) recorded on pass 1, resolved on pass 2 so - # that forward references (e.g. `let rec ... and ...`) resolve correctly. - call_sites: list[tuple[str, str, int]] = [] + # Names of modules DEFINED in this file. Used to decide whether a qualified + # call `M.f` may bind to a local `f`: if `M` is not a local module it is an + # external library (e.g. `Reg_spec.create`), so binding to a same-named local + # `f` would be a false edge (and a self-loop when the caller is that `f`). + local_modules: set[str] = set() + # (caller_nid, callee_name, qualifier_root_module_or_None, full_path_text, + # line) recorded on pass 1, resolved on pass 2 so that forward references + # (e.g. `let rec ... and ...`) resolve correctly. + call_sites: list[tuple[str, str, str | None, str, int]] = [] def add_node(nid: str, label: str, line: int) -> None: if nid not in seen_ids: @@ -112,6 +118,23 @@ def extract_ocaml(path: Path) -> dict: found = _read_text(n, source) return found + def path_root_module(path_node) -> str | None: + """Leftmost (outermost) module segment qualifying a *_path node: + `Reg_spec.create` -> `Reg_spec`, `Stdlib.List.map` -> `Stdlib`. Returns + None when the path is unqualified (`create`), which has no child + module_path.""" + mp = next((c for c in path_node.children if c.type == "module_path"), None) + if mp is None: + return None + node = mp + while True: + inner = next((c for c in node.children if c.type == "module_path"), None) + if inner is None: + break + node = inner + mn = next((c for c in node.children if c.type == "module_name"), None) + return _read_text(mn, source) if mn is not None else None + def register_def(name: str, nid: str) -> None: if name in ambiguous: return @@ -166,6 +189,7 @@ def extract_ocaml(path: Path) -> dict: add_edge(container_nid, mnid, "defines" if container_nid == file_nid else "contains", line) register_def(mname, mnid) + local_modules.add(mname) for child in binding.children: walk(child, mnid, enclosing_value) return @@ -179,6 +203,7 @@ def extract_ocaml(path: Path) -> dict: add_edge(container_nid, mnid, "defines" if container_nid == file_nid else "contains", line) register_def(mname, mnid) + local_modules.add(mname) for child in node.children: walk(child, mnid, enclosing_value) return @@ -232,7 +257,8 @@ def extract_ocaml(path: Path) -> dict: callee = last_name(fn) if callee: caller = enclosing_value if enclosing_value else file_nid - call_sites.append((caller, callee, line_of(node))) + call_sites.append((caller, callee, path_root_module(fn), + _read_text(fn, source), line_of(node))) # Fall through: arguments may contain further applications/definitions. for child in node.children: @@ -240,8 +266,22 @@ def extract_ocaml(path: Path) -> dict: walk(root, file_nid, "") - for caller, callee, line in call_sites: - if callee in local_defs: + for caller, callee, qualifier, full_path, line in call_sites: + # A qualified call `M.f` where `M` is NOT a module defined in this file + # is an external-library call (e.g. `Reg_spec.create`). Binding it to a + # same-named local `f` would be a false edge (and a `create -> create` + # self-loop when the caller is that local `f`), so keep it distinct: a + # stub keyed by the FULL qualified name (`Reg_spec.create`) never + # collapses onto the local `f` in the corpus rewire. Unqualified calls, + # and qualified calls into a locally-defined module, still resolve to a + # local definition; a bare-name stub still allows the cross-file rewire + # to collapse `Geo.area` onto another file's `area` (#hardcaml). + if qualifier is not None and qualifier not in local_modules: + if callee in local_defs: + add_edge(caller, ref_stub(full_path), "calls", line, confidence="INFERRED") + else: + add_edge(caller, ref_stub(callee), "calls", line, confidence="INFERRED") + elif callee in local_defs: add_edge(caller, local_defs[callee], "calls", line) else: add_edge(caller, ref_stub(callee), "calls", line, confidence="INFERRED") diff --git a/tests/test_ocaml.py b/tests/test_ocaml.py index 336693be..275951e6 100644 --- a/tests/test_ocaml.py +++ b/tests/test_ocaml.py @@ -86,6 +86,44 @@ def test_impl_calls_resolve_same_file(tmp_path): assert ("main", "Shapes") not in calls +def test_qualified_external_call_does_not_bind_to_local_same_name(tmp_path): + """A qualified call `M.f` to an EXTERNAL module (not defined in this file) + must not bind to a same-named local `f` — that would be a false edge and, + when the caller is that local `f`, a `f -> f` self-loop. It is kept as a + distinct external target labelled by the full path (e.g. Hardcaml's + `Reg_spec.create` next to a local `let create`).""" + src = ( + "let create x =\n" + " let spec = Reg_spec.create x in\n" # external, same bare name as local + " spec\n" + "let run () = Scope.create ()\n" # external, same bare name + ) + r = extract_ocaml(_write(tmp_path, "counter.ml", src)) + calls = _rel_pairs(r, "calls") + assert ("create", "create") not in calls # no self-loop + assert ("create", "Reg_spec.create") in calls # kept distinct + assert ("run", "Scope.create") in calls + assert ("run", "create") not in calls # not the local create + # no dangling edges introduced by the qualified external stubs + ids = {n["id"] for n in r["nodes"]} + assert all(e["source"] in ids and e["target"] in ids for e in r["edges"]) + + +def test_qualified_call_into_local_module_resolves(tmp_path): + """A qualified call whose qualifier IS a module defined in this file still + resolves to the local definition.""" + src = ( + "module M = struct\n" + " let helper x = x\n" + "end\n" + "let run () = M.helper 1\n" + ) + r = extract_ocaml(_write(tmp_path, "m.ml", src)) + calls = _rel_pairs(r, "calls") + assert ("run", "helper") in calls + assert not any(t == "M.helper" for _, t in calls) # not left as an external stub + + def test_impl_open_emits_import(tmp_path): r = extract_ocaml(_write(tmp_path, "shapes.ml", IMPL)) imports = _rel_pairs(r, "imports_from")