From 27746d6d61885a226d1da6df656fc79ab1d79de3 Mon Sep 17 00:00:00 2001 From: xiongjianxu <6457197+xiongjianxu@users.noreply.github.com> Date: Mon, 7 Sep 2026 20:45:52 +0800 Subject: [PATCH] fix(go): fold a package's type into one node when its methods span files (#3399) extract_go keys a type on its package directory, so every file declaring a method on it mints the type again under the same id. Disambiguation then splits those apart by path, and the type ends up fragmented into several partial nodes, each owning some of its methods: the declaring file's node owns none of them, and every single-definition guard downstream reads the set as an ambiguity. Fold the id-collision before disambiguation, the same point and the same shape as _merge_decl_def_classes for C/C++/ObjC. Guards: all members must be .go files in one directory (the id folds in only the directory's name), and a _test.go member stops the fold, since its package clause may be a separate external test package. The declaring file wins; no edge re-pointing is needed because the group already shares one id. --- graphify/extract.py | 5 ++ graphify/extractors/resolution.py | 73 ++++++++++++++++++++ tests/test_go_package_type_fold.py | 106 +++++++++++++++++++++++++++++ 3 files changed, 184 insertions(+) create mode 100644 tests/test_go_package_type_fold.py diff --git a/graphify/extract.py b/graphify/extract.py index 7cc9e62c90..2c7f14f049 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -103,6 +103,7 @@ _load_workspace_packages, _match_tsconfig_alias, _merge_decl_def_classes, + _merge_go_package_types, _node_disambiguation_source_key, _package_entry_candidates, _parse_js_tree, @@ -6466,6 +6467,10 @@ def _describe_syntax_error(rel: str, line: "int | None", kept: int) -> str: # also means disambiguation sees one source_file per id and won't split them. _merge_decl_def_classes(all_nodes, all_edges) + # Same fold for a Go package's type, which every file declaring a method on it mints + # again under one id; also before disambiguation, which would otherwise split them. + _merge_go_package_types(all_nodes, all_edges) + # Remap file node IDs from absolute-path-derived to the canonical # {parent_dir}_{stem} spec form so (a) graph.json edge endpoints are stable # across machines (#502) and (b) AST file nodes match the IDs semantic diff --git a/graphify/extractors/resolution.py b/graphify/extractors/resolution.py index ed9e857e87..a1cb4fedb8 100644 --- a/graphify/extractors/resolution.py +++ b/graphify/extractors/resolution.py @@ -2432,6 +2432,79 @@ def _merge_decl_def_classes( rewritten.append(e) all_edges[:] = rewritten +def _merge_go_package_types( + all_nodes: list[dict], + all_edges: list[dict], +) -> None: + """Merge a Go package's type declared in one file with the copies minted by the + other files that declare methods on it into ONE node (#3399). + + ``extract_go`` keys a type on its package directory, so ``type Server`` in ``a.go`` + and ``func (s *Server) Close()`` in ``b.go`` mint the SAME id twice, differing only + in ``source_file``. Left alone, ``_disambiguate_colliding_node_ids`` splits them + apart by path, fragmenting one type into several partial nodes that each own some of + its methods — the declaring file's node owning none of them — which every + single-definition god-node guard downstream then reads as an ambiguity and bails on. + This is the Go analogue of ``_merge_decl_def_classes`` and runs at the same point, + before disambiguation. + + GOD-NODE GUARDS: + + * Every node in the id-collision must be a ``.go`` file in ONE directory — Go's + package unit. The id folds in only the directory's NAME, so ``a/svc`` and + ``b/svc`` collide on id while being different packages; those stay split. + * Folding stops at a ``_test.go`` member, whose package clause may be the separate + ``svc_test`` package declaring a type of its own name. The clause is not parsed, + so same-directory is not evidence of same package there. + + No edge re-pointing is needed: the group already shares one id, so every edge + already points at the survivor. The declaration site wins — the file holding + ``type X``, the only one with a ``contains`` edge to it — and the dropped nodes' + methods keep their own ``source_file``, so no location is lost with them. + """ + by_id: dict[str, list[dict]] = {} + for n in all_nodes: + nid = n.get("id") + label = str(n.get("label", "")) + if not isinstance(nid, str) or not nid or n.get("file_type") != "code": + continue + # A Go type name holds neither character, while every other label the extractor + # mints does: `Run()`, `.Close()`, and the file node's own `a.go`. + if not label or "." in label or "(" in label: + continue + if Path(str(n.get("source_file", ""))).suffix.lower() != ".go": + continue + by_id.setdefault(nid, []).append(n) + + declaring_files: dict[str, set[str]] = {} + for e in all_edges: + if e.get("relation") == "contains": + declaring_files.setdefault(str(e.get("target", "")), set()).add( + str(e.get("source_file", ""))) + + drop_objs: set[int] = set() + for nid, group in by_id.items(): + if len(group) < 2: + continue + files = [Path(str(n.get("source_file", ""))) for n in group] + if len({f.parent for f in files}) != 1: + continue + if any(f.name.endswith("_test.go") for f in files): + continue + # A type whose declaring file is outside the corpus leaves the whole group + # method-only; one node is still the answer, so fold on the lowest path. + declared = declaring_files.get(nid, set()) + candidates = [n for n in group if str(n.get("source_file", "")) in declared] or group + keeper = min(candidates, key=lambda n: (str(n.get("source_file", "")), + str(n.get("source_location", "")))) + for node in group: + if node is not keeper: + drop_objs.add(id(node)) + + if drop_objs: + all_nodes[:] = [n for n in all_nodes if id(n) not in drop_objs] + + def _resolve_cross_file_java_imports( per_file: list[dict], paths: list[Path], diff --git a/tests/test_go_package_type_fold.py b/tests/test_go_package_type_fold.py new file mode 100644 index 0000000000..0a93886cee --- /dev/null +++ b/tests/test_go_package_type_fold.py @@ -0,0 +1,106 @@ +"""A Go package's type is one node even when its methods are spread across files. + +``extract_go`` keys a type on its package directory, so every file declaring a method on +it mints the type again under the same id, and disambiguation then split those apart by +path — leaving one type fragmented into several partial nodes, each owning some of its +methods, which every single-definition guard downstream reads as an ambiguity. The fold +runs before disambiguation; the negative cases pin what must stay split, since the id +folds in the directory's name rather than its path and the package clause is not parsed. +""" +from __future__ import annotations + +import importlib +from pathlib import Path + +import pytest + +from graphify.extract import extract + +pytestmark = pytest.mark.skipif( + importlib.util.find_spec("tree_sitter_go") is None, + reason="tree_sitter_go not installed", +) + + +def _extract(tmp_path, files: dict[str, str]): + for name, body in files.items(): + path = tmp_path / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(body, encoding="utf-8") + return extract([tmp_path / n for n in files], + cache_root=tmp_path / "graphify-out", parallel=False) + + +def _nodes(result, label: str) -> list[dict]: + return [n for n in result["nodes"] if n["label"] == label] + + +def _edges_from(result, node_id: str, relation: str) -> set[str]: + label = {n["id"]: n["label"] for n in result["nodes"]} + return {label.get(e["target"], e["target"]) for e in result["edges"] + if e["relation"] == relation and e["source"] == node_id} + + +def test_a_type_whose_methods_live_in_other_files_owns_all_of_them(tmp_path): + result = _extract(tmp_path, { + "svc/a.go": ("package svc\n\ntype Server struct{}\n\n" + "func Run(srv *Server) { srv.Close() }\n"), + "svc/b.go": "package svc\n\nfunc (s *Server) Close() {}\n", + "svc/c.go": "package svc\n\nfunc (s *Server) Save() {}\n", + }) + servers = _nodes(result, "Server") + assert len(servers) == 1, [n["id"] for n in servers] + assert Path(str(servers[0]["source_file"])).name == "a.go" + assert _edges_from(result, servers[0]["id"], "method") == {".Close()", ".Save()"} + + +def test_a_reference_to_the_type_reaches_the_node_that_owns_the_methods(tmp_path): + result = _extract(tmp_path, { + "svc/a.go": "package svc\n\ntype Server struct{}\n", + "svc/b.go": "package svc\n\nfunc (s *Server) Close() {}\n", + "svc/c.go": "package svc\n\nfunc Run(srv *Server) {}\n", + }) + servers = _nodes(result, "Server") + assert len(servers) == 1, [n["id"] for n in servers] + refs = [e for e in result["edges"] + if e["relation"] == "references" and e["target"] == servers[0]["id"]] + assert refs, "the parameter type must reference the one Server node" + assert _edges_from(result, servers[0]["id"], "method") == {".Close()"} + + +def test_the_declaring_file_survives_a_lower_sorting_method_file(tmp_path): + result = _extract(tmp_path, { + "svc/a.go": "package svc\n\nfunc (s *Server) Close() {}\n", + "svc/z.go": "package svc\n\ntype Server struct{}\n", + }) + servers = _nodes(result, "Server") + assert len(servers) == 1, [n["id"] for n in servers] + assert Path(str(servers[0]["source_file"])).name == "z.go" + + +def test_two_packages_sharing_a_directory_name_stay_split(tmp_path): + result = _extract(tmp_path, { + "a/svc/x.go": "package svc\n\ntype Server struct{}\n\nfunc (s *Server) Save() {}\n", + "b/svc/y.go": "package svc\n\nfunc (s *Server) Close() {}\n", + }) + assert len(_nodes(result, "Server")) == 2 + + +def test_an_external_test_package_in_the_same_directory_stays_split(tmp_path): + result = _extract(tmp_path, { + "svc/a.go": "package svc\n\ntype Fixture struct{}\n\nfunc (f *Fixture) Save() {}\n", + "svc/a_test.go": ("package svc_test\n\ntype Fixture struct{}\n\n" + "func (f *Fixture) Reset() {}\n"), + }) + assert len(_nodes(result, "Fixture")) == 2 + + +def test_a_function_and_a_method_of_the_same_name_are_untouched(tmp_path): + # The fold only ever considers bare type labels, so a package whose helper and method + # share a name keeps both nodes. + result = _extract(tmp_path, { + "svc/a.go": "package svc\n\ntype Server struct{}\n\nfunc Close() {}\n", + "svc/b.go": "package svc\n\nfunc (s *Server) Close() {}\n", + }) + assert len(_nodes(result, "Close()")) == 1 + assert len(_nodes(result, ".Close()")) == 1