diff --git a/gitnexus-shared/src/scope-resolution/reference-site.ts b/gitnexus-shared/src/scope-resolution/reference-site.ts index e8a58e1b2..6629dacd3 100644 --- a/gitnexus-shared/src/scope-resolution/reference-site.ts +++ b/gitnexus-shared/src/scope-resolution/reference-site.ts @@ -162,6 +162,30 @@ export interface ReferenceSite { * in callee position, so nothing changes for languages that never set it. */ readonly inCalleePosition?: boolean; + /** + * This `inherits` site describes an embedded field written as a POINTER + * (`struct S { *T }`) rather than as a value (`struct S { T }`). + * + * Go's method-set rules make the two forms genuinely different, so the + * distinction cannot be normalized away without producing wrong answers + * (go.dev/ref/spec#Struct_types): + * + * - `S` embeds `T` → `MS(S)` and `MS(*S)` get promoted methods with + * receiver `T`; only `MS(*S)` also gets those with + * receiver `*T`. + * - `S` embeds `*T` → `MS(S)` AND `MS(*S)` get promoted methods with + * receiver `T` **or** `*T`. + * + * So with `func (t *T) Ping()`, `S{T}` does not implement a `Ping` interface + * by value while `S{*T}` does. Collapsing the forms makes both answers the + * same, and one of them is then wrong. + * + * A POSITION FACT, like `inCalleePosition`: the capture layer records how the + * field was spelled and resolution decides what it means. Set only by + * languages with pointer-embedding semantics (Go today); absent everywhere + * else, so every other language's sites stay byte-identical. + */ + readonly embeddedAsPointer?: boolean; } /** diff --git a/gitnexus/bench/scope-capture/baselines.json b/gitnexus/bench/scope-capture/baselines.json index 4860470f1..58b449287 100644 --- a/gitnexus/bench/scope-capture/baselines.json +++ b/gitnexus/bench/scope-capture/baselines.json @@ -1,7 +1,7 @@ { "_comment": "Per-language baselines for bench/scope-capture/measure.mjs --check. fingerprint = order-independent sha256 over the lang-resolution/-* fixture corpus + a 20-entity synthetic source (correctness gate; re-baseline intentionally on a legitimate capture change). scaling_budget = max allowed (t800/t250)/(800/250); ~1.0 is linear, ~3.2 is quadratic. The synthetic source is now HERITAGE-BEARING for every language (each Entity extends/implements/embeds/uses-trait/conforms-to a shared base) so the #1951 @reference.inherits synth is gated at scale, not just the base capture loop. All languages thread the tree-sitter captured node instead of re-deriving it with findNodeAtRange(tree.rootNode,...) per match, so all are linear (go #1915, python #1918, ruby/php/rust/csharp #1951, java #1956).", "go": { - "fingerprint": "e47302079e17a5e73711bbed5416557b49327cb67e4932008700ec6b8fb468b3", + "fingerprint": "c27fb803598581fa4eb7ddf5ef6f8369b9e3a150082d11362e7aa3ec8faaa832", "scaling_budget": 1.5, "_rebaselined_callable_flow_2522_review": "PR #2522 review hardening: callable operands retain expression/qualified identity and formals retain signature metadata. Prior 3d4e32e7490c830516126e28931827949baa3594cb521f7a3d8dcfed95b6018a -> 57b3c55135af8d2af33b9a7c4bf89796a7bee5b5822b402a2dea91af7232cf4a; scaling 1.058 < 1.5.", "_rebaselined_callable_flow_2522_followup": "PR #2522 follow-up: provider-owned callable assignment/copy/formal/argument/invoke facts with invocation/constructor-result suppression. Prior 09ecd94911b830f52fa8807560abcbd79f163d02a2072870c1a59297e9a326e1 -> 3d4e32e7490c830516126e28931827949baa3594cb521f7a3d8dcfed95b6018a; scaling 1.039 < 1.5.", @@ -11,10 +11,12 @@ "_rebaselined_2766_receiver_chain_wire_v2": "#2766: receiver-chain wire format v1 -> v2 (name-free `await` / `index` step kinds). The VERSION prefix is part of every emitted `@reference.receiver-chain` capture, so every chain-minting language's capture text changed. WIRE-FORMAT CHANGE, NOT A CAPTURE-SET CHANGE: the same chains are minted for the same sites, spelled `2|\u2026` instead of `1|\u2026`. Exactly the 12 chain-minting languages drifted; c, cobol and dart did not, which is the check that this is the prefix and not a capture regression. Accompanied by SCHEMA_BUMP 34 -> 37 and INCREMENTAL_SCHEMA_VERSION 28 -> 31 so a stale index is rejected rather than replaying chains a v2 decoder refuses. Prior 8cba537ff211fab3bac5fb4456cd1ffba14d6a2db75c40acae28ab8bf29f3d2e -> 8162272bb897b0b89472c406321cf8d88a5ae4ea83ea9e3c45f8e817041bff9f.", "_rebaselined_2766_await_subscript_emission": "#2766: extractMixedChain now walks THROUGH await and subscript nodes and peels transparent wrappers at loop entry, so sites whose receiver is `repos[0]` or `(await f())` mint a receiver chain where they previously minted none. EMISSION CHANGE: more sites carry `@reference.receiver-chain`; no existing chain changed shape. Only go and kotlin drifted of 15 \u2014 the two whose fixture corpora contain such receivers. Prior 8162272bb897b0b89472c406321cf8d88a5ae4ea83ea9e3c45f8e817041bff9f -> c9c908f441e3be12fad2448120ed3ea35dc235a12b3f63b0ec532ffdae11d9e9.", "_rebaselined_2766_phantom_callee_read_site": "#2766: Go's `@reference.read` pattern matches EVERY selector_expression, so a member call `h.dep.Work()` minted THREE sites \u2014 the call, the genuine `h.dep` field read, and a PHANTOM read on the callee `h.dep.Work`. The phantom resolved through findOwnedMember (which prefers methods over fields) and emitted an ACCESSES edge to the METHOD duplicating the CALLS edge at the same position; visible today on any receiver the text cascade can type (`RunFromValueReceiver -> DoWork`). The emitter now drops a read match whose selector is in FUNCTION position. FEWER capture matches for Go, no other language affected \u2014 go was the only fingerprint of 15 that moved. A method VALUE (`f := h.dep.Work`) is not in function position and is untouched. Prior c9c908f441e3be12fad2448120ed3ea35dc235a12b3f63b0ec532ffdae11d9e9 -> 7bb524a32a2eed57a15b454e3a33480e92a496c683e6856ef02179693c0e02e3.", - "_rebaselined_2766_callee_position_marker": "#2766 review fix: a call's callee selector is no longer DROPPED at capture. An earlier commit on this branch dropped it outright, which also deleted the genuine field read on a func-typed struct field (`h.dep.Work()` where `Work func() error`) - callback/hook/mock structs lost their only ACCESSES evidence. The match is now emitted carrying `@reference.callee-position`, and the phantom is suppressed at EMIT by the resolved target's kind instead. Go only: the other 14 languages' fingerprints are byte-identical, which is the check that this is not a cross-language capture change. Prior 7bb524a32a2eed57a15b454e3a33480e92a496c683e6856ef02179693c0e02e3 -> e47302079e17a5e73711bbed5416557b49327cb67e4932008700ec6b8fb468b3; scaling 1.001 < 1.5; fixtures 102 (unchanged), capture_groups_fp 2103." + "_rebaselined_2766_callee_position_marker": "#2766 review fix: a call's callee selector is no longer DROPPED at capture. An earlier commit on this branch dropped it outright, which also deleted the genuine field read on a func-typed struct field (`h.dep.Work()` where `Work func() error`) - callback/hook/mock structs lost their only ACCESSES evidence. The match is now emitted carrying `@reference.callee-position`, and the phantom is suppressed at EMIT by the resolved target's kind instead. Go only: the other 14 languages' fingerprints are byte-identical, which is the check that this is not a cross-language capture change. Prior 7bb524a32a2eed57a15b454e3a33480e92a496c683e6856ef02179693c0e02e3 -> e47302079e17a5e73711bbed5416557b49327cb67e4932008700ec6b8fb468b3; scaling 1.001 < 1.5; fixtures 102 (unchanged), capture_groups_fp 2103.", + "_rebaselined_2813_interface_field_dispatch_fixture": "#2813: added test/fixtures/lang-resolution/go-interface-field-dispatch/ (8 Go files) as the committed regression fixture for calls through an interface-typed struct field. Go fixture_count 102 -> 110. FIXTURE-CORPUS GROWTH, NOT A CAPTURE CHANGE: the accompanying fixes are a detection-time method-set change (interface-impls.ts) and a resolution-time fan-out in the shared receiver pass, neither of which emits captures; go/query.ts and go/captures.ts are untouched. Go was the ONLY language whose fingerprint drifted, and every other language matched its baseline on the same run - the same check used for the #2766 fixture growth above. Prior e47302079e17a5e73711bbed5416557b49327cb67e4932008700ec6b8fb468b3 -> cffee41cadbf350855d99bd5aee7c015b1e8b31d1c343d02f113540abe86c765; scaling 1.074 < 1.5, capture_groups_fp 2303." }, "cobol": { "fingerprint": "c8c00b56a7da24e04080eb885714fbbf45e3903324f0cf9df0754f5b5a92e3aa", + "_rebaselined_2813_exact_method_sets": "#2813: Go embedded fields now emit `@reference.embedded-pointer` when spelled `*T` rather than `T`. A CAPTURE-EMISSION CHANGE, not fixture growth: fixture_count is unchanged at 110 and capture_groups_fp moves 2303 -> 2339 (+36), which is the new marker plus the WrongSigRepo/Recount rows added to two existing fixture files. The marker is required for exactness — Go gives `struct{ Base }` and `struct{ *Base }` different method sets, so structural interface satisfaction cannot be correct without knowing which was written (go.dev/ref/spec#Struct_types). Go was the ONLY language of 15 whose fingerprint moved, which is the check that this is a Go capture change and not a cross-language regression. Accompanied by SCHEMA_BUMP 39 -> 43 (skipping 40/41/42, taken by origin/main during review) so a warm cache cannot replay the pre-marker capture set. Prior cffee41cadbf350855d99bd5aee7c015b1e8b31d1c343d02f113540abe86c765 -> c27fb803598581fa4eb7ddf5ef6f8369b9e3a150082d11362e7aa3ec8faaa832; scaling 0.987 < 1.5.", "scaling_budget": 1.5, "_rebaselined_callable_flow_2522_followup": "PR #2522 follow-up: COBOL procedure-pointer callable flow facts; multi-topic extraction now consumes each grouped scope/declaration match once instead of requiring a duplicate declaration-only match. Prior 68ee0e95eb9f86f2d92ca35f730f4c2d4d83abc1b5241ae767ff3437780ec8d1 -> d45bb091b0893d0de4fae2486b31ba21719c9377bf35a0908fd3a36fa1c3bf4e; scaling 0.853 < 1.5.", "_note": "Updated for F17-F23 fixes (P2: TIMES guard, ADD GIVING, SQL AS alias). See PR #1959.", diff --git a/gitnexus/src/core/ingestion/languages/go/captures.ts b/gitnexus/src/core/ingestion/languages/go/captures.ts index baff677af..c1a8cf23c 100644 --- a/gitnexus/src/core/ingestion/languages/go/captures.ts +++ b/gitnexus/src/core/ingestion/languages/go/captures.ts @@ -41,6 +41,21 @@ const CALLEE_POSITION_MARKER: Capture = Object.freeze({ text: '', }); +/** + * Presence-only marker for an embedded field written as `*T` rather than `T`. + * + * Go's method-set rules treat the two forms differently — `S{T}` promotes only + * the value-receiver methods of `T` into `MS(S)`, while `S{*T}` promotes both + * value- and pointer-receiver ones (go.dev/ref/spec#Struct_types). Structural + * interface detection cannot give an exact answer without knowing which was + * written, so the spelling is recorded here and interpreted downstream. + */ +const EMBEDDED_POINTER_MARKER: Capture = Object.freeze({ + name: '@reference.embedded-pointer', + range: ZERO_RANGE, + text: '', +}); + const GO_CALLABLE_CAPTURE_OPTIONS = { functionNodeTypes: new Set(['function_declaration', 'method_declaration', 'func_literal']), callNodeTypes: new Set(['call_expression']), @@ -358,7 +373,12 @@ function synthesizeGoInheritanceReferences(root: SyntaxNode): CaptureMatch[] { if (field.childForFieldName('name') !== null) continue; // `field.type` is the embedded base — bare/qualified/generic, with any // `*` pointer marker as an unnamed sibling token (already unwrapped). - emitGoEmbedInheritance(field.childForFieldName('type'), out); + // That token is the ONLY record of `*T` versus `T`, and the two have + // different method sets, so read it off the field text before it is + // lost. An embedded field has no name, so a leading `*` can only be + // the pointer marker. + const embeddedAsPointer = field.text.trimStart().startsWith('*'); + emitGoEmbedInheritance(field.childForFieldName('type'), out, embeddedAsPointer); } } else if (typeNode.type === 'interface_type') { for (const elem of typeNode.namedChildren) { @@ -367,7 +387,8 @@ function synthesizeGoInheritanceReferences(root: SyntaxNode): CaptureMatch[] { // `type_elem` with >1 named child — skip them (legacy // `shouldSkipExtends`); a single-element `type_elem` is the embed. if (elem.type !== 'type_elem' || elem.namedChildCount !== 1) continue; - emitGoEmbedInheritance(elem.namedChild(0), out); + // An interface embed is never a pointer form. + emitGoEmbedInheritance(elem.namedChild(0), out, false); } } } @@ -380,13 +401,20 @@ function synthesizeGoInheritanceReferences(root: SyntaxNode): CaptureMatch[] { * node, reducing the name to its bare simple identifier. No-ops when `baseNode` * is null or not one of the embed shapes. */ -function emitGoEmbedInheritance(baseNode: SyntaxNode | null, out: CaptureMatch[]): void { +function emitGoEmbedInheritance( + baseNode: SyntaxNode | null, + out: CaptureMatch[], + embeddedAsPointer: boolean, +): void { if (baseNode === null) return; const nameNode = goEmbedBaseNameNode(baseNode); if (nameNode === null) return; out.push({ '@reference.inherits': nodeToCapture('@reference.inherits', baseNode), '@reference.name': nodeToCapture('@reference.name', nameNode), + // Added ONLY for the pointer form, so a value embed's capture set is + // byte-identical to what it was before this marker existed. + ...(embeddedAsPointer ? { '@reference.embedded-pointer': EMBEDDED_POINTER_MARKER } : {}), }); } diff --git a/gitnexus/src/core/ingestion/languages/go/interface-impls.ts b/gitnexus/src/core/ingestion/languages/go/interface-impls.ts index 55b3186a3..d684340d3 100644 --- a/gitnexus/src/core/ingestion/languages/go/interface-impls.ts +++ b/gitnexus/src/core/ingestion/languages/go/interface-impls.ts @@ -12,7 +12,22 @@ type MethodSetEntry = { readonly ambiguous: boolean; }; type MutableMethodSetEntries = Map; -type GoMethodDefinition = SymbolDefinition & { readonly goReceiverKind?: 'value' | 'pointer' }; +/** A struct embedded in another, plus HOW it was embedded (`T` vs `*T`). */ +type EmbeddedParent = { readonly structId: string; readonly asPointer: boolean }; +/** + * The two method sets Go defines for a defined type, kept separately because + * they answer different questions and only one of them is assignability. + * `pointer` = MS(*T) — every method callable on a *T value. + * `value` = MS(T) — the subset callable on a T value. + */ +type DualMethodSet = { readonly value: MutableMethodSet; readonly pointer: MutableMethodSet }; +/** Which method set satisfied an interface. `value` implies pointer too. */ +export type GoReceiverForm = 'value' | 'pointer'; +/** One structural implementor plus the form in which it implements. */ +export type GoStructuralImplementor = { + readonly structDefId: string; + readonly receiverForm: GoReceiverForm; +}; type SignatureContext = { readonly packageQualifier: string | undefined; readonly importQualifiers: ReadonlyMap; @@ -25,7 +40,8 @@ type DetectionIndexes = { readonly interfaceById: ReadonlyMap; readonly interfaceOwnMethodsById: ReadonlyMap; readonly embeddedSitesByInterfaceId: ReadonlyMap; - readonly parentStructIdsByStructId: ReadonlyMap; + readonly parentStructIdsByStructId: ReadonlyMap; + readonly valueMethodsByStructId: ReadonlyMap; readonly structIdsByMethodName: ReadonlyMap>; readonly signatureContextByDefId: ReadonlyMap; readonly scopeIndexes: ScopeResolutionIndexes; @@ -35,7 +51,7 @@ export function detectGoInterfaceImplementations( parsedFiles: readonly ParsedFile[], _indexes: ScopeResolutionIndexes, _model: SemanticModel, -): Map { +): Map { return detectGoInterfaceImplementationsFromIndexes(buildDetectionIndexes(parsedFiles, _indexes)); } @@ -50,7 +66,8 @@ function buildDetectionIndexes( const interfaceById = new Map(); const interfaceOwnMethodsById = new Map(); const embeddedSitesByInterfaceId = new Map(); - const parentStructIdsByStructId = new Map(); + const parentStructIdsByStructId = new Map(); + const valueMethodsByStructId = new Map(); const structIdsByMethodName = new Map>(); const signatureContextByDefId = new Map(); const interfaceIdByScopeId = new Map(); @@ -71,11 +88,44 @@ function buildDetectionIndexes( } if (def.type !== 'Method' && def.type !== 'Function') continue; if (def.ownerId === undefined) continue; - if (isPointerReceiverMethod(def)) continue; - const methodName = simpleQualifiedName(def); - if (methodName === undefined || methodName.length === 0) continue; + // POINTER-receiver methods count toward the method set (#2813). + // + // Go's rule is per-type, and there are two types here: the method set of + // `T` holds only its value-receiver methods, while the method set of `*T` + // holds BOTH. #1966 kept only the value-receiver half, which makes the + // `T` answer exactly right — and the `*T` answer permanently empty, so a + // struct whose methods all read `func (r *T)` satisfied nothing and got + // no IMPLEMENTS edge at all. + // + // That is the shape idiomatic Go actually writes: methods take pointer + // receivers so they can mutate, and `*T` is what gets stored in an + // interface-typed field. Excluding it did not make the graph + // conservative, it made it silent — every call through such a field + // resolved to the interface DECLARATION and `impact()` on the + // implementation reported zero callers, indistinguishable from a symbol + // that genuinely has none. + // + // GitNexus models one Struct node per type with no separate `*T` node, so + // the two method sets cannot both be represented. This picks the `*T` + // reading: the graph now answers "which types provide this interface's + // behaviour", and no longer proves `var x I = T{}` invalid. That trade is + // deliberate — blast radius is what every consumer of IMPLEMENTS asks for + // (verified: MRO/METHOD_IMPLEMENTS derivation, community clustering, the + // receiver-dispatch fan-out index, and the epistemic heritage probe; none + // performs value-assignability checking). + // + // `goReceiverKind` is still stamped in method-owners.ts and is the hook a + // future value/pointer-aware model would read; it is deliberately no + // longer a filter here. + const simpleName = simpleQualifiedName(def); + if (simpleName === undefined || simpleName.length === 0) continue; - addMethod(methodsByOwner, def.ownerId, methodName, def); + addMethod( + methodsByOwner, + def.ownerId, + methodSetKey(simpleName, def, signatureContextByDefId), + def, + ); } } @@ -104,9 +154,11 @@ function buildDetectionIndexes( for (const childScope of childScopesByParent.get(scope.id) ?? []) { for (const def of childScope.ownedDefs) { if (def.type !== 'Method' && def.type !== 'Function') continue; - const methodName = simpleQualifiedName(def); - if (methodName === undefined || methodName.length === 0) continue; - addMethodOverload(methods, methodName, def); + const simpleName = simpleQualifiedName(def); + if (simpleName === undefined || simpleName.length === 0) continue; + // Same key function as the struct side — an interface requiring an + // unexported `seal()` must only be satisfied from its own package. + addMethodOverload(methods, methodSetKey(simpleName, def, signatureContextByDefId), def); } } interfaceOwnMethodsById.set(ifaceId, methods); @@ -126,13 +178,20 @@ function buildDetectionIndexes( if (structId === undefined) continue; const parent = resolveInheritanceBaseInScope(site.inScope, site.name, indexes); if (parent === undefined || parent.type !== 'Struct') continue; - addParentStruct(parentStructIdsByStructId, structId, parent.nodeId); + // `embeddedAsPointer` is the capture-layer record of `*T` vs `T`; the two + // promote different method sets (go.dev/ref/spec#Struct_types). + addParentStruct( + parentStructIdsByStructId, + structId, + parent.nodeId, + site.embeddedAsPointer === true, + ); } } - const structMethodSetCache = new Map(); + const structMethodSetCache = new Map(); for (const structId of structsById.keys()) { - const effective = collectStructMethodSet( + const dual = collectStructMethodSet( structId, { parentStructIdsByStructId, @@ -141,9 +200,12 @@ function buildDetectionIndexes( new Set(), structMethodSetCache, ); - if (effective === undefined) continue; - effectiveMethodsByStructId.set(structId, effective); - for (const methodName of effective.keys()) { + if (dual === undefined) continue; + // `pointer` is MS(*T) and is the superset, so it drives candidate lookup: + // a type that implements only in pointer form is still an implementor. + effectiveMethodsByStructId.set(structId, dual.pointer); + valueMethodsByStructId.set(structId, dual.value); + for (const methodName of dual.pointer.keys()) { addStructMethodCandidate(structIdsByMethodName, methodName, structId); } } @@ -158,6 +220,7 @@ function buildDetectionIndexes( embeddedSitesByInterfaceId, parentStructIdsByStructId, structIdsByMethodName, + valueMethodsByStructId, signatureContextByDefId, scopeIndexes: indexes, }; @@ -165,21 +228,31 @@ function buildDetectionIndexes( function detectGoInterfaceImplementationsFromIndexes( indexes: DetectionIndexes, -): Map { - const implementations = new Map(); +): Map { + const implementations = new Map(); const methodSetCache = new Map(); for (const iface of indexes.interfaces) { const required = collectInterfaceMethodSet(iface, indexes, new Set(), methodSetCache); if (required === undefined || required.size === 0) continue; if (!methodSetHasVerifiableSignatures(required)) continue; - const implementors: string[] = []; + const implementors: GoStructuralImplementor[] = []; for (const structId of candidateStructIdsFor(required, indexes)) { - const actual = indexes.effectiveMethodsByStructId.get(structId); - if (actual === undefined) continue; - if (methodSetSatisfies(actual, required, indexes.signatureContextByDefId)) { - implementors.push(structId); - } + const pointerSet = indexes.effectiveMethodsByStructId.get(structId); + if (pointerSet === undefined) continue; + // MS(*T) is the superset: if it does not satisfy, neither does MS(T). + if (!methodSetSatisfies(pointerSet, required, indexes.signatureContextByDefId)) continue; + // Then ask the narrower question separately — does the VALUE type satisfy? + // This is the distinction `var x I = T{}` turns on, and it is a fact about + // the program, not a heuristic. + const valueSet = indexes.valueMethodsByStructId.get(structId); + const satisfiesByValue = + valueSet !== undefined && + methodSetSatisfies(valueSet, required, indexes.signatureContextByDefId); + implementors.push({ + structDefId: structId, + receiverForm: satisfiesByValue ? 'value' : 'pointer', + }); } if (implementors.length > 0) implementations.set(iface.nodeId, implementors); } @@ -187,6 +260,46 @@ function detectGoInterfaceImplementationsFromIndexes( return implementations; } +/** + * The key a method occupies in a method set. + * + * Go spec, Uniqueness of identifiers: "Two identifiers are different if they are + * spelled differently, **or if they appear in different packages and are not + * exported**." So an UNEXPORTED method name is scoped to its declaring package — + * `seal` in package `sealed` is a different identifier from `seal` in package + * `foreign`, and a type outside `sealed` can never satisfy `interface { seal() }`. + * That is the whole basis of the sealed-interface idiom. + * + * Matching on the bare name made those types satisfy each other, which is a + * FALSE implementation rather than an over-approximation. Qualifying the key + * with the declaring package for unexported names makes the comparison exact. + * + * Exported names are deliberately left unqualified: the spec makes them the same + * identifier across packages, which is what allows cross-package interface + * satisfaction to work at all. + */ +function methodSetKey( + simpleName: string, + def: SymbolDefinition, + signatureContextByDefId: ReadonlyMap, +): string { + if (isExportedGoIdentifier(simpleName)) return simpleName; + const pkg = signatureContextByDefId.get(def.nodeId)?.packageQualifier; + return pkg === undefined ? simpleName : `${pkg}\u0000${simpleName}`; +} + +/** + * Go spec, Exported identifiers: exported iff the first character is a Unicode + * uppercase letter (category Lu). Method names always satisfy the second clause + * (they are method names), so the first character is the whole test. + */ +function isExportedGoIdentifier(name: string): boolean { + const first = name.codePointAt(0); + if (first === undefined) return false; + const ch = String.fromCodePoint(first); + return ch !== ch.toLowerCase() && ch === ch.toUpperCase(); +} + function addMethod( methodsByOwner: Map>, ownerId: string, @@ -222,59 +335,115 @@ function addStructMethodCandidate( } function addParentStruct( - parentStructIdsByStructId: Map, + parentStructIdsByStructId: Map, structId: string, parentStructId: string, + asPointer: boolean, ): void { const parents = parentStructIdsByStructId.get(structId) ?? []; - parents.push(parentStructId); + parents.push({ structId: parentStructId, asPointer }); parentStructIdsByStructId.set(structId, parents); } +/** The two entry maps built in parallel: MS(T) and MS(*T). */ +type DualEntries = { + readonly value: MutableMethodSetEntries; + readonly pointer: MutableMethodSetEntries; +}; + function collectStructMethodSet( structId: string, indexes: Pick, visiting: Set, - cache: Map, -): MutableMethodSet | undefined { + cache: Map, +): DualMethodSet | undefined { const entries = collectStructMethodEntries(structId, indexes, visiting, cache); - return entries === undefined ? undefined : methodEntriesToMethodSet(entries); + if (entries === undefined) return undefined; + return { + value: methodEntriesToMethodSet(entries.value), + pointer: methodEntriesToMethodSet(entries.pointer), + }; } +/** + * Build MS(T) and MS(*T) together, applying the spec's promotion table exactly + * (go.dev/ref/spec#Method_sets, #Struct_types): + * + * declared receiver T -> in MS(T) and MS(*T) + * declared receiver *T -> in MS(*T) only + * S embeds T (value) -> MS(S) gets P's MS(T); MS(*S) gets P's MS(*T) + * S embeds *T (pointer) -> MS(S) AND MS(*S) both get P's MS(*T) + * + * The last row is the one that makes the embed FORM load-bearing: with + * `func (b *Base) Ping()`, `struct{ Base }` does not implement a `Ping` + * interface by value while `struct{ *Base }` does. Collapsing the forms gives + * both the same answer and one of them is then wrong. + */ function collectStructMethodEntries( structId: string, indexes: Pick, visiting: Set, - cache: Map, -): MutableMethodSetEntries | undefined { + cache: Map, +): DualEntries | undefined { const cached = cache.get(structId); - if (cached !== undefined) return cloneMethodEntries(cached); + if (cached !== undefined) { + return { value: cloneMethodEntries(cached.value), pointer: cloneMethodEntries(cached.pointer) }; + } if (visiting.has(structId)) return undefined; visiting.add(structId); - const merged = directMethodEntries(indexes.methodsByOwner.get(structId)); + const own = indexes.methodsByOwner.get(structId); + // MS(*T) holds every declared method; MS(T) drops the pointer-receiver ones. + const pointer = directMethodEntries(own); + const value = directMethodEntries(filterValueReceiverMethods(own)); - for (const parentStructId of indexes.parentStructIdsByStructId.get(structId) ?? []) { - const parentEntries = collectStructMethodEntries(parentStructId, indexes, visiting, cache); + for (const parent of indexes.parentStructIdsByStructId.get(structId) ?? []) { + const parentEntries = collectStructMethodEntries(parent.structId, indexes, visiting, cache); if (parentEntries === undefined) { visiting.delete(structId); return undefined; } - for (const [methodName, entry] of parentEntries) { - if (entry.ambiguous) continue; - mergePromotedMethodEntry(merged, methodName, { - overloads: entry.overloads, - depth: entry.depth + 1, - ambiguous: false, - }); - } + // Embedding by POINTER lifts the parent's pointer-receiver methods into the + // embedder's VALUE method set; embedding by value does not. + const promotedIntoValue = parent.asPointer ? parentEntries.pointer : parentEntries.value; + promoteEntries(value, promotedIntoValue); + promoteEntries(pointer, parentEntries.pointer); } visiting.delete(structId); - cache.set(structId, cloneMethodEntries(merged)); - return merged; + cache.set(structId, { value: cloneMethodEntries(value), pointer: cloneMethodEntries(pointer) }); + return { value, pointer }; } +/** Merge one depth-level of promoted entries, preserving the shallowest-depth + * and ambiguity rules the selector spec defines. */ +function promoteEntries(target: MutableMethodSetEntries, source: MutableMethodSetEntries): void { + for (const [methodName, entry] of source) { + if (entry.ambiguous) continue; + mergePromotedMethodEntry(target, methodName, { + overloads: entry.overloads, + depth: entry.depth + 1, + ambiguous: false, + }); + } +} + +/** MS(T) excludes methods declared with a `*T` receiver. */ +function filterValueReceiverMethods(methods: MethodSet | undefined): MutableMethodSet | undefined { + if (methods === undefined) return undefined; + const out = new Map(); + for (const [name, overloads] of methods) { + const valueOnly = overloads.filter( + (def) => (def as GoMethodDefinition).goReceiverKind !== 'pointer', + ); + if (valueOnly.length > 0) out.set(name, valueOnly); + } + return out; +} + +/** Receiver-kind sidecar stamped by `populateGoOwners` (method-owners.ts). */ +type GoMethodDefinition = SymbolDefinition & { readonly goReceiverKind?: 'value' | 'pointer' }; + function collectInterfaceMethodSet( iface: SymbolDefinition, indexes: DetectionIndexes, @@ -475,10 +644,6 @@ function methodSetHasVerifiableSignatures(methods: MethodSet): boolean { return true; } -function isPointerReceiverMethod(def: SymbolDefinition): boolean { - return (def as GoMethodDefinition).goReceiverKind === 'pointer'; -} - function hasVerifiableSignature(def: SymbolDefinition): boolean { return ( def.parameterCount !== undefined || diff --git a/gitnexus/src/core/ingestion/languages/go/interpret.ts b/gitnexus/src/core/ingestion/languages/go/interpret.ts index d3a9d6113..cb9fcc047 100644 --- a/gitnexus/src/core/ingestion/languages/go/interpret.ts +++ b/gitnexus/src/core/ingestion/languages/go/interpret.ts @@ -29,8 +29,12 @@ export function interpretGoTypeBinding(captures: CaptureMatch): ParsedTypeBindin if (captures['@type-binding.self'] !== undefined) { source = 'self'; // Preserve pointer shape on receiver self-bindings (`*T` vs `T`). - // Method-owner enrichment consumes that raw shape to model Go value and - // pointer receiver method sets conservatively. + // Method-owner enrichment consumes that raw shape to stamp `goReceiverKind` + // on each method. Since #2813 that stamp is metadata, NOT a filter: + // structural interface satisfaction counts pointer-receiver methods, because + // `*T`'s method set is what an interface-typed field actually holds. The + // preserved distinction is the hook a future value/pointer-aware model would + // read — do not reintroduce it as an exclusion. normalizedType = type.trim(); } else if (captures['@type-binding.constructor'] !== undefined) { source = 'constructor-inferred'; diff --git a/gitnexus/src/core/ingestion/scope-extractor.ts b/gitnexus/src/core/ingestion/scope-extractor.ts index 231d37c97..63a4058fc 100644 --- a/gitnexus/src/core/ingestion/scope-extractor.ts +++ b/gitnexus/src/core/ingestion/scope-extractor.ts @@ -1149,6 +1149,12 @@ function pass5CollectReferences( // languages whose read pattern has no call-position exclusion; absent // everywhere else, so the site stays byte-identical for them. const inCalleePosition = match['@reference.callee-position'] !== undefined; + // Pointer-embedding marker: `struct S { *T }` rather than `struct S { T }`. + // Recorded, not acted on — Go's method-set rules make the two forms differ + // (see `ReferenceSite.embeddedAsPointer`), and only structural interface + // detection knows what to do with that. Absent for every language without + // pointer embedding, so their sites stay byte-identical. + const embeddedAsPointer = match['@reference.embedded-pointer'] !== undefined; const site: ReferenceSite = { name: nameCap.text, @@ -1168,6 +1174,7 @@ function pass5CollectReferences( ...(argumentTypeClasses !== undefined ? { argumentTypeClasses } : {}), ...(receiverChain !== undefined ? { receiverChain } : {}), ...(inCalleePosition ? { inCalleePosition: true } : {}), + ...(embeddedAsPointer ? { embeddedAsPointer: true } : {}), }; referenceSites.push(site); } @@ -1569,6 +1576,7 @@ const KNOWN_SUB_TAGS: ReadonlySet = new Set([ '@reference.qualified-name', '@reference.property-key', '@reference.callee-position', + '@reference.embedded-pointer', '@reference.receiver', '@reference.operator', '@reference.arity', diff --git a/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts b/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts index aa0641367..089d98364 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts @@ -113,8 +113,9 @@ * 3. Case 0.5 implicit-`this` chain walk — GATED: fires only for * languages that set `resolveThisViaEnclosingClass === true`; * it intercepts every bare-`this` call/read/write site ahead of - * Case 4 and does NOT emit Case 4's interface-dispatch fan-out, - * so enabling the toggle for a language changes that language's + * Case 4 and does NOT emit the interface-dispatch fan-out that + * Cases 0 and 4 both perform (Case 0 gained it in #2829), so + * enabling the toggle for a language changes that language's * `this` dispatch semantics (see the toggle's doc below) * 4. Case 1 namespace-receiver * 5. Case 2 class-name receiver @@ -319,6 +320,13 @@ export type ElementAccessRoute = | { readonly kind: 'index' } | { readonly kind: 'accessor'; readonly name: string }; +/** One structurally-detected implementor plus the receiver form in which it + * satisfies the interface (see `detectInterfaceImplementations`). */ +export interface StructuralImplementor { + readonly structDefId: string; + readonly receiverForm: 'value' | 'pointer'; +} + export interface ScopeResolver { /** Identity for telemetry + per-language flag check. */ readonly language: SupportedLanguages; @@ -1230,14 +1238,23 @@ export interface ScopeResolver { * Languages like Go use structural typing — a struct satisfies an * interface if its method set is a superset, without an explicit * `implements` keyword. Runs after finalize, before resolution passes. - * Returns: Map. + * Returns: Map, where each entry + * names the implementing type AND the form in which it implements. + * + * `receiverForm` is not a confidence signal — it is the language's own + * distinction. In Go the method set of `T` and of `*T` differ (a + * pointer-receiver method belongs only to `*T`), so `receiverForm: 'pointer'` + * means the VALUE type does not implement the interface and only `*T` does. + * `'value'` means both do. Consumers that only want blast radius can ignore + * it; consumers reasoning about assignability must not. + * * Default: undefined (no structural interface detection). */ readonly detectInterfaceImplementations?: ( parsedFiles: readonly ParsedFile[], indexes: ScopeResolutionIndexes, model: SemanticModel, - ) => Map; + ) => Map; /** * Optional: mirror typeBindings from namespace-import target modules diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts index e75aa0045..1202687a4 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts @@ -8,14 +8,18 @@ * * 1. **super branch** — `provider.isSuperReceiver(receiverName)` → * MRO walk skipping self - * 2. **Case 0 (compound)** — receiver has `.` or `(` → compound resolver + * 2. **Case 0 (compound)** — receiver has `.` or `(` → compound resolver. + * Also emits the interface-dispatch fan-out when the folded receiver type + * is an Interface (#2829) — see Case 4, which does the same. * 3. **Case 0.5 (implicit `this` receiver)** — GATED: fires only when * the language sets `resolveThisViaEnclosingClass === true` AND the * receiver is literally `this` → enclosing-class + MRO chain walk * with C++ member-name-hiding semantics. Languages that leave the * toggle unset skip this case entirely; their `this` sites fall * through to Case 4 via the synthesized `this` typeBinding (which - * also emits interface-dispatch fan-out that this case does not). + * emits the interface-dispatch fan-out that this case does not — + * as does Case 0 since #2829; Case 0.5 remains the only resolving + * case without it). * 4. **Case 1 (namespace)** — receiver in `namespaceTargets` → exported def * 5. **Case 2 (class-name / static receiver)** — receiver resolves to a * class-like binding (Class/Interface/Struct/Record/Enum/Trait) → MRO @@ -283,6 +287,38 @@ export function classifyReceiverOrigin( return 'unknown'; } +/** + * Upper bound on how many implementors ONE interface member may fan out to at a + * single call site (#2829). + * + * Mirrors `MAX_PROPERTY_DISPATCH_FANOUT` in `property-dispatch.ts`, deliberately + * including its reporting half: a bare cap would silently discard valid dispatch + * targets, which is the same false-safe silence #2813 was filed about. The + * default matches that sibling's 32 — the fan-out is a per-call-site product, so + * an interface with hundreds of implementors (mock proliferation is the usual + * cause) multiplies the graph without adding information a reader can act on. + * + * Override with `GITNEXUS_MAX_INTERFACE_DISPATCH_FANOUT` for a repo with + * legitimately high implementor counts. + */ +export const MAX_INTERFACE_DISPATCH_FANOUT = (() => { + const env = Number(process.env.GITNEXUS_MAX_INTERFACE_DISPATCH_FANOUT); + return Number.isInteger(env) && env >= 1 ? env : 32; +})(); + +/** Bound on the sample of over-cap interface members kept for the warning. */ +const MAX_REPORTED_SKIPPED_INTERFACES = 20; + +/** What `emitReceiverBoundCalls` reports back to the orchestrator. */ +export interface ReceiverBoundResult { + /** CALLS/ACCESSES edges emitted by this pass. */ + readonly emitted: number; + /** Dispatch targets DROPPED because a member exceeded the fan-out cap. */ + readonly dispatchFanoutSkipped: number; + /** Bounded sample naming which interface members lost targets. */ + readonly dispatchFanoutSkippedNames: readonly string[]; +} + export function emitReceiverBoundCalls( graph: KnowledgeGraph, scopes: ScopeResolutionIndexes, @@ -308,8 +344,10 @@ export function emitReceiverBoundCalls( * edge. */ readonly isBuiltInName?: (name: string) => boolean; } = {}, -): number { +): ReceiverBoundResult { let emitted = 0; + let dispatchFanoutSkipped = 0; + const dispatchFanoutSkippedNames: string[] = []; // Per-pass dedup so the multiple cases don't double-emit if two of // them resolve the same site to the same target. NEVER pre-seed // from the reference index — see Contract Invariant I5. @@ -345,23 +383,73 @@ export function emitReceiverBoundCalls( if (graphId !== undefined) graphIdToClassDef.set(graphId, def); } } - const implementorsByInterfaceDefId = new Map(); - for (const rel of graph.iterRelationshipsByType('IMPLEMENTS')) { - const ifaceDef = graphIdToClassDef.get(rel.targetId); - const implDef = graphIdToClassDef.get(rel.sourceId); - if (ifaceDef === undefined || implDef === undefined) continue; - let list = implementorsByInterfaceDefId.get(ifaceDef.nodeId); + // Direct subtypes of a type, keyed by the SUPERtype's def id. + // + // Built from IMPLEMENTS **and** EXTENDS (#2829). IMPLEMENTS alone is not the + // set of implementations: `preEmitInheritanceEdges` classifies heritage by the + // TARGET's kind, so `interface B extends A` is stored as `B IMPLEMENTS A` and + // an INTERFACE lands in A's list; and a concrete class reaches its interface + // through `class C extends AbstractBase` (EXTENDS) + `AbstractBase implements + // I` (IMPLEMENTS), so it is two hops away and invisible to a depth-1 walk. + // Both shapes previously ended the fan-out on a bodiless declaration while the + // only executable target got no edge at all. + const subtypesBySupertypeDefId = new Map(); + const addSubtype = (superId: string, sub: SymbolDefinition): void => { + let list = subtypesBySupertypeDefId.get(superId); if (list === undefined) { list = []; - implementorsByInterfaceDefId.set(ifaceDef.nodeId, list); + subtypesBySupertypeDefId.set(superId, list); + } + list.push(sub); + }; + for (const relType of ['IMPLEMENTS', 'EXTENDS'] as const) { + for (const rel of graph.iterRelationshipsByType(relType)) { + const superDef = graphIdToClassDef.get(rel.targetId); + const subDef = graphIdToClassDef.get(rel.sourceId); + if (superDef === undefined || subDef === undefined) continue; + addSubtype(superDef.nodeId, subDef); } - list.push(implDef); } - /** Emit secondary CALLS edges with reason='interface-dispatch' - * when the primary receiver-typed edge targeted an Interface's - * method. Each implementing class's same-named method gets a - * secondary edge (excluding the primary target itself). */ + /** + * Is this member a bodiless DECLARATION rather than an implementation? + * + * An interface method, and an `abstract` method on an abstract base, are + * both declarations: dispatching to them names something with no body while + * the executable target sits further down the hierarchy. `isAbstract` lives + * on the graph NODE (the structure phase sets it), not on `SymbolDefinition`, + * so this resolves the def to its node. A def that cannot be resolved is + * treated as NOT abstract — the fail-open direction, matching how the rest of + * this pass treats an unresolvable lookup. + */ + const isDeclarationOnly = (def: SymbolDefinition): boolean => { + const graphId = resolveDefGraphId(def.filePath, def, nodeLookup); + if (graphId === undefined) return false; + return graph.getNode(graphId)?.properties.isAbstract === true; + }; + + /** + * Emit secondary CALLS edges with reason='interface-dispatch' when the primary + * receiver-typed edge targeted an Interface's method. + * + * Walks the SUBTYPE CLOSURE of the interface rather than its direct + * implementors (#2829). Two shapes made the depth-1 walk both wrong and + * incomplete, each reproduced in plain Java: + * + * interface ReadCloser extends Reader { int read(String p); } // re-declares + * abstract class AbstractHandler implements Handler { public abstract void handle(String s); } + * + * In both, the depth-1 list contains a type whose `read`/`handle` is a bodiless + * declaration, so the fan-out emitted an edge to *that* — while the only class + * with a body (`FileRC`, `RealHandler`) is one hop further down and received + * nothing at all. Descending and skipping declarations fixes both directions + * at once: the wrong edge disappears and the real implementation gains one. + * + * Descent continues THROUGH a type that supplied a concrete member, because an + * override further down is an equally real runtime target — dispatch is an + * over-approximation by design, and stopping early would silently prefer the + * base. + */ const emitInterfaceDispatchFor = ( ownerDef: SymbolDefinition, memberName: string, @@ -371,19 +459,53 @@ export function emitReceiverBoundCalls( calleeCapture: CalleeIdCaptureCtx | undefined, ): number => { if (ownerDef.type !== 'Interface') return 0; - const impls = implementorsByInterfaceDefId.get(ownerDef.nodeId); - if (impls === undefined) return 0; - let n = 0; - for (const implDef of impls) { - const implMember = pickOverload(implDef.nodeId, memberName, site, model, provider); - if ( - implMember === undefined || - implMember === OVERLOAD_AMBIGUOUS || - implMember.isDeleted === true - ) { - continue; + if (subtypesBySupertypeDefId.get(ownerDef.nodeId) === undefined) return 0; + + // Collect concrete targets across the closure first, so the cap below counts + // real dispatch targets rather than types visited. + const targets: SymbolDefinition[] = []; + const seenTypes = new Set([ownerDef.nodeId]); + const queue: string[] = [ownerDef.nodeId]; + while (queue.length > 0) { + const superId = queue.shift() as string; + for (const subDef of subtypesBySupertypeDefId.get(superId) ?? []) { + if (seenTypes.has(subDef.nodeId)) continue; + seenTypes.add(subDef.nodeId); + queue.push(subDef.nodeId); + const implMember = pickOverload(subDef.nodeId, memberName, site, model, provider); + if ( + implMember === undefined || + implMember === OVERLOAD_AMBIGUOUS || + implMember.isDeleted === true + ) { + continue; + } + if (implMember.nodeId === primaryMemberDef.nodeId) continue; + // A re-declared interface method or an `abstract` override is not an + // implementation — keep descending past it rather than emitting to it. + if (isDeclarationOnly(implMember)) continue; + targets.push(implMember); } - if (implMember.nodeId === primaryMemberDef.nodeId) continue; + } + + // Bounded, and NEVER silently (#2829). An interface with a very large + // implementor set multiplies edges by every call site — Go, TypeScript and + // Kotlin do not set `collapseMemberCallsByCallerTarget`, so the product is + // per SITE. Truncating without saying so would recreate the false-safe + // silence this whole issue is about, which is why the sibling + // `MAX_PROPERTY_DISPATCH_FANOUT` reports its dropped keys too. + if (targets.length > MAX_INTERFACE_DISPATCH_FANOUT) { + dispatchFanoutSkipped += targets.length - MAX_INTERFACE_DISPATCH_FANOUT; + if (dispatchFanoutSkippedNames.length < MAX_REPORTED_SKIPPED_INTERFACES) { + dispatchFanoutSkippedNames.push( + `${ownerDef.qualifiedName ?? ownerDef.nodeId}.${memberName} (${targets.length} targets)`, + ); + } + targets.length = MAX_INTERFACE_DISPATCH_FANOUT; + } + + let n = 0; + for (const implMember of targets) { const ok = tryEmitEdge( graph, scopes, @@ -644,6 +766,48 @@ export function emitReceiverBoundCalls( calleeCapture, ); if (ok) emitted++; + // Interface dispatch, exactly as Case 4 does it (#2813). When the + // folded receiver type is an Interface, the primary edge above + // lands on the interface's own method DECLARATION; these secondary + // edges are what reach the implementations. + // + // Case 4 had this and Case 0 did not, which made the gap a property + // of receiver SYNTAX rather than of types: a struct-field receiver + // (`s.orderRepo`) contains a dot, so it always takes Case 0, while + // the same interface reached through a local or parameter is a bare + // name and reaches Case 4. Field-held interfaces — dependency + // injection, in other words — were the half that silently lost every + // implementation edge. + // + // `currentClass` is the receiver's own folded type, matching what + // Case 4 passes. `emitInterfaceDispatchFor` self-gates on + // `ownerDef.type !== 'Interface'`, so this is inert for every + // concrete receiver and needs no language check of its own — a + // member whose owner resolved to a Struct emits nothing extra. + // + // Confidence mirrors THIS case's own primary emit above — the 0.85 + // literal — so a site's dispatch edges never claim more certainty + // than the edge they hang off. + // + // Case 4 passes a site.kind-dependent value instead (1.0 for + // read/write, `:1399-1405`) because ITS primary varies the same way. + // Case 0 does branch on `site.kind` when picking the member + // (`:713-716`), it simply does not vary reason/confidence with it, + // so there is no 1.0 arm here to mirror. That leaves a read/write + // ACCESSES through a compound receiver at 0.85 while the same access + // through a bare name is 1.0 — a PRE-EXISTING difference between the + // two cases' primaries, not something this fan-out introduces. + // Deliberately not "fixed" here: changing Case 0's primary + // confidence is a separate behavioural change affecting every + // language, and is out of scope for #2813. + emitted += emitInterfaceDispatchFor( + currentClass, + memberName, + memberDef, + site, + 0.85, + calleeCapture, + ); // Always mark handled when the site was resolved, even // if the edge was deduplicated (collapse mode), so // `emitReferencesViaLookup` doesn't re-emit from the @@ -1541,7 +1705,7 @@ export function emitReceiverBoundCalls( } } - return emitted; + return { emitted, dispatchFanoutSkipped, dispatchFanoutSkippedNames }; } /** Resolve a member by name on a class def, narrowing by argument diff --git a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts index e11ae7c9a..72f1b1168 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/pipeline/run.ts @@ -66,7 +66,10 @@ import type { FunctionCfg } from '../../cfg/types.js'; import { resolveDefGraphId } from '../graph-bridge/ids.js'; import { buildPopulatedMethodDispatch } from '../graph-bridge/method-dispatch.js'; import { propagateImportedReturnTypes } from '../passes/imported-return-types.js'; -import { emitReceiverBoundCalls } from '../passes/receiver-bound-calls.js'; +import { + emitReceiverBoundCalls, + MAX_INTERFACE_DISPATCH_FANOUT, +} from '../passes/receiver-bound-calls.js'; import { emitFreeCallFallback } from '../passes/free-call-fallback.js'; import { emitPropertyDispatchCalls, @@ -244,8 +247,8 @@ function emitDetectedInterfaceImplementations( for (const [interfaceDefId, implementorDefIds] of detected) { const targetId = graphIdByDefId.get(interfaceDefId); if (targetId === undefined) continue; - for (const implementorDefId of implementorDefIds) { - const sourceId = graphIdByDefId.get(implementorDefId); + for (const implementor of implementorDefIds) { + const sourceId = graphIdByDefId.get(implementor.structDefId); if (sourceId === undefined) continue; const edgeKey = `${sourceId}->${targetId}`; if (existing.has(edgeKey)) continue; @@ -256,7 +259,17 @@ function emitDetectedInterfaceImplementations( targetId, type: 'IMPLEMENTS', confidence: 0.85, - reason: `${provider.language}-structural-implements`, + // The receiver form rides in `reason` because relationships carry no + // arbitrary properties — adding one would change the relation DDL and + // move SCHEMA_FINGERPRINT, forcing a full re-analyze for a fact that a + // string already expresses. `-pointer` means ONLY the pointer type + // implements: `var x I = T{}` is invalid, `var x I = &T{}` is fine. + // The unsuffixed form is unchanged from before, so a consumer matching + // the old string keeps seeing exactly the value-form implementors. + reason: + implementor.receiverForm === 'pointer' + ? `${provider.language}-structural-implements-pointer` + : `${provider.language}-structural-implements`, }); emitted++; } @@ -796,8 +809,12 @@ export function runScopeResolution( : (filePath, line, col) => callableArgumentSites.has(`${filePath}:${line}:${col}`), ) : undefined; - const receiverExtras = callableFlowOnly - ? 0 + const receiverBound = callableFlowOnly + ? { + emitted: 0, + dispatchFanoutSkipped: 0, + dispatchFanoutSkippedNames: [] as readonly string[], + } : emitReceiverBoundCalls( graph, indexes, @@ -816,6 +833,22 @@ export function runScopeResolution( isBuiltInName: provider.languageProvider.isBuiltInName, }, ); + const receiverExtras = receiverBound.emitted; + if (receiverBound.dispatchFanoutSkipped > 0) { + // Never drop dispatch coverage silently (#2829) — same contract as the + // property-dispatch cap below. An interface member over the cap loses real + // implementors, so `impact()` on those implementations under-reports; an + // operator has to be able to see WHICH member lost them. + logger.warn( + { + lang: provider.language, + dispatchFanoutSkipped: receiverBound.dispatchFanoutSkipped, + dispatchFanoutSkippedNames: receiverBound.dispatchFanoutSkippedNames, + fanoutCap: MAX_INTERFACE_DISPATCH_FANOUT, + }, + 'interface-dispatch: members over the fan-out cap dropped implementors (their CALLS edges were not emitted)', + ); + } const unresolvedReceiverExtras = !callableFlowOnly && provider.emitUnresolvedReceiverEdges !== undefined ? provider.emitUnresolvedReceiverEdges( diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index 8b4880554..a5688b77f 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -205,7 +205,24 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j // they replay the pre-fix capture set for byte-unchanged files and keep serving // the fabricated edge. main is at 39, so 40/41/42 are all this branch's. // RE-CHECK AGAINST origin/main IMMEDIATELY BEFORE MERGING. -const SCHEMA_BUMP = 42; +// v43: Go embedded fields now emit `@reference.embedded-pointer` when written +// as `*T` rather than `T` (#2813 exact method sets). Go's method-set rules make +// the two forms genuinely different — `struct{ Base }` does NOT get Base's +// pointer-receiver methods in its value method set while `struct{ *Base }` does +// — so structural interface satisfaction cannot be exact without the spelling. +// This is PARSE-TIME capture emission, so a warm cache replays the pre-fix +// capture set for byte-unchanged files and the distinction never appears: +// silently, with no error, the v27/v30 failure mode. `analyze` skips tree-sitter +// dispatch for unchanged chunks (GUARDRAILS.md), so a plain re-analyze does NOT +// surface it without this bump. +// +// 42 -> 43: this branch originally allocated 43 while sitting at 39, because +// main had already taken 40/41/42 for #2807. main has since merged those and +// this branch rebased onto it, so 43 remains the next free number and the +// value is unchanged by the rebase — the reason it was chosen is simply now +// visible in the history above. RE-CHECK AGAINST origin/main IMMEDIATELY +// BEFORE MERGING; this file records eight prior collisions, two EXACT. +const SCHEMA_BUMP = 43; const GITNEXUS_PKG_VERSION = (() => { try { // package.json sits at gitnexus/package.json — two levels up from diff --git a/gitnexus/test/fixtures/go-captures-golden/expected-captures.json b/gitnexus/test/fixtures/go-captures-golden/expected-captures.json index 0205468f6..3065bfa0d 100644 --- a/gitnexus/test/fixtures/go-captures-golden/expected-captures.json +++ b/gitnexus/test/fixtures/go-captures-golden/expected-captures.json @@ -127,6 +127,38 @@ "captureGroups": 23, "digest": "fd35d917bc7a039ef4bcb0a6501c31ffbea0fe39bf3604938191597415aa33f7" }, + "go-interface-field-dispatch/internal/handlers/orders.go": { + "captureGroups": 22, + "digest": "438b02c0990fa6c830767327d07de642f7aff6287e95217c3dfd7423a520942c" + }, + "go-interface-field-dispatch/internal/handlers/picking.go": { + "captureGroups": 28, + "digest": "0ad401ba19e7cf7deace88ea7973572cb6ee2a63a6006aa5cc06f55bac56e14b" + }, + "go-interface-field-dispatch/internal/repository/audit_repo.go": { + "captureGroups": 18, + "digest": "2710feb4bee288248e346d7ab0a32a8511b002d8c0fad4cd7db3588ee1926bc2" + }, + "go-interface-field-dispatch/internal/repository/interfaces.go": { + "captureGroups": 10, + "digest": "ee5b78fc03ea3283b2e06b9b8d0cf4999c2de091262571966d1d4670036c479d" + }, + "go-interface-field-dispatch/internal/repository/mock_repo.go": { + "captureGroups": 47, + "digest": "7b9219cc06e8c45fb08c304b71ebc58e1a9e052cb9c92fd01b9f22ab8328a91d" + }, + "go-interface-field-dispatch/internal/repository/order_repo.go": { + "captureGroups": 26, + "digest": "00e0f98586043ff84f96af92d9e6b3593018b6b37f92d183b5c05c6de745e2b0" + }, + "go-interface-field-dispatch/internal/services/pick_service.go": { + "captureGroups": 39, + "digest": "036d1a4b7304bbbf963f640710fd286702ed34809df2940411c14da8f9de9a6f" + }, + "go-interface-field-dispatch/internal/services/wave_service.go": { + "captureGroups": 46, + "digest": "62a3830311106e4a349c5b604e914531249aacc47001a888ecf5d38e2bc4c821" + }, "go-local-shadow/cmd/main.go": { "captureGroups": 14, "digest": "ce7220df98df08741f772150b5572bec0364d576741bc8c76647615fab859f92" @@ -285,7 +317,7 @@ }, "go-qualified-base/consumers/qualified.go": { "captureGroups": 14, - "digest": "01fe99cadcdf3ce6f00cafce8db0474ad152983f766c8dee9b9da42c9256ae2f" + "digest": "4680bb7a7c62a54a91d1d7b8ab3150dfb0c54b1683bee8cf7fc747afe99ca0f8" }, "go-receiver-method-free-call/example.go": { "captureGroups": 8, diff --git a/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/go.mod b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/go.mod new file mode 100644 index 000000000..02eafb037 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/go.mod @@ -0,0 +1,3 @@ +module github.com/example/wms + +go 1.21 diff --git a/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/handlers/orders.go b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/handlers/orders.go new file mode 100644 index 000000000..de3e40fc5 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/handlers/orders.go @@ -0,0 +1,13 @@ +package handlers + +import "github.com/example/wms/internal/repository" + +type OrderHandlers struct { + repo repository.OrderRepository + auditRepo *repository.AuditRepo +} + +func (h *OrderHandlers) Delete(id string) error { + h.auditRepo.LogAuditEventAsync("delete") + return h.repo.DeleteItem(id) +} diff --git a/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/handlers/picking.go b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/handlers/picking.go new file mode 100644 index 000000000..f71db2ec0 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/handlers/picking.go @@ -0,0 +1,15 @@ +package handlers + +import "github.com/example/wms/internal/repository" + +type PickHandlers struct { + repo repository.OrderRepository +} + +func (h *PickHandlers) Queue(id string) ([]string, error) { + return h.repo.GetPickQueue(id) +} + +func (h *PickHandlers) Unsplit(id string) error { + return h.repo.UnsplitOrder(id) +} diff --git a/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/repository/audit_repo.go b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/repository/audit_repo.go new file mode 100644 index 000000000..ad115ff0a --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/repository/audit_repo.go @@ -0,0 +1,12 @@ +package repository + +// AuditRepo and CartRepo are the CONCRETE-field controls: calls through a +// concrete-typed field already resolved before #2813 and must keep resolving +// to the implementation, with no interface-dispatch fan-out. +type AuditRepo struct{} + +func (a *AuditRepo) LogAuditEventAsync(msg string) {} + +type CartRepo struct{} + +func (c *CartRepo) Get(id string) string { return "" } diff --git a/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/repository/interfaces.go b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/repository/interfaces.go new file mode 100644 index 000000000..bd3da43d4 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/repository/interfaces.go @@ -0,0 +1,18 @@ +package repository + +// OrderRepository is satisfied only by types whose methods use POINTER +// receivers (#2813). In Go the method set of *T includes both value- and +// pointer-receiver methods, so *OrderRepo implements this interface even +// though OrderRepo (the value type) does not. +type OrderRepository interface { + DeleteItem(id string) error + GetPickQueue(id string) ([]string, error) + UnsplitOrder(id string) error +} + +// PartialRepository is deliberately satisfied by NOTHING in this fixture — +// the negative control for structural detection. +type PartialRepository interface { + DeleteItem(id string) error + NeverImplemented(id string) error +} diff --git a/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/repository/mock_repo.go b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/repository/mock_repo.go new file mode 100644 index 000000000..78261cdb8 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/repository/mock_repo.go @@ -0,0 +1,23 @@ +package repository + +// MockOrderRepo mirrors the mock proliferation of a real codebase: a second +// pointer-receiver implementor, so the fan-out must reach BOTH. +type MockOrderRepo struct{} + +func (m *MockOrderRepo) DeleteItem(id string) error { return nil } + +func (m *MockOrderRepo) GetPickQueue(id string) ([]string, error) { return nil, nil } + +func (m *MockOrderRepo) UnsplitOrder(id string) error { return nil } + +// WrongSigRepo has every method NAME the interface requires, at the same arity, +// but with an incompatible parameter type. It must NOT be detected as an +// implementor — signature comparison is the only guard left now that +// pointer-receiver methods count toward the method set (#2813). +type WrongSigRepo struct{} + +func (w *WrongSigRepo) DeleteItem(id int) error { return nil } + +func (w *WrongSigRepo) GetPickQueue(id int) ([]string, error) { return nil, nil } + +func (w *WrongSigRepo) UnsplitOrder(id int) error { return nil } diff --git a/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/repository/order_repo.go b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/repository/order_repo.go new file mode 100644 index 000000000..081f50c69 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/repository/order_repo.go @@ -0,0 +1,11 @@ +package repository + +type OrderRepo struct { + dsn string +} + +func (r *OrderRepo) DeleteItem(id string) error { return nil } + +func (r *OrderRepo) GetPickQueue(id string) ([]string, error) { return nil, nil } + +func (r *OrderRepo) UnsplitOrder(id string) error { return nil } diff --git a/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/services/pick_service.go b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/services/pick_service.go new file mode 100644 index 000000000..cbd910d1c --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/services/pick_service.go @@ -0,0 +1,21 @@ +package services + +import "github.com/example/wms/internal/repository" + +// PickService holds BOTH an interface-typed field and concrete-typed fields, +// so one fixture covers the failing case and its control. +type PickService struct { + orderRepo repository.OrderRepository + cartRepo *repository.CartRepo + auditRepo *repository.AuditRepo +} + +func (s *PickService) GetPickQueue(id string) ([]string, error) { + s.auditRepo.LogAuditEventAsync("pick") + return s.orderRepo.GetPickQueue(id) +} + +func (s *PickService) StartSession(id string) error { + _ = s.cartRepo.Get(id) + return s.orderRepo.DeleteItem(id) +} diff --git a/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/services/wave_service.go b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/services/wave_service.go new file mode 100644 index 000000000..7ea0d25de --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/go-interface-field-dispatch/internal/services/wave_service.go @@ -0,0 +1,28 @@ +package services + +import "github.com/example/wms/internal/repository" + +// WaveService declares the SAME field shape as PickService — the issue +// reported these two behaving differently at scale (#2813). +type WaveService struct { + orderRepo repository.OrderRepository + orderRepo2 *repository.OrderRepo + auditRepo *repository.AuditRepo +} + +func (s *WaveService) Release(id string) error { + s.auditRepo.LogAuditEventAsync("wave") + return s.orderRepo.DeleteItem(id) +} + +func (s *WaveService) Queue(id string) ([]string, error) { + return s.orderRepo.GetPickQueue(id) +} + +// Recount calls through a CONCRETE field whose own type (*OrderRepo) is itself +// an implementor of OrderRepository. This is the shape where the dispatch type +// gate is load-bearing: the call must resolve to OrderRepo only and must NOT +// fan out to the sibling implementor MockOrderRepo (#2829 review). +func (s *WaveService) Recount(id string) error { + return s.orderRepo2.UnsplitOrder(id) +} diff --git a/gitnexus/test/integration/resolvers/go.test.ts b/gitnexus/test/integration/resolvers/go.test.ts index 7ccea1aeb..3f92964ae 100644 --- a/gitnexus/test/integration/resolvers/go.test.ts +++ b/gitnexus/test/integration/resolvers/go.test.ts @@ -355,14 +355,20 @@ describe('Go structural interface dispatch', () => { } it('emits signature-checked structural IMPLEMENTS edges only for valid implementors', () => { - const implementsEdges = getRelationships(result, 'IMPLEMENTS').filter( - (edge) => edge.rel.reason === 'go-structural-implements', + const implementsEdges = getRelationships(result, 'IMPLEMENTS').filter((edge) => + (edge.rel.reason ?? '').startsWith('go-structural-implements'), ); expect(edgeSet(implementsEdges)).toEqual([ 'File → ReadCloser', 'File → Reader', 'FileBase → Reader', 'MemoryRepository → Repository', + // ADDED in #2813, same deliberate reversal as the PointerOnlyThing pin + // below: `func (p *PointerOnlyThing) Touch()` puts Touch in the method + // set of *PointerOnlyThing, which is the type idiomatic Go stores in a + // PointerOnly-typed field. This exact-set assertion was the second place + // the #1966 value-only reading was encoded. + 'PointerOnlyThing → PointerOnly', 'SqlRepository → Repository', ]); expect(implementsEdges.every((edge) => edge.rel.confidence === 0.85)).toBe(true); @@ -409,9 +415,36 @@ describe('Go structural interface dispatch', () => { expect(edgeSet(implementsEdges)).not.toContain('ShadowReadFile → ReadCloser'); }); - it('does not emit value-type IMPLEMENTS for pointer-receiver-only methods', () => { + // POLARITY DELIBERATELY REVERSED in #2813 (was: `.not.toContain`). + // + // #1966 read Go's method-set rule for the VALUE type `T`, where pointer- + // receiver methods genuinely do not count. But GitNexus has one Struct node + // per type and no separate `*T` node, so that reading left `*T` — the shape + // idiomatic Go stores in an interface-typed field — unable to implement + // anything. The cost was silence, not caution: calls through such a field + // stopped at the interface declaration and `impact()` on the implementation + // reported zero callers. See the rationale block in interface-impls.ts. + it('emits IMPLEMENTS for a pointer-receiver-only implementor', () => { const implementsEdges = getRelationships(result, 'IMPLEMENTS'); - expect(edgeSet(implementsEdges)).not.toContain('PointerOnlyThing → PointerOnly'); + expect(edgeSet(implementsEdges)).toContain('PointerOnlyThing → PointerOnly'); + }); + + // The FORM is the exact fact, not a confidence hedge. `func (p *PointerOnlyThing) + // Touch()` puts Touch in MS(*PointerOnlyThing) only, so `var x PointerOnly = + // PointerOnlyThing{}` is a Go compile error while `&PointerOnlyThing{}` is fine. + // Value-satisfying implementors keep the unsuffixed reason. + it('records WHICH method set satisfies the interface', () => { + const byPair = new Map( + getRelationships(result, 'IMPLEMENTS').map((e) => [ + `${e.source} → ${e.target}`, + e.rel.reason, + ]), + ); + expect(byPair.get('PointerOnlyThing → PointerOnly')).toBe('go-structural-implements-pointer'); + // SqlRepository/MemoryRepository use VALUE receivers, so the value type + // itself implements and the reason stays unsuffixed. + expect(byPair.get('SqlRepository → Repository')).toBe('go-structural-implements'); + expect(byPair.get('MemoryRepository → Repository')).toBe('go-structural-implements'); }); it('fans out embedded-interface receivers only to complete implementors', () => { @@ -446,8 +479,8 @@ describe('Go cross-package structural interface dispatch', () => { } it('matches local interface types against package-qualified implementation signatures', () => { - const implementsEdges = getRelationships(result, 'IMPLEMENTS').filter( - (edge) => edge.rel.reason === 'go-structural-implements', + const implementsEdges = getRelationships(result, 'IMPLEMENTS').filter((edge) => + (edge.rel.reason ?? '').startsWith('go-structural-implements'), ); expect(edgeSet(implementsEdges)).toEqual([ 'File → ReadCloser', @@ -1794,3 +1827,203 @@ describe('Go pointer-receiver field chains (#2766)', () => { expect(imports.map((e) => `${e.source} → ${e.target}`)).toContain('handler.go → repo.go'); }); }); + +// --------------------------------------------------------------------------- +// #2813: calls through an interface-typed struct field must reach the +// IMPLEMENTATION, not stop at the interface declaration. +// --------------------------------------------------------------------------- +// +// Two stacked defects produced the reported symptom, and either alone is +// enough to reproduce it — which is why the pre-existing fixtures, uniformly +// value-receiver, could not observe it: +// +// D1 `buildDetectionIndexes` skipped every POINTER-receiver method, so a +// struct whose methods are all `func (r *T)` had an empty method set, +// structurally satisfied nothing, and got no IMPLEMENTS edge. Go's rule +// is that the method set of *T includes pointer-receiver methods, and +// idiomatic Go stores *T in an interface-typed field. +// D2 Case 0 (compound receiver) emitted its primary edge and short-circuited +// without the interface-dispatch fan-out Case 4 performs. A struct-field +// receiver `s.orderRepo` contains a dot, so it always takes Case 0; a +// local or parameter receiver is a bare name and reaches Case 4. +// +// The fixture is deliberately pointer-receiver throughout, cross-package, and +// carries concrete-field controls in the same structs. +describe('Go interface-typed struct field dispatch (#2813)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'go-interface-field-dispatch'), + () => {}, + ); + }, 120000); + + const calls = (): string[] => edgeSet(getRelationships(result, 'CALLS')); + const implementsEdges = (): string[] => edgeSet(getRelationships(result, 'IMPLEMENTS')); + /** CALLS rows carrying the emitting reason, so a fan-out edge is + * distinguishable from the primary edge to the interface declaration. */ + const callsWithReason = (): string[] => + getRelationships(result, 'CALLS').map((e) => `${e.source} → ${e.target}:${e.rel.reason}`); + /** CALLS rows qualified by the target's FILE — `→ DeleteItem` alone cannot + * tell the interface declaration apart from the implementation, which is + * the entire distinction under test. */ + const callsToFile = (): string[] => + getRelationships(result, 'CALLS').map( + (e) => `${e.source} → ${e.target}@${e.targetFilePath.split('/').slice(-1)[0]}`, + ); + /** Both ENDS file-qualified. Required wherever two callers share a method + * name — `callsToFile()` alone cannot tell them apart, so a row asserting + * per-file behaviour must use this instead. */ + const callsFromFileToFile = (): string[] => + getRelationships(result, 'CALLS').map( + (e) => + `${e.sourceFilePath.split('/').slice(-1)[0]}:${e.source} → ` + + `${e.target}@${e.targetFilePath.split('/').slice(-1)[0]}`, + ); + /** CALLS rows carrying the emitted confidence, so a change to the literal + * is caught rather than silently accepted. */ + const callsWithConfidence = (): string[] => + getRelationships(result, 'CALLS').map( + (e) => + `${e.source} → ${e.target}@${e.targetFilePath.split('/').slice(-1)[0]}=${e.rel.confidence}`, + ); + + // D1: a pointer-receiver implementor must be discoverable at all. + it('detects a pointer-receiver struct as an interface implementor', () => { + expect(implementsEdges()).toContain('OrderRepo → OrderRepository'); + }); + + it('detects every pointer-receiver implementor, not just the first', () => { + expect(implementsEdges()).toContain('MockOrderRepo → OrderRepository'); + }); + + // D2: the headline defect. Before the fix these were absent entirely. + it('resolves an interface-typed field call to the implementation', () => { + expect(callsToFile()).toContain('StartSession → DeleteItem@order_repo.go'); + }); + + it('resolves an interface-typed field call from a handler to the implementation', () => { + expect(callsToFile()).toContain('Delete → DeleteItem@order_repo.go'); + }); + + it('fans out to every implementor, not only the first', () => { + expect(callsToFile()).toContain('StartSession → DeleteItem@mock_repo.go'); + }); + + it('emits the implementation edge with reason interface-dispatch', () => { + expect(callsWithReason()).toContain('StartSession → DeleteItem:interface-dispatch'); + }); + + // R11-style control: the fan-out must ADD edges, never MOVE them. If the + // primary edge disappears, the fix relocated resolution instead of widening + // it and every consumer of the interface node silently loses its callers. + it('keeps the primary edge to the interface declaration', () => { + expect(callsToFile()).toContain('StartSession → DeleteItem@interfaces.go'); + }); + + // The epistemic half of #2813, pinned at its MECHANISM. + // + // The reporter's disqualifying complaint was that `impact()` returned + // `impactedCount 0, epistemic "exact"` for a method reached only through an + // interface field — byte-identical to a symbol that genuinely has no callers, + // so a zero could not be trusted defensively. That verdict is produced by + // `computeEpistemicBoundary`, which walks HERITAGE edges out of the queried + // symbol; with no IMPLEMENTS/METHOD_IMPLEMENTS edge it found no boundary, and + // the call sites were never *dropped* (they resolved, to the wrong node), so + // neither of its two producers fired. + // + // These are the edges that make the hedge fire. Measured on this fixture + // after the fix, `impact(OrderRepo.DeleteItem, upstream)` returns + // impactedCount 3 with epistemic "lower-bound" and an interface-boundary + // note, while the concrete `CartRepo.Get` still returns "exact" — so the + // signal discriminates rather than hedging on everything. Asserting the edges + // here keeps that mechanism from silently regressing without requiring the + // resolver suite to reach into the MCP layer. + it('emits METHOD_IMPLEMENTS from the implementation to the interface method', () => { + const methodImpls = getRelationships(result, 'METHOD_IMPLEMENTS').map( + (e) => + `${e.sourceFilePath.split('/').slice(-1)[0]}:${e.source} → ` + + `${e.targetFilePath.split('/').slice(-1)[0]}:${e.target}`, + ); + expect(methodImpls).toContain('order_repo.go:DeleteItem → interfaces.go:DeleteItem'); + expect(methodImpls).toContain('mock_repo.go:DeleteItem → interfaces.go:DeleteItem'); + }); + + // Concrete-field control: resolved before #2813 and must be untouched. + it('keeps resolving a concrete-typed field to its implementation', () => { + expect(callsToFile()).toContain('GetPickQueue → LogAuditEventAsync@audit_repo.go'); + }); + + // `AuditRepo` implements nothing, so it is never a key in the implementor + // index — this row is satisfied by that map miss alone and would still pass + // with the `ownerDef.type !== 'Interface'` gate deleted. Kept as a regression + // test for the user-visible property, NOT as a control for the gate. + it('does not fan out a concrete-typed field receiver that implements nothing', () => { + expect(callsWithReason()).not.toContain('GetPickQueue → LogAuditEventAsync:interface-dispatch'); + }); + + // The gate's real control. `OrderRepo` IS an implementor of `OrderRepository`, + // so it IS a live participant in the dispatch index; a call through a + // *concrete* `*OrderRepo` field must still resolve only to `OrderRepo` and + // must not fan out to its sibling implementor `MockOrderRepo`. This is the + // shape where the type gate does the work, so deleting the gate fails here. + it('does not fan out a concrete field whose own type is an implementor', () => { + expect(callsToFile()).toContain('Recount → UnsplitOrder@order_repo.go'); + expect(callsWithReason()).not.toContain('Recount → UnsplitOrder:interface-dispatch'); + expect(callsToFile()).not.toContain('Recount → UnsplitOrder@mock_repo.go'); + }); + + // Negative control for structural detection: a partial match is not an + // implementation. Without this, "everything implements everything" passes. + it('does not treat a partial signature match as an implementation', () => { + expect(implementsEdges()).not.toContain('OrderRepo → PartialRepository'); + }); + + // Signature comparison is the ONLY remaining guard now that pointer-receiver + // methods are admitted, so it needs a same-name/same-arity/different-type row + // and not just the missing-method negative above. + it('does not treat a same-name same-arity method with a different signature as an implementation', () => { + expect(implementsEdges()).not.toContain('WrongSigRepo → OrderRepository'); + }); + + // The fan-out emits at the same confidence as the primary edge it hangs off. + // Without this row the 0.85 literal can be changed with nothing failing — + // the sibling IMPLEMENTS block already pins its own confidence. + it('emits dispatch edges at the same confidence as the primary edge', () => { + expect(callsWithConfidence()).toContain('StartSession → DeleteItem@order_repo.go=0.85'); + expect(callsWithConfidence()).toContain('StartSession → DeleteItem@interfaces.go=0.85'); + }); + + // Bounds the cross product. Two implementors x the call sites below is the + // whole fan-out for this interface, so a future cap, collapse or dedup change + // becomes visible here instead of silently multiplying or truncating edges. + it('emits exactly one dispatch edge per implementor per call site', () => { + const deleteItemDispatch = getRelationships(result, 'CALLS').filter( + (e) => e.target === 'DeleteItem' && e.rel.reason === 'interface-dispatch', + ); + // 3 call sites on DeleteItem (pick_service StartSession, wave_service + // Release, handlers Delete) x 2 implementors (OrderRepo, MockOrderRepo). + expect(deleteItemDispatch.length).toBe(6); + const targets = [...new Set(deleteItemDispatch.map((e) => e.targetFilePath.split('/').pop()))]; + expect(targets.sort()).toEqual(['mock_repo.go', 'order_repo.go']); + }); + + // The issue reported these two files behaving differently at scale despite + // declaring the same field shape; both must resolve here. + // + // The SOURCE is file-qualified on purpose. Two methods in this fixture are + // named `Queue` (services/wave_service.go and handlers/picking.go), and + // `callsToFile()` qualifies only the target — so a bare `'Queue → …'` row is + // satisfied by EITHER of them and cannot detect one file resolving while the + // other does not. That per-file divergence is the exact #2813 symptom this + // row exists to catch, so it has to name both sides. + it('resolves the same field shape identically across two service files', () => { + expect(callsFromFileToFile()).toContain('wave_service.go:Release → DeleteItem@order_repo.go'); + expect(callsFromFileToFile()).toContain('wave_service.go:Queue → GetPickQueue@order_repo.go'); + expect(callsFromFileToFile()).toContain( + 'pick_service.go:StartSession → DeleteItem@order_repo.go', + ); + expect(callsFromFileToFile()).toContain('picking.go:Queue → GetPickQueue@order_repo.go'); + }); +}); diff --git a/gitnexus/test/unit/incremental-parse-cache.test.ts b/gitnexus/test/unit/incremental-parse-cache.test.ts index 1251a6eb2..ec4224b21 100644 --- a/gitnexus/test/unit/incremental-parse-cache.test.ts +++ b/gitnexus/test/unit/incremental-parse-cache.test.ts @@ -118,8 +118,10 @@ describe('PARSE_CACHE_VERSION', () => { // second clash was caught — after review, while the branch sat waiting to // merge — which is precisely the window in which `main` allocates. Re-check // against origin/main immediately before merge, not at review time. - it('pins SCHEMA_BUMP to 42 so concurrent bumps cannot silently collide (#2766)', () => { - expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(42); + // Moved 42 -> 43 for #2813's `@reference.embedded-pointer` capture, which is + // parse-time emission and so cannot be served from a v42 warm cache. + it('pins SCHEMA_BUMP to 43 so concurrent bumps cannot silently collide (#2766)', () => { + expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(43); }); it('embeds the gitnexus package version (so upgrades invalidate the cache)', () => { diff --git a/gitnexus/test/unit/scope-resolution/go/go-hooks.test.ts b/gitnexus/test/unit/scope-resolution/go/go-hooks.test.ts index 330cbb85e..d631b69a0 100644 --- a/gitnexus/test/unit/scope-resolution/go/go-hooks.test.ts +++ b/gitnexus/test/unit/scope-resolution/go/go-hooks.test.ts @@ -15,6 +15,7 @@ import { goReceiverBinding, } from '../../../../src/core/ingestion/languages/go/index.js'; import { detectGoInterfaceImplementations } from '../../../../src/core/ingestion/languages/go/interface-impls.js'; +import { populateGoOwners } from '../../../../src/core/ingestion/languages/go/method-owners.js'; describe('Go arity compatibility', () => { const makeDef = (overrides: Partial = {}): SymbolDefinition => ({ @@ -272,6 +273,23 @@ function inheritsSite(name: string, inScope: ScopeId): ReferenceSite { }; } +/** + * Structural detection now returns the receiver FORM alongside each + * implementor (`{ structDefId, receiverForm }`), because Go's method sets for + * `T` and `*T` genuinely differ. Most rows below only care about WHICH types + * implement, so this extracts the ids; the rows that care about the form assert + * it explicitly. + */ +function implIds( + result: Map, + ifaceId: string, +): string[] | undefined { + const found = result.get(ifaceId); + // Deliberately NOT sorted: several rows below assert detection ORDER + // (shallowest-promoted-first), which sorting would silently destroy. + return found === undefined ? undefined : found.map((i) => i.structDefId); +} + describe('Go structural interface detection', () => { it('detects a struct implementing every interface method with matching signatures', () => { const iface = goDef('iface:Repository', 'Interface', 'Repository'); @@ -319,10 +337,18 @@ describe('Go structural interface detection', () => { {} as any, ); - expect(result.get(iface.nodeId)).toEqual([struct.nodeId]); + expect(implIds(result, iface.nodeId)).toEqual([struct.nodeId]); }); - it('does not treat pointer-receiver-only methods as value type implementations', () => { + // POLARITY DELIBERATELY REVERSED in #2813 (was: expected `undefined`). + // A type whose methods use pointer receivers is exactly what idiomatic Go + // stores in an interface-typed field; excluding it emitted no IMPLEMENTS edge + // at all, so calls through such a field never reached the implementation. + // See the rationale block in interface-impls.ts. + // Exactness, not just presence: a pointer-receiver-only type implements the + // interface ONLY in pointer form. `var x Closer = PointerOnlyCloser{}` is a + // compile error in Go and the graph must be able to say so. + it('treats pointer-receiver-only methods as implementations in POINTER form only', () => { const iface = goDef('iface:Closer', 'Interface', 'Closer'); const struct = goDef('struct:PointerOnlyCloser', 'Struct', 'PointerOnlyCloser'); const ifaceClose = goDef('iface:Closer.Close', 'Method', 'Closer.Close', iface.nodeId, { @@ -347,7 +373,8 @@ describe('Go structural interface detection', () => { {} as any, ); - expect(result.get(iface.nodeId)).toBeUndefined(); + expect(implIds(result, iface.nodeId)).toEqual([struct.nodeId]); + expect(result.get(iface.nodeId)?.[0]?.receiverForm).toBe('pointer'); }); it('rejects same-name methods with incompatible parameter types', () => { @@ -577,7 +604,7 @@ describe('Go structural interface detection', () => { {} as any, ); - expect(result.get(readCloser.nodeId)).toEqual([struct.nodeId]); + expect(implIds(result, readCloser.nodeId)).toEqual([struct.nodeId]); }); it('accepts structs implementing interface methods through promoted embedded struct methods', () => { @@ -606,7 +633,7 @@ describe('Go structural interface detection', () => { {} as any, ); - expect(result.get(reader.nodeId)).toEqual([base.nodeId, file.nodeId]); + expect(implIds(result, reader.nodeId)).toEqual([base.nodeId, file.nodeId]); }); it('lets direct struct methods shadow promoted embedded struct methods', () => { @@ -650,7 +677,7 @@ describe('Go structural interface detection', () => { {} as any, ); - expect(result.get(reader.nodeId)).toEqual([base.nodeId]); + expect(implIds(result, reader.nodeId)).toEqual([base.nodeId]); }); it('does not use ambiguous promoted embedded struct methods for interface matching', () => { @@ -689,7 +716,7 @@ describe('Go structural interface detection', () => { {} as any, ); - expect(result.get(reader.nodeId)).toEqual([baseA.nodeId, baseB.nodeId]); + expect(implIds(result, reader.nodeId)).toEqual([baseA.nodeId, baseB.nodeId]); }); it('uses the shallowest promoted embedded struct method when deeper methods share the name', () => { @@ -734,7 +761,7 @@ describe('Go structural interface detection', () => { {} as any, ); - expect(result.get(reader.nodeId)).toEqual([ + expect(implIds(result, reader.nodeId)).toEqual([ shallow.nodeId, deepBase.nodeId, deepWrapper.nodeId, @@ -840,7 +867,7 @@ describe('Go structural interface detection', () => { {} as any, ); - expect(result.get(readCloser.nodeId)).toEqual([file.nodeId]); + expect(implIds(result, readCloser.nodeId)).toEqual([file.nodeId]); }); it('does not emit implementations for cyclic embedded interfaces', () => { @@ -908,8 +935,8 @@ describe('Go structural interface detection', () => { {} as any, ); - expect(result.get(reader.nodeId)).toEqual([file.nodeId]); - expect(result.get(closer.nodeId)).toEqual([file.nodeId]); + expect(implIds(result, reader.nodeId)).toEqual([file.nodeId]); + expect(implIds(result, closer.nodeId)).toEqual([file.nodeId]); }); it('does not emit implementations when an embedded interface cannot be resolved', () => { @@ -977,7 +1004,7 @@ describe('Go structural interface detection', () => { {} as any, ); - expect(result.get(iface.nodeId)).toEqual([struct.nodeId]); + expect(implIds(result, iface.nodeId)).toEqual([struct.nodeId]); }); it('preserves package qualifiers when checking signatures', () => { @@ -1113,3 +1140,54 @@ describe('Go structural interface detection', () => { expect(result.get(iface.nodeId)).toBeUndefined(); }); }); + +// --------------------------------------------------------------------------- +// #2829 review: `goReceiverKind` must stay stamped even though nothing filters +// on it any more. +// --------------------------------------------------------------------------- +// +// #2813 removed the only functional READER of `goReceiverKind` (the +// pointer-receiver exclusion in `buildDetectionIndexes`). The field is kept +// deliberately — it is the hook a future value/pointer-aware model would read, +// and `interpret.ts` preserves the raw `*T` shape specifically to feed it. But +// a written-and-never-read field rots: before these rows you could delete the +// assignment in `method-owners.ts` and the whole suite stayed green. +// +// These pin the stamp itself, so the promise the comment makes stays true. +describe('Go method-owner receiver-kind stamping (#2829)', () => { + const ownerScope = (receiverRaw: string, methodDef: SymbolDefinition): Scope => { + const s = scope('fn:1' as ScopeId, 'Function', [methodDef]); + (s.typeBindings as Map).set('r', { + rawName: receiverRaw, + source: 'self', + }); + return s; + }; + + const stampFor = (receiverRaw: string): SymbolDefinition => { + const struct = goDef('struct:Repo', 'Struct', 'Repo'); + const method = goDef('struct:Repo.Save', 'Method', 'Save'); + const parsed = { + filePath: 'repo.go', + language: 'go', + scopes: [scope('mod' as ScopeId, 'Module', [struct]), ownerScope(receiverRaw, method)], + imports: [], + localDefs: [struct, method], + referenceSites: [], + } as any; + populateGoOwners(parsed); + return method; + }; + + it('stamps a POINTER receiver as pointer', () => { + const m = stampFor('*Repo') as SymbolDefinition & { goReceiverKind?: string }; + expect(m.goReceiverKind).toBe('pointer'); + expect(m.ownerId).toBe('struct:Repo'); + }); + + it('stamps a VALUE receiver as value', () => { + const m = stampFor('Repo') as SymbolDefinition & { goReceiverKind?: string }; + expect(m.goReceiverKind).toBe('value'); + expect(m.ownerId).toBe('struct:Repo'); + }); +});