feat(go): infer structural interface implementations#1966
Conversation
|
@evander-wang is attempting to deploy a commit to the NexusCore Team on Vercel. A member of the Team first needs to authorize it. |
CI Report✅ All checks passed Pipeline Status
Test Results
✅ All 10889 tests passed 12 test(s) skipped — expand for details
Code CoverageTests
📋 View full run · Generated by CI |
|
Could you please resolve the conflict? |
|
@evander-wang Could we also write benchmarking for these changes? we need to ensure linear scaling. We already have an implementation in place for other languages. |
Add signature-aware structural interface detection for Go and emit inferred IMPLEMENTS edges during scope resolution so interface-dispatch and METHOD_IMPLEMENTS can use them. The detector now builds interface method sets, includes locally embedded interfaces, suppresses unresolved embedded interfaces, preserves Go type shape and package qualifiers, validates return types, expands grouped named returns, and avoids treating pointer-receiver-only methods as value-type implementations. Also improve Go type metadata for interface method elements, receiver bindings, explicit interface variables with concrete composite literal RHS, multi-assignment/range inference, and struct owner lookup in receiver-bound calls. Add Go structural interface dispatch fixtures and unit/integration coverage for precise assignment dispatch, interface fan-out, embedded interfaces, invalid signatures, grouped returns, and pointer receiver method-set semantics.
Add focused coverage for Go interface method return signature captures, cyclic embedded interfaces, and structs satisfying multiple unrelated interfaces. Document why structural IMPLEMENTS edges are emitted before MRO/interface dispatch and why Go signature normalization preserves type shape and package qualifiers.
Qualify Go structural interface signature types with file package context and finalized namespace import aliases so interface methods using local package types match implementations that spell the same type as pkg.Type or an import alias. Add a cross-package fixture that proves GoodStore satisfies api.Saver through aliased api.User signatures while WrongStore using other.User is rejected, and verifies interface dispatch only fans out to the valid implementor.
Build Go structural interface detection indexes once per pipeline run instead of repeatedly scanning parsed files and scopes for each interface. Cache interface method sets, embedded interface sites, methods by owner, signature contexts, and method-name-to-struct candidates. Candidate structs are narrowed by the rarest required method name before signature compatibility checks, preserving output while reducing work on large Go repositories.
Cache collected Go interface method sets so shared embedded interfaces are not recomputed repeatedly during structural implementation detection. Document why receiver self-bindings preserve pointer shape and assert that inferred structural IMPLEMENTS edges keep the expected confidence value.
Drop the unused nodeQualifiedName helper from the Go structural interface integration tests after the assertions were narrowed to edge sets and owner names.
The Go legacy DAG does not synthesize structural IMPLEMENTS or METHOD_IMPLEMENTS edges and cannot feed those inferred relationships into interface dispatch. Register the new structural interface integration cases as expected legacy parity failures so the scope-resolution parity job continues to validate the supported registry-primary path without requiring a legacy backport.
|
Cross-ref against the Go audit sub-issue #1927 (parent #1919) — this PR looks like the right home for F30 and a meaningful slice of F31, but a few ticket items still look open after merge. Already covered here (thanks):
Still missing from #1927 — could we implement these in this PR (or follow up immediately if scope is already large)?
Happy to split F32/F34 into a tiny follow-up if you’d rather keep this PR focused on structural interface dispatch — but it’d be good to either land them here or open tracked follow-ups on #1927 so the audit boxes don’t stay unchecked by accident. Let me know which of the above you prefer in-scope vs follow-up. |
d5eca84 to
d39ae85
Compare
Add two benchmark suites for detectGoInterfaceImplementations: 1. Ungated tripwire (runs in CI): 50 interfaces × 50 structs, asserts completion within 5s budget and correct IMPLEMENTS edges. 2. Gated scaling benchmark (GITNEXUS_BENCH=1): measures at 50×50, 200×200, and 800×800 pairs, asserts scaling ratio < 1.5. Results: 0.96x and 1.16x scaling — confirms near-linear behavior thanks to the structIdsByMethodName candidate index optimization.
of course let me first look at the existing benchmark implementation |
Go capture output changed due to structural interface detection (method_elem, return-type, pointer receiver raw form, new container types). Scaling remains linear (1.08x).
I'll include F31 (remainder) in this PR since it's closely tied to the structural interface dispatch work. |
Add signature-aware Go structural interface implementation inference to the scope-resolution pipeline. The detector now preserves Go type-shape and package-qualified signature checks, resolves embedded interface references through scope/import context, and uses promoted methods from embedded structs when building candidate method sets. Keep the shared inheritance resolver path centralized so explicit heritage emission and Go structural detection consume the same binding logic. Remove the duplicate Go embedded-interface query now covered by synthesized inheritance references. Cover pointer receiver exclusions, embedded interfaces, cross-package embedded interfaces, promoted embedded struct methods, direct method shadowing, ambiguous promoted methods, grouped returns, missing returns, and legacy expected-failure parity. Update Go capture golden data for the intentional capture changes.
magyargergo
left a comment
There was a problem hiding this comment.
Tri-method review — PR #1966 feat(go): infer structural interface implementations
Methods: GitNexus swarm + Compound-Engineering personas (both Claude) + Codex gpt-5.5/xhigh.
Engine-independence caveat: five of the six lanes are Claude (same base model) — only Codex is a genuinely independent engine, and its sandbox blocked code execution (static-analysis only). Cross-Claude agreement reduces individual noise but does not eliminate shared blind spots, so the coordinator independently verified every posted finding: the three soundness findings were reproduced end-to-end via runPipelineFromRepo under npx tsx in a worktree (real edge output, named receivers, with positive/negative controls), and the CI blocker was reproduced both on CI and via a local bench/scope-capture/measure.mjs --check. Lane status: 5 Claude lanes returned findings; 2 gitnexus lanes ran partial (their domains — shared-file blast radius, CI/test wiring — were verified directly by the coordinator).
Severity key: P1 = blocks merge or emits an incorrect edge that propagates downstream (call graph / dispatch); P2 = isolated false positive/negative; P3 = minor.
Bottom line: currently merge-blocked by CI — not ready to merge as-is
tests / benchmarks (GITNEXUS_BENCH) fails on a stale Go scope-capture baseline (→ CI Gate fails, mergeStateStatus: BLOCKED). That is a one-line re-baseline fix. Separately, three reproduced signature/embedding soundness corner cases remain — one P1 false-positive that pollutes the call graph, two P2s. No happy-path regressions; scope-parity, typecheck, and coverage all pass.
What's solid (credit where due)
The disabled naive method-name superset matcher is replaced by signature-aware structural matching (param counts/types, return types, package-qualified) with interface embedding, struct-embedding method promotion, recursion cycle guards, and rarest-method candidate pruning, wired into the generic pipeline behind a gated, dedup-aware hook (existing explicit IMPLEMENTS win; edges emitted before MRO). Test coverage is strong — exact edge-set assertions plus explicit negatives (wrong-signature, shadowed-method, missing-method, pointer-only). Validated suspicions that turned out fine: the pointer-receiver exclusion is a deliberate, test-codified value-method-set choice; range-binding.ts's +1 is a latent correctness fix (1-based scope-line convention), not a regression; the qualifiedName mutation is guarded against double-qualify; same-depth embedding ambiguity is handled correctly. (Codex + correctness verified these.)
Findings (inline; all reproduced by the coordinator)
- P1 (CI-verified) — stale Go scope-capture baseline blocks the benchmark gate → merge BLOCKED. (
baselines.json) - P1 — cross-package signature normalization emits a false
IMPLEMENTSwhen an imported type's package can't be resolved. Strong: Codex + adversarial independently. (interface-impls.ts:481) - P2 — struct-embedding promotion drops a method as "ambiguous" when it's unambiguous at the shallowest depth → false negative. Strong: Codex + correctness independently. (
interface-impls.ts:251) - P2 — variadic
...Telement types are never package-qualified → cross-package falseIMPLEMENTS. (interface-impls.ts:479)
Non-blocking
anyvsinterface{}treated as distinct → false negative for a common idiom.- Perf — required-side signature normalization is recomputed per candidate struct (regex per type); pre-normalize once. Bounded by candidate pruning except on ubiquitous method names (Stringer/Closer-like) where the candidate set collapses to all structs.
cloneMethodSetallocates on every cache hit. - Maintainability —
goReceiverKindis a structural-cast bolt-on (not onSymbolDefinition);extractCompositeLiteralTypeNodeduplicatesextractTypeNode; the emitted-edge count is dropped from run stats;packageQualifierForFileuses the directory path, not the Go module path. - Defensive — the undefined-metadata leniency is fail-open; flagged by Codex + 3 Claude lanes but unanimously unreachable today (arity-metadata co-populates count+types), so a fail-closed guard is hardening, not a bug fix.
- Shared-file note —
CLASS_CONTAINER_TYPESnow matches Gostruct_type/interface_type(consumed by call-processor / parsing-processor / type-env / parse-worker); Go-specific node names +scope-paritygreen → low risk.
Test gaps
- The cross-package
describeassertsIMPLEMENTSonly withtoContain/not.toContain— add an exacttoEqual([...])bound (would have caught finding 2 and the variadic finding 4). - No unit coverage for
qualifyGoSignatureTypeswith a realpackageQualifier(variadic / func-typed / unresolved alias),goBindingScopeFor, thevar_specunary arm, or the "existing IMPLEMENTS wins" dedup.
CI at review time
format / lint / typecheck / typecheck-web, scope-parity / scope-resolution parity, and tests / ubuntu / coverage PASS; tests / benchmarks (GITNEXUS_BENCH) and CI Gate FAIL (the stale Go baseline, finding 1); Vercel fails on auth only (ignore). mergeStateStatus: BLOCKED.
Automated multi-tool digest (3 review methods; only Codex is a separate engine, and it was execution-sandboxed). Every inline finding was reproduced by the coordinator against the real pipeline. Verify before acting.
Apply Go method promotion depth rules so shallow embedded methods win and same-depth promoted methods remain ambiguous. Treat unresolved import-qualified signature types as unverifiable, preserve variadic element package identity, and tighten cross-package structural edge tests. Update the Go scope-capture benchmark fingerprint for the structural interface capture changes.
Summary
This PR adds signature-aware structural interface implementation detection for Go and feeds the inferred relationships into the scope-resolution pipeline.
Go does not have explicit
implementsdeclarations, so the analyzer has to infer interface satisfaction from method sets. Before this change, the Go analyzer could only reason about direct receiver calls and did not have a reliable way to emitIMPLEMENTSedges for structurally satisfied interfaces. That meant interface-typed calls such asrepo.Save(user)could not fan out through the graph to known concrete implementations.The new flow is:
IMPLEMENTSedges from Go interface method sets.interface-dispatchandMETHOD_IMPLEMENTSlogic consume the inferred relationships.What Changed
Structural interface implementation detection
detectGoInterfaceImplementationsfrom method-name-only matching into method-set + signature matching.Useris not equal to[]User[]Useris not equal to...Usera.Useris not equal tob.Usergo-structural-implements.Embedded interface handling
io.Reader.Example:
ReadClosernow requires bothRead()andClose(). A type with onlyClose()no longer gets a falseIMPLEMENTSedge.Pointer receiver method-set correctness
Go value and pointer method sets differ:
This PR preserves the raw receiver shape for Go self bindings and records whether a method was declared on
Tor*T.func (t T) M()contributes to the value type method set.func (t *T) M()does not causeTto structurally implement the interface.*TtoTwhere appropriate so existing member-call resolution continues to work.Return type and grouped return fixes
Close() erroris not compatible withClose().M() (a, b int)is treated as returning(int, int), notint.Go capture and type metadata improvements
method_elemdeclarations.This lets the resolver prefer the concrete local assignment over broad interface fan-out.
Scope-resolution pipeline integration
Structdefinitions.Behavior Examples
Precise concrete dispatch
The analyzer resolves
repo.Save(user)directly toSqlRepository.Savebecause the concrete RHS type is known.Interface fan-out when runtime type is unknown
When the receiver is only known as
Repository, the analyzer emits interface-dispatch calls to all statically known valid implementors.Invalid implementors are rejected
BadRepositoryno longer receives anIMPLEMENTSedge because the parameter type does not match.Tests
Added a dedicated Go fixture:
gitnexus/test/fixtures/lang-resolution/go-structural-interface-dispatch/repository.goAdded/updated coverage for:
METHOD_IMPLEMENTSgeneration from inferred Go structural edgesValidation run:
Pre-commit also passed:
eslint --fixprettier --writepre-commit: typechecking gitnexus...Notes
interface-dispatchedges are expected.Refs #1927