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