-
-
Notifications
You must be signed in to change notification settings - Fork 11.2k
Go: fold a package's type into one node when its methods span files (#3399) - #3400
Go: fold a package's type into one node when its methods span files (#3399) #3400xiongjianxu wants to merge 1 commit into
Conversation
...les (Graphify-Labs#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.
e16909f to
fdf5fe9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds _merge_go_package_types, run right after _merge_decl_def_classes and before id disambiguation, to fold the duplicate Go type nodes that extract_go mints once per file declaring a method on a package type into a single node, keeping the declaring file (type X) as the survivor and preserving each dropped method's own source_file. It only collapses bare type labels among .go files sharing one directory, and deliberately leaves splits in place for same-named packages in different directories, _test.go external test packages, and function/method name collisions, since the id folds in the directory name and the package clause isn't parsed. Without it, disambiguation fragments one type across partial nodes and downstream single-definition guards bail on the resulting ambiguity.
No blocking issues surfaced. 7 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2043 functions depend on the 371 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 546 callers, 44 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
_extract_generic()— 18 callers, 26 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - new:
_resolve_js_module_path()— 27 callers, 6 callees - ...and 44 more — each is listed as a finding
Verification — 2043 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1878 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract.
The verifier did not have enough to check extract, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `cache_root` is annotated `Path | None` — outside the synthesizable primitive/collection set
· 52 more finding(s) on lines outside this diff (see the check run).
Fixes #3399.
A Go type whose methods live in other files of the same package became several nodes, one per
file, each owning only the methods declared there — and the declaring file's node owning none of
them.
extract_gomints the receiver's type in every file that declares a method on it, keyed_make_id(pkg_scope, receiver_type), so those nodes share one id and differ only insource_file;_disambiguate_colliding_node_idsreads that as a collision between distinctentities and salts each one with its own path.
Change
One pass,
_merge_go_package_types, run immediately after_merge_decl_def_classesand for thesame reason: before disambiguation, so it sees one
source_fileper id and leaves it alone. Thecolliding nodes already share an id, so every edge already points at the survivor — only the
redundant duplicates are dropped, no edge moves.
The declaring file wins, identified as the only file with a
containsedge to the type. When thedeclaration lives outside the corpus the whole group is method-only; one node is still the
answer, so it folds on the lowest path.
Guards against a false merge
a/svcandb/svccollideon id while being different packages. Those stay split, as today.
_test.gostops the fold. Its package clause may be the separate externalsvc_testpackage declaring a type of the same name.
extract_goderivespkg_scopefrom the directoryand never parses the package clause, so same-directory is not evidence of same package there.
.nor(, while every other labelthe extractor mints does (
Run(),.Close(), the file node'sa.go), so a package whosehelper function and a method share a name is untouched.
Verification
Before, on the three-file package from the issue: three
Servernodes,.Close()on theb.goone and
.Save()on thec.goone. After: onesvc_serversourced ata.goowning both, withthe
parameter_typereference fromRun()pointing at it.tests/test_go_package_type_fold.py— 6 tests: methods across files fold onto one owner; areference reaches the node that owns the methods; the declaring file survives a lower-sorting
method file; and the three guards above. The three positive tests fail on
v8and pass here;the guard tests pass both ways, pinning the no-regression half.
Full suite: 5315 passed, 93 skipped,
ruff check graphify testsclean. The only failures in thisenvironment are the pre-existing
tests/test_ollama_retry_cap.pyones (noopenaimoduleinstalled), which fail identically on plain
v8.Relation to #3395
Independent of it and based on
v8. The Go member-call resolver added there needs exactly onedeclaration of the receiver's type, so on a multi-file package it currently bails; this fold is
what lets it fire. Neither PR needs the other to land first.