Repository navigation
fix(go): resolve same-file member calls through receiver types - #4966
xiongjianxu wants to merge 1 commit into
Conversation
|
Thanks for the pull request, @xiongjianxu. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
Resolve same-file member calls through declared receiver types, preserve lexical receiver bindings, and keep bare package calls distinct from methods. Refs Graphify-Labs#4962
3bcc8e2 to
4872e28
Compare
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 3 advisory finding(s) below merit a look before merge.
Not checked: tests were not run; no formal proof of the changed code.
Graphify review — findings
Reworks Go member-call resolution so x.Method() binds through the receiver's declared type, same-file calls included, and carries INFERRED confidence, while bare package-level function calls stay EXTRACTED. The receiver-type table is now built with lexical scopes, so each call picks up the type in effect at its site, and receivers with unknown or ambiguous types no longer fall back to binding by method name. Bumps the AST cache schema to 37 so Go extractions made under the old binding rules are re-run.
Worth a look
- Receiver table no longer records locals declared inside function bodies —
graphify/extractors/go.py:221· Escalate · high- NOT verified (no proof, no reproducing execution) — needs human review
- Go call filter reads key 'language' while neighboring code reads 'lang' —
graphify/extract.py:11228· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
- Block-scoped Go declarations leak into the enclosing function's receiver table —
graphify/extractors/go.py:992· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 4606 functions depend on the 558 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
extract()— 1117 callers, 66 callees
Verification — 4606 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: 3982 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
502 of 502 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/pi.test.ts— full-run-safetytests/test_4874_jvm_resolution.py— impact, full-run-safetytests/test_4909_reference_binding_scale.py— full-run-safetytests/test_affected_cli.py— full-run-safetytests/test_affected_defines.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_affected_overrides.py— impact, full-run-safetytests/test_affected_path_file_node.py— full-run-safetytests/test_affected_seed_reference_forms.py— full-run-safetytests/test_agents_platform.py— full-run-safetytests/test_alias_folding.py— impact, full-run-safetytests/test_analyze.py— impact, full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— impact, full-run-safetytests/test_astro_import_ids.py— impact, full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_backend_opt_in.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_benchmark_real_query.py— full-run-safetytests/test_binary_media_words.py— impact, full-run-safetytests/test_blade_extractor.py— impact, full-run-safetytests/test_build.py— impact, full-run-safetytests/test_build_located_semantic_identity.py— impact, full-run-safetytests/test_build_merge_dedup_scope.py— impact, full-run-safetytests/test_build_merge_hyperedges_and_prune.py— impact, full-run-safetytests/test_build_merge_shrink_guard.py— impact, full-run-safetytests/test_builtin_global_type_refs.py— impact, full-run-safetytests/test_c_include_resolution.py— impact, full-run-safetytests/test_cache.py— impact, full-run-safetytests/test_cache_long_paths.py— impact, full-run-safetytests/test_cache_stale_import_target.py— impact, full-run-safetytests/test_cache_windows_case_4925.py— impact, full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_cargo_missing_manifest.py— full-run-safetytests/test_carried_hyperedge_remap.py— impact, full-run-safetytests/test_case_only_decl_ids_4904.py— impact, full-run-safetytests/test_case_sensitive_resolution.py— impact, full-run-safetytests/test_charmap_encoding.py— impact, full-run-safetytests/test_chunking.py— impact, full-run-safetytests/test_cjs_module_extension.py— impact, full-run-safetytests/test_claude_cli_backend.py— full-run-safety- … and 452 more
non-code file(s) changed (
ARCHITECTURE.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
ARCHITECTURE.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
· 1 more finding(s) on lines outside this diff (see the check run).
a.apply()could bind toB.applyeven whenawas declared as*A; method declaration order chose the target. A method could also capture a bare call to a package-level function. Route member calls through receiver-aware resolution and exclude methods from local/global bare-call candidates.Receiver types follow lexical scope and declaration order. Unknown local bindings block package-variable fallback, fields resolve through their owning struct, and direct calls take priority over method-value references. Generic receiver bindings and explicit
T.M(t)/(*T).M(t)expressions use their declared types. Unknown or ambiguous receivers remain unresolved.Same-file member calls now carry
INFERREDconfidence, matching cross-file receiver resolution; same-file bare function calls retainEXTRACTED. AST cache schema advances from 36 to 37. The historical schema regression keeps a lower bound without requiring an old issue number in the current assignment's comment.Scope: this improves extraction within the visible corpus. It does not add complete Go package/import resolution, package-qualified type inference or general return-type inference.
global addcontinues to use existing parked-call hints rather than rerunning receiver inference.Validation after rebasing onto v8
8e6a6d21, using the existing macOS environment without installing dependencies:git diff --check: passed; added Python lines are at most 120 Unicode code points.python -m graphify update . --no-cluster: succeeded. Generated graphs and caches are excluded.Coordinated full-suite validation on v8
8e6a6d21usedUV_OFFLINE=1 PYTHONHASHSEED=0 python -m pytest tests -q --tb=short --disable-warnings, withPYTHONPATHpointing to each checkout: baseline 9,094 passed / 15 failed, all four changes combined 9,224 passed / 15 failed, with identical failure lists and zero added failures. Both had 357 skipped and 1 xfailed. The 15 baseline failures cover SQL grammar, wheel-build dependency, shell-parser control and Terraform parallel-cache cases. Combined full Pyright had 704 errors / 4 warnings versus baseline 705 / 4, with zero added diagnostics. Combined validation uses cache schema 39 to keep its namespace separate from the individual extraction patches.Closes #4962