diff --git a/graphify/extractors/go.py b/graphify/extractors/go.py index b2483db1..d8f203b7 100644 --- a/graphify/extractors/go.py +++ b/graphify/extractors/go.py @@ -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] = {} diff --git a/tests/test_go_qualified_resolution.py b/tests/test_go_qualified_resolution.py index 8a0d367f..dd5bd0ac 100644 --- a/tests/test_go_qualified_resolution.py +++ b/tests/test_go_qualified_resolution.py @@ -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}" + )