mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-26 16:26:42 +00:00
fix(go): keep both halves of a case-only symbol collision (#2779)
Node ids casefold (deliberately, for AST/LLM/builder parity), but Go is case-sensitive and uses case for visibility: exported `Run` and unexported `run` in one file collapsed to one id, so the second was dropped by add_node and a local `run()` call phantomed to a same-named symbol in another package. The Go extractor now salts the non-canonical half of a same-file case-only collision (exported keeps the stable plain id), so both survive and the in-file resolver binds the unexported call locally. ids.py / normalize_id are untouched, so the id contract holds and non-Go corpora are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
safishamsi
co-authored by
Claude Opus 4.8
parent
7addee1159
commit
1e68a7aa76
@@ -2,6 +2,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
|
||||
import hashlib
|
||||
from pathlib import Path
|
||||
from graphify.extractors.base import _LANGUAGE_BUILTIN_GLOBALS, _file_stem, _make_id, _read_text
|
||||
|
||||
@@ -206,6 +207,68 @@ def extract_go(path: Path) -> dict:
|
||||
if tgt != func_nid:
|
||||
add_edge(func_nid, tgt, "references", line, context=ctx)
|
||||
|
||||
# Node IDs are casefolded (ids.py), so `Run` and `run` declared in one file
|
||||
# produce the same id and add_node silently dropped the second — the unexported
|
||||
# half vanished from the graph and its call sites bound by bare name to a
|
||||
# same-named function in another package, which Go's visibility rules make
|
||||
# impossible (#2779). Salt the non-canonical member of a case-only collision
|
||||
# so both survive.
|
||||
#
|
||||
# The EXPORTED member keeps the plain id. Only exported symbols are reachable
|
||||
# across packages, so cross-package edges (and edges cached in graph.json from
|
||||
# files an incremental rebuild does not touch) target the exported one — keeping
|
||||
# its id stable means adding/removing an unexported sibling in an update never
|
||||
# re-points them. Docs likewise reference the exported API, so the casefolded id
|
||||
# a semantic node produces lands on the symbol it actually describes. Calls to
|
||||
# the unexported sibling can only come from the same package and only resolve
|
||||
# once the sibling exists, so salting it re-points nothing. When the collision
|
||||
# has no unique exported member (`Run`/`RUN`), every member is salted rather
|
||||
# than picking one arbitrarily, so the result never depends on declaration order.
|
||||
case_groups: dict[str, set[str]] = {}
|
||||
|
||||
def _receiver_type_of(node) -> str | None:
|
||||
receiver = node.child_by_field_name("receiver")
|
||||
if not receiver:
|
||||
return None
|
||||
for param in receiver.children:
|
||||
if param.type == "parameter_declaration":
|
||||
type_node = param.child_by_field_name("type")
|
||||
if type_node:
|
||||
return _read_text(type_node, source).lstrip("*").strip()
|
||||
break
|
||||
return None
|
||||
|
||||
def _plain_symbol_nid(node) -> tuple[str, str] | None:
|
||||
name_node = node.child_by_field_name("name")
|
||||
if not name_node:
|
||||
return None
|
||||
name = _read_text(name_node, source)
|
||||
if node.type == "method_declaration":
|
||||
receiver_type = _receiver_type_of(node)
|
||||
base = _make_id(pkg_scope, receiver_type) if receiver_type else stem
|
||||
else:
|
||||
base = stem
|
||||
return _make_id(base, name), name
|
||||
|
||||
def _scan_declarations(node) -> None:
|
||||
if node.type in ("function_declaration", "method_declaration"):
|
||||
found = _plain_symbol_nid(node)
|
||||
if found:
|
||||
case_groups.setdefault(found[0], set()).add(found[1])
|
||||
return
|
||||
for child in node.children:
|
||||
_scan_declarations(child)
|
||||
|
||||
def symbol_nid(plain_nid: str, name: str) -> str:
|
||||
names = case_groups.get(plain_nid) or set()
|
||||
if len(names) < 2:
|
||||
return plain_nid
|
||||
exported = [n for n in names if n[:1].isupper()]
|
||||
if len(exported) == 1 and name == exported[0]:
|
||||
return plain_nid
|
||||
salt = hashlib.sha1(name.encode("utf-8"), usedforsecurity=False).hexdigest()[:6]
|
||||
return _make_id(plain_nid, salt)
|
||||
|
||||
def walk(node) -> None:
|
||||
t = node.type
|
||||
|
||||
@@ -214,7 +277,7 @@ def extract_go(path: Path) -> dict:
|
||||
if name_node:
|
||||
func_name = _read_text(name_node, source)
|
||||
line = node.start_point[0] + 1
|
||||
func_nid = _make_id(stem, func_name)
|
||||
func_nid = symbol_nid(_make_id(stem, func_name), func_name)
|
||||
add_node(func_nid, f"{func_name}()", line)
|
||||
add_edge(file_nid, func_nid, "contains", line)
|
||||
emit_go_method_refs(node, func_nid, line)
|
||||
@@ -242,11 +305,11 @@ def extract_go(path: Path) -> dict:
|
||||
if receiver_type:
|
||||
parent_nid = _make_id(pkg_scope, receiver_type)
|
||||
add_node(parent_nid, receiver_type, line)
|
||||
method_nid = _make_id(parent_nid, method_name)
|
||||
method_nid = symbol_nid(_make_id(parent_nid, method_name), method_name)
|
||||
add_node(method_nid, f".{method_name}()", line)
|
||||
add_edge(parent_nid, method_nid, "method", line)
|
||||
else:
|
||||
method_nid = _make_id(stem, method_name)
|
||||
method_nid = symbol_nid(_make_id(stem, method_name), method_name)
|
||||
add_node(method_nid, f"{method_name}()", line)
|
||||
add_edge(file_nid, method_nid, "contains", line)
|
||||
|
||||
@@ -357,6 +420,7 @@ def extract_go(path: Path) -> dict:
|
||||
for child in node.children:
|
||||
walk(child)
|
||||
|
||||
_scan_declarations(root)
|
||||
walk(root)
|
||||
|
||||
label_to_nid: dict[str, str] = {}
|
||||
|
||||
@@ -170,3 +170,151 @@ def test_incremental_qualified_resolution_uses_unchanged_context(tmp_path: Path)
|
||||
and edge.get("target") in full_user_ids
|
||||
for edge in changed["edges"]
|
||||
)
|
||||
|
||||
|
||||
def _case_only_sibling_corpus(tmp_path: Path) -> None:
|
||||
"""Exported wrapper + unexported worker of the same name, plus a decoy."""
|
||||
(tmp_path / "go.mod").write_text("module example.com/repro\n\ngo 1.22\n")
|
||||
pkg_a = tmp_path / "pkga"
|
||||
pkg_a.mkdir()
|
||||
(pkg_a / "a.go").write_text(
|
||||
"package pkga\n"
|
||||
"\n"
|
||||
"func Run() error {\n"
|
||||
"\treturn run(1)\n"
|
||||
"}\n"
|
||||
"\n"
|
||||
"func run(n int) error {\n"
|
||||
"\treturn nil\n"
|
||||
"}\n"
|
||||
)
|
||||
pkg_b = tmp_path / "pkgb"
|
||||
pkg_b.mkdir()
|
||||
(pkg_b / "b.go").write_text(
|
||||
"package pkgb\n\nfunc run(s string) error {\n\treturn nil\n}\n"
|
||||
)
|
||||
|
||||
|
||||
def test_case_only_sibling_functions_are_both_extracted(tmp_path: Path) -> None:
|
||||
"""``Run`` and ``run`` in one file are two symbols, not one."""
|
||||
_case_only_sibling_corpus(tmp_path)
|
||||
result = _extract(tmp_path)
|
||||
|
||||
exported = _ids(result, label="Run", suffix="a.go")
|
||||
unexported = _ids(result, label="run", suffix="a.go")
|
||||
assert len(exported) == 1, f"exported Run missing: {exported}"
|
||||
assert len(unexported) == 1, f"unexported run missing: {unexported}"
|
||||
assert exported != unexported, "Run and run collapsed onto one node id"
|
||||
|
||||
|
||||
def test_case_only_sibling_does_not_bind_to_another_package(tmp_path: Path) -> None:
|
||||
"""``Run`` calls its own file's ``run``, never another package's."""
|
||||
_case_only_sibling_corpus(tmp_path)
|
||||
result = _extract(tmp_path)
|
||||
|
||||
exported = _ids(result, label="Run", suffix="a.go")
|
||||
foreign = _ids(result, label="run", suffix="b.go")
|
||||
phantom = [
|
||||
edge
|
||||
for edge in result["edges"]
|
||||
if edge.get("relation") == "calls"
|
||||
and edge.get("source") in exported
|
||||
and edge.get("target") in foreign
|
||||
]
|
||||
assert phantom == [], f"Run bound to another package's unexported run: {phantom}"
|
||||
|
||||
|
||||
def test_case_only_sibling_exported_keeps_stable_id(tmp_path: Path) -> None:
|
||||
"""The exported member keeps the plain id whether or not a sibling exists.
|
||||
|
||||
Cross-package edges and graph.json entries from files an incremental rebuild
|
||||
does not touch all target the exported symbol, so its id must not depend on
|
||||
the presence of an unexported case-only sibling.
|
||||
"""
|
||||
(tmp_path / "go.mod").write_text("module example.com/repro\n\ngo 1.22\n")
|
||||
pkg = tmp_path / "pkga"
|
||||
pkg.mkdir()
|
||||
solo = "package pkga\n\nfunc Run() error {\n\treturn nil\n}\n"
|
||||
with_sibling = (
|
||||
"package pkga\n"
|
||||
"\n"
|
||||
"func Run() error {\n"
|
||||
"\treturn run(1)\n"
|
||||
"}\n"
|
||||
"\n"
|
||||
"func run(n int) error {\n"
|
||||
"\treturn nil\n"
|
||||
"}\n"
|
||||
)
|
||||
|
||||
(pkg / "a.go").write_text(solo)
|
||||
solo_ids = _ids(_extract(tmp_path), label="Run", suffix="a.go")
|
||||
|
||||
(pkg / "a.go").write_text(with_sibling)
|
||||
result = _extract(tmp_path)
|
||||
exported = _ids(result, label="Run", suffix="a.go")
|
||||
unexported = _ids(result, label="run", suffix="a.go")
|
||||
|
||||
assert exported == solo_ids, (
|
||||
f"adding an unexported sibling moved the exported id: {solo_ids} -> {exported}"
|
||||
)
|
||||
assert unexported and unexported != exported
|
||||
|
||||
|
||||
def test_incremental_sibling_addition_keeps_cross_package_edge(tmp_path: Path) -> None:
|
||||
"""A hook-style partial rebuild must not re-point edges from untouched files.
|
||||
|
||||
Build the full corpus, then add an unexported ``run`` sibling and re-extract
|
||||
ONLY a.go (the --update path). The stored ``Start -> Run`` edge from the
|
||||
untouched app package has to keep pointing at the exported ``Run``.
|
||||
"""
|
||||
from graphify.build import build_from_json, build_merge
|
||||
from graphify.export import to_json
|
||||
|
||||
(tmp_path / "go.mod").write_text("module example.com/repro\n\ngo 1.22\n")
|
||||
(tmp_path / "pkga").mkdir()
|
||||
(tmp_path / "app").mkdir()
|
||||
(tmp_path / "pkga" / "a.go").write_text(
|
||||
"package pkga\n\nfunc Run() error {\n\treturn nil\n}\n"
|
||||
)
|
||||
(tmp_path / "app" / "app.go").write_text(
|
||||
'package app\n\nimport "example.com/repro/pkga"\n\n'
|
||||
"func Start() error {\n\treturn pkga.Run()\n}\n"
|
||||
)
|
||||
|
||||
full = _extract(tmp_path)
|
||||
graph = build_from_json(full, root=str(tmp_path), directed=False)
|
||||
graph_path = tmp_path / "graph.json"
|
||||
to_json(graph, {i: [n] for i, n in enumerate(graph.nodes)}, str(graph_path))
|
||||
|
||||
(tmp_path / "pkga" / "a.go").write_text(
|
||||
"package pkga\n"
|
||||
"\n"
|
||||
"func Run() error {\n"
|
||||
"\treturn run(1)\n"
|
||||
"}\n"
|
||||
"\n"
|
||||
"func run(n int) error {\n"
|
||||
"\treturn nil\n"
|
||||
"}\n"
|
||||
)
|
||||
partial = extract(
|
||||
[tmp_path / "pkga" / "a.go"],
|
||||
cache_root=tmp_path,
|
||||
root=tmp_path,
|
||||
parallel=False,
|
||||
)
|
||||
merged = build_merge(
|
||||
[partial], graph_path=str(graph_path), root=str(tmp_path), directed=False
|
||||
)
|
||||
|
||||
# The merged graph is undirected; edge iteration order is arbitrary. The
|
||||
# build stores the real direction in _src/_tgt, so read those.
|
||||
start_targets = {
|
||||
merged.nodes[d.get("_tgt", v)].get("label")
|
||||
for u, v, d in merged.edges(data=True)
|
||||
if d.get("relation") == "calls" and "start" in str(d.get("_src", u))
|
||||
}
|
||||
assert start_targets == {"Run()"}, (
|
||||
f"cross-package edge re-pointed after partial rebuild: {start_targets}"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user