mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-26 16:26:42 +00:00
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
7281f27eac
commit
9e2387f3e8
+5
-1
@@ -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.
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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")
|
||||
|
||||
Reference in New Issue
Block a user