diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index 782b51f8d..309c3c95e 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -493,9 +493,10 @@ jobs: # set (fingerprint), stays linear in def count, and that the #2693 # widened gate — which now considers VALUE bindings, a population that # outnumbers callables in real source — stays within its measured - # overhead of the pre-#2693 callable-only cost. Catches a regression in - # the value-binding pre-filter, without which every value binding pays - # the full resolveDefGraphId key chain just to be rejected. + # overhead of the pre-#2693 callable-only cost. The overhead budget also + # guards the DESIGN: value bindings are joined to their callable node by + # position, never by name through resolveDefGraphId, whose label-agnostic + # simpleKey fallback would alias a binding onto any same-named callable. run: node --import tsx bench/callable-value-flow/measure.mjs --check working-directory: gitnexus diff --git a/gitnexus/bench/callable-value-flow/baselines.json b/gitnexus/bench/callable-value-flow/baselines.json index d42c566d9..c69d63c83 100644 --- a/gitnexus/bench/callable-value-flow/baselines.json +++ b/gitnexus/bench/callable-value-flow/baselines.json @@ -2,7 +2,7 @@ "_comment": "Baselines for bench/callable-value-flow/measure.mjs --check (#2693). `fingerprint` is an order-independent sha256 over every (defNodeId -> graphId) pair buildGraphTargetIndex resolves on the synthetic corpus; it is a CORRECTNESS gate, so drift means the callable-value target set moved and must be explained, never re-baselined to make CI green. The two budgets are timing gates and carry deliberate headroom for shared CI runners.", "fingerprint": "70bebf6a26ff6fc9f231a0933678274b44c4883ddab5e719a61a9c77d6223e51", "scaling_budget": 1.6, - "_scaling_note": "(t_large/t_small)/(800/250). ~1.0 is linear; measured 1.16-1.31. The index build is one pass over defs plus map lookups, so a jump toward 3.x means someone made the per-def work depend on corpus size (e.g. a scan inside the loop).", + "_scaling_note": "(t_large/t_small)/(800/250). ~1.0 is linear; measured 1.14-1.16. The index build is one pass over defs plus map lookups, so a jump toward 3.x means someone made the per-def work depend on corpus size (e.g. a scan inside the loop).", "widening_overhead_budget": 1.9, - "_widening_overhead_note": "large_ms / callable_only_ms — how much more the #2693 widened gate costs than the pre-#2693 callable-only population on the SAME corpus. Measured 1.45-1.50 WITH the value-binding pre-filter and 2.50-2.82 WITHOUT it, so this budget sits between the two bands: it cannot be met if the pre-filter is removed or defeated. Value bindings outnumber callables in real source, and without the filter every one of them pays the full resolveDefGraphId key chain only to be rejected." + "_widening_overhead_note": "large_ms / callable_only_ms — how much more the #2693 widened gate costs than the pre-#2693 callable-only population on the SAME corpus. Measured 1.43-1.58 with the positional join (value bindings are matched against a file/line/name index built in the existing graph walk and never run the resolveDefGraphId key chain); a name-only match that fell through to resolveDefGraphId measured 2.50-2.82. The budget sits between the two bands, so it cannot be met by reverting to the slower — and incorrect — name-match design." } diff --git a/gitnexus/bench/callable-value-flow/measure.mjs b/gitnexus/bench/callable-value-flow/measure.mjs index 70d27b5f9..d8d7a2373 100644 Binary files a/gitnexus/bench/callable-value-flow/measure.mjs and b/gitnexus/bench/callable-value-flow/measure.mjs differ diff --git a/gitnexus/src/core/ingestion/languages/dart/captures.ts b/gitnexus/src/core/ingestion/languages/dart/captures.ts index 7700b2d89..0c6fc9e0c 100644 --- a/gitnexus/src/core/ingestion/languages/dart/captures.ts +++ b/gitnexus/src/core/ingestion/languages/dart/captures.ts @@ -58,21 +58,31 @@ const DART_CALLABLE_CAPTURE_OPTIONS = { callNodeTypes: new Set(['selector']), parameterListNodeTypes: new Set(['formal_parameter_list', 'arguments']), parameterNodeTypes: new Set(['formal_parameter']), - // `initialized_identifier` covers TOP-LEVEL bindings, which Dart parses as a - // loose initialized_identifier_list under program rather than wrapping them - // in the local form. Without it a top-level `var f = (x) => x;` emitted no - // flow captures at all, so `f()` never resolved (#2693). - bindingNodeTypes: new Set(['initialized_variable_definition', 'initialized_identifier']), + // `initialized_identifier` covers TOP-LEVEL `var` bindings and the second and + // later declarators of a multi-name local; `static_final_declaration` covers + // top-level `final`/`const`, which parse into a different list node entirely. + // Dart wraps only the FIRST local declarator in `initialized_variable_ + // definition`, so without the other two a top-level `var f = (x) => x;`, a + // `final f = …`, and the `g` of `var f = …, g = …;` all emitted no flow + // captures at all and never resolved (#2693). + bindingNodeTypes: new Set([ + 'initialized_variable_definition', + 'initialized_identifier', + 'static_final_declaration', + ]), assignmentNodeTypes: new Set(['assignment_expression']), identifierNodeTypes: new Set(['identifier', 'type_identifier']), - // `initialized_identifier` is FIELDLESS, so the shared field-based fallback - // (`left`/`name`/`value`/…) decomposes nothing and a top-level binding - // produced no flow facts at all — the same shape as Kotlin's fieldless - // `assignment` node. Positional: first named child is the bound name, last is - // the initializer. `initialized_variable_definition` carries real `name:` / - // `value:` fields, so it is left to the shared path by returning undefined. + // `initialized_identifier` and `static_final_declaration` are FIELDLESS, so + // the shared field-based fallback (`left`/`name`/`value`/…) decomposes + // nothing and those bindings produced no flow facts at all — the same shape + // as Kotlin's fieldless `assignment` node. Positional: first named child is + // the bound name, last is the initializer. + // `initialized_variable_definition` carries real `name:` / `value:` fields, + // so it is left to the shared path by returning undefined. extractAssignment: (node: SyntaxNode) => { - if (node.type !== 'initialized_identifier') return undefined; + if (node.type !== 'initialized_identifier' && node.type !== 'static_final_declaration') { + return undefined; + } const named = node.namedChildren.filter((child): child is SyntaxNode => child !== null); if (named.length < 2) return undefined; return { destination: named[0]!, source: named[named.length - 1]! }; diff --git a/gitnexus/src/core/ingestion/languages/dart/query.ts b/gitnexus/src/core/ingestion/languages/dart/query.ts index 49d88b90f..06cb3496f 100644 --- a/gitnexus/src/core/ingestion/languages/dart/query.ts +++ b/gitnexus/src/core/ingestion/languages/dart/query.ts @@ -141,9 +141,21 @@ const DART_SCOPE_QUERY = ` (initialized_identifier (identifier) @declaration.name (function_expression))) @declaration.variable) +(program + (static_final_declaration_list + (static_final_declaration + (identifier) @declaration.name + (function_expression))) @declaration.variable) (initialized_variable_definition name: (identifier) @declaration.name value: (function_expression)) @declaration.variable +; Second and later declarators of \`var f = .., g = ..;\` are nested +; initialized_identifier children of the same initialized_variable_definition, +; which the field-based rule above only reaches for the first name. +(initialized_variable_definition + (initialized_identifier + (identifier) @declaration.name + (function_expression)) @declaration.variable) ; ── Imports / re-exports ───────────────────────────────────────────────────── (import_or_export diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/callable-value-flow.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/callable-value-flow.ts index a7c52e454..7d6d35792 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/callable-value-flow.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/callable-value-flow.ts @@ -754,44 +754,43 @@ export function buildGraphTargetIndex( graph: KnowledgeGraph, ): ReadonlyMap { const out = new Map(); - const { byAnchor, callableNameKeys } = buildGraphCallableIndexes(graph); + const { byAnchor, callableByPosition } = buildGraphCallableIndexes(graph); for (const def of scopes.defs.byId.values()) { const callableDef = isCallable(def) || providerTarget?.(def) === true; - // #2693: a closure bound to a name (`val f = { }`) is a callable the def - // type cannot see. The scope-resolution layer declares such a binding with - // its VALUE label (Kotlin/Swift `Property`, Dart `Variable`) while #2687 - // makes the graph emit a single `Function` node for it. Only the graph - // knows, so value bindings are resolved below on the label of the node they - // actually reach. if (!callableDef) { - if (!VALUE_BINDING_DEF_TYPES.has(def.type)) continue; - // Exact pre-filter, not a heuristic. Every qualified key - // `resolveDefGraphId` tries embeds `def.type`, so for a VALUE def those - // can only ever hit a value-labelled node; its one route to a callable is - // the label-agnostic `simpleKey(filePath, simpleName)` fallback, which by - // construction requires a callable node with the SAME file and simple - // name. So a value binding with no such node cannot possibly resolve to a - // callable, and skipping it here is equivalent to running the full - // resolve and rejecting the result — at one Set lookup instead of the - // whole key chain. Value bindings outnumber callables in real source, so - // this is the difference between paying resolve cost per binding and - // paying it per binding that can actually match. - const simple = simpleQualifiedName(def); - if (simple === undefined || !callableNameKeys.has(`${def.filePath}\0${simple}`)) continue; + // #2693: a closure bound to a name (`val f = { }`) is a callable the def + // type cannot see — the scope layer declares it with its VALUE label + // (Kotlin/Swift `Property`, Dart `Variable`) while #2687 makes the graph + // emit a single callable node for it. Only the graph knows. + // + // The join MUST be positional. Resolving such a def through + // `resolveDefGraphId` is unsafe: every qualified key it builds embeds + // `def.type`, so for a value def they can only ever hit a value-labelled + // node — and when the def's own node is absent (Rust `let`) or carries a + // DIFFERENT label than the def's type (TypeScript declares `const` as + // `Variable` but emits a `Const` node), the chain falls through to the + // label-agnostic, first-write-wins `simpleKey(filePath, simpleName)`. + // That aliases the binding onto ANY same-named callable in the file — + // `const save = cb` next to an unrelated `Svc.save` mints a CALLS edge to + // the method, and the result depends on declaration order. + // + // A closure binding IS its callable node: same file, same line, same + // name. An aliasing local is not. So look the node up by position and + // admit only an exact hit. + const positionalKey = valueBindingPositionKey(def); + const positionalId = + positionalKey === undefined ? undefined : callableByPosition.get(positionalKey); + // `''` marks an ambiguous position (two callables claiming one + // file/line/name) — undecidable, so admit neither. + if (positionalId === undefined || positionalId === '') continue; + out.set(def.nodeId, { id: positionalId, def }); + continue; } - // Only callable defs can match the anchor index — it is keyed by callable - // LABEL, and `definitionAnchorKey` builds its key from `def.type`. Running - // it for a value def costs a regex per def and can never hit. - const anchorKey = callableDef ? definitionAnchorKey(def) : undefined; + const anchorKey = definitionAnchorKey(def); const anchored = anchorKey === undefined ? undefined : byAnchor.get(anchorKey); const id = anchored?.length === 1 ? anchored[0] : resolveDefGraphId(def.filePath, def, nodeLookup); if (id === undefined) continue; - // A genuine constant keeps its own `Const`/`Property`/`Variable` node, so - // `resolveDefGraphId`'s qualified key hits before its label-agnostic - // `simpleKey` fallback can reach a same-named callable — only a binding - // whose own value node was replaced by a callable one gets through here. - if (!callableDef && !isCallableGraphNode(graph, id)) continue; // Overloads can intentionally share one graph node ID. Index by the // definition identity so contextual signature narrowing still sees the // complete overload set before a selected target collapses to graph ID. @@ -801,22 +800,39 @@ export function buildGraphTargetIndex( } /** - * Value-binding labels whose initializer can be a callable. Kept separate from - * `isCallable` because these are admitted on graph-node evidence, never on the - * def type alone. + * `file\0line\0name` for a value binding, matching the callable-node key built + * in `buildGraphCallableIndexes`. Definition lines come from the def id and are + * 1-based; graph `startLine` is 0-based, which is the `+ 1` there. + */ +function valueBindingPositionKey(def: SymbolDefinition): string | undefined { + if (!VALUE_BINDING_DEF_TYPES.has(def.type)) return undefined; + const line = def.nodeId.match(/#(\d+):(\d+):/)?.[1]; + const name = simpleQualifiedName(def); + if (line === undefined || name === undefined) return undefined; + return `${def.filePath}\0${line}\0${name}`; +} + +/** + * Value-binding labels whose initializer can be a callable. Admitted ONLY on + * positional graph-node evidence (see `buildGraphTargetIndex`), never on the def + * type alone. + * + * Deliberately NOT `isOwnableValueLabel` (scope/walkers.ts), which lists the + * same labels for the value-receiver bridge: that predicate is contracted to + * `reconcileOwnership`, and coupling the two would let a label added for + * ownership silently widen call-target admission. Two lists, two reasons — + * changing either means checking the other. + * + * `Static` is excluded: `normalizeNodeLabel` (scope-extractor.ts) has no + * `static` case, so no scope-resolution def can carry that type. Including it + * added an entry no fixture could ever exercise. */ const VALUE_BINDING_DEF_TYPES: ReadonlySet = new Set([ 'Const', 'Property', - 'Static', 'Variable', ]); -function isCallableGraphNode(graph: KnowledgeGraph, id: string): boolean { - const label = graph.getNode(id)?.label; - return label === 'Function' || label === 'Method' || label === 'Constructor'; -} - interface CanonicalCallableTargets { /** Definition identity remains the key; declaration keys may point at the definition target. */ readonly targets: ReadonlyMap; @@ -935,32 +951,46 @@ interface GraphCallableIndexes { /** Callable graph nodes by `file\0label\0line\0name` — the definition anchor. */ readonly byAnchor: ReadonlyMap; /** - * `file\0name` for every callable graph node. The value-binding pre-filter in - * `buildGraphTargetIndex` needs only existence, and deriving it here keeps the - * graph to ONE walk rather than a second pass for the same nodes. + * Callable graph nodes by `file\0line\0name` — the same anchor WITHOUT the + * label, because a value binding's def type never matches its callable node's + * label (that is the whole point of #2693). Value = the node id, or `''` when + * two callables claim one position and the join is undecidable. + * + * Derived in the same walk as `byAnchor`: a second pass over the graph for + * the same nodes would double the cost of the largest loop in this pass. */ - readonly callableNameKeys: ReadonlySet; + readonly callableByPosition: ReadonlyMap; } function buildGraphCallableIndexes(graph: KnowledgeGraph): GraphCallableIndexes { const byAnchor = new Map(); - const callableNameKeys = new Set(); + const callableByPosition = new Map(); for (const node of graph.iterNodes()) { if (node.label !== 'Function' && node.label !== 'Method' && node.label !== 'Constructor') { continue; } const filePath = node.properties.filePath; const name = node.properties.name; - if (typeof filePath !== 'string' || typeof name !== 'string') continue; - callableNameKeys.add(`${filePath}\0${name}`); const zeroBasedLine = node.properties.startLine; - if (typeof zeroBasedLine !== 'number') continue; - const key = `${filePath}\0${node.label}\0${zeroBasedLine + 1}\0${name}`; + if ( + typeof filePath !== 'string' || + typeof name !== 'string' || + typeof zeroBasedLine !== 'number' + ) { + continue; + } + const oneBasedLine = zeroBasedLine + 1; + const positionKey = `${filePath}\0${oneBasedLine}\0${name}`; + const existing = callableByPosition.get(positionKey); + // First wins would be order-dependent; mark the collision instead so an + // ambiguous position admits nothing rather than something arbitrary. + callableByPosition.set(positionKey, existing === undefined ? node.id : ''); + const key = `${filePath}\0${node.label}\0${oneBasedLine}\0${name}`; const bucket = byAnchor.get(key); if (bucket === undefined) byAnchor.set(key, [node.id]); else bucket.push(node.id); } - return { byAnchor, callableNameKeys }; + return { byAnchor, callableByPosition }; } function definitionAnchorKey(def: SymbolDefinition): string | undefined { diff --git a/gitnexus/src/core/ingestion/tree-sitter-queries.ts b/gitnexus/src/core/ingestion/tree-sitter-queries.ts index 4f864595a..197fcb621 100644 --- a/gitnexus/src/core/ingestion/tree-sitter-queries.ts +++ b/gitnexus/src/core/ingestion/tree-sitter-queries.ts @@ -1730,6 +1730,17 @@ export const DART_QUERIES = ` (identifier) @name (function_expression))) @definition.function) +; ── Top-level final/const closure bindings (#2693) ────────────────────────── +; \`final handler = (x) => x;\` parses as a static_final_declaration_list, not an +; initialized_identifier_list, so the rules above never reach it — \`final\` is +; the idiomatic top-level binding keyword and was the one closure form getting +; neither the callable label nor resolution. +(program + (static_final_declaration_list + (static_final_declaration + (identifier) @name + (function_expression))) @definition.function) + ; ── Function-local closure bindings (#2693) ───────────────────────────────── ; \`void m() { var f = (x) => x; }\` — locals parse as initialized_variable_ ; definition, which the top-level rules above never reach, so a local closure @@ -1738,6 +1749,17 @@ export const DART_QUERIES = ` (initialized_variable_definition name: (identifier) @name value: (function_expression)) @definition.function + +; Second and later declarators of a multi-name local (\`var f = .., g = ..;\`) +; are initialized_identifier children NESTED INSIDE the same +; initialized_variable_definition, which the \`name:\`/\`value:\` field rule above +; only reaches for the FIRST name — so \`g\` silently had no node. Anchored on the +; inner node so each name gets its own range; the top-level form lives under +; initialized_identifier_list instead, so these never double-match. +(initialized_variable_definition + (initialized_identifier + (identifier) @name + (function_expression)) @definition.function) (program (static_final_declaration_list (static_final_declaration diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index 2b98b8618..c87f2d840 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -55,6 +55,8 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j // the main thread (the #1983 OOM). Because the two stores share this version, // any future change to the `ParsedFile` serialization shape MUST bump // SCHEMA_BUMP so both invalidate in lockstep. +// v23: Dart closure bindings emit Function nodes for function-local closures +// and flow captures for top-level ones (#2693). // v22: `const X = ` emits one `Function` node // instead of a `Function` plus an edgeless `Const` twin (#2687). Cached worker // results are replayed verbatim — including across `--force` — so without this @@ -63,8 +65,6 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j // method injection sites plus bean-name and @Primary provider metadata. // v20: Java/Kotlin capture side-channels persist package and class-annotation // facts for shared Spring Bean resolution. -// v23: Dart closure bindings emit Function nodes for function-local closures -// and flow captures for top-level ones (#2693). // v21: Java local class/enum/record/interface captures use javac-compatible, // source-type-relative JLS 13.1 identities and declaration-to-block scopes // (#2562). diff --git a/gitnexus/test/integration/closure-binding-labels.test.ts b/gitnexus/test/integration/closure-binding-labels.test.ts index eaf23c0aa..ad820702f 100644 --- a/gitnexus/test/integration/closure-binding-labels.test.ts +++ b/gitnexus/test/integration/closure-binding-labels.test.ts @@ -204,7 +204,7 @@ describeIfWorkerBuilt('calls to a closure binding resolve to its Function node', 'val handler = { x: Int -> x }\n\nfun caller(): Int {\n return handler(1)\n}\n', ); - expect(targets).toContain('Function:App.kt:handler'); + expect(targets).toEqual(['Function:App.kt:handler']); }); it('Swift: handler(1) resolves', async () => { @@ -213,7 +213,7 @@ describeIfWorkerBuilt('calls to a closure binding resolve to its Function node', 'let handler = { (x: Int) -> Int in return x }\n\nfunc caller() -> Int {\n return handler(1)\n}\n', ); - expect(targets).toContain('Function:App.swift:handler'); + expect(targets).toEqual(['Function:App.swift:handler']); }); it('Dart: a top-level closure binding resolves', async () => { // The top-level form parses as `initialized_identifier`; the function-local @@ -224,7 +224,7 @@ describeIfWorkerBuilt('calls to a closure binding resolve to its Function node', 'var handler = (int x) => x;\n\nint caller() {\n return handler(1);\n}\n', ); - expect(targets).toContain('Function:app.dart:handler'); + expect(targets).toEqual(['Function:app.dart:handler']); }); it('Dart: a function-local closure binding resolves', async () => { @@ -233,7 +233,44 @@ describeIfWorkerBuilt('calls to a closure binding resolve to its Function node', 'int caller() {\n var handler = (int x) => x;\n return handler(1);\n}\n', ); - expect(targets).toContain('Function:local.dart:handler'); + expect(targets).toEqual(['Function:local.dart:handler']); + }); + + it('Dart: a top-level `final` closure binding resolves', async () => { + // `final` is the idiomatic top-level binding keyword and parses as a + // static_final_declaration_list, not an initialized_identifier_list, so it + // reaches neither the #2687 label rule nor the #2693 flow captures unless + // both are taught about it. + const targets = await callTargetsFor( + 'final.dart', + 'final handler = (int x) => x;\n\nint caller() {\n return handler(1);\n}\n', + ); + + expect(targets).toEqual(['Function:final.dart:handler']); + }); + + it('Dart: every declarator of a multi-name local closure resolves', async () => { + // Dart wraps only the FIRST declarator in initialized_variable_definition; + // `g` is a nested initialized_identifier, so a rule keyed on the `name:` + // field alone silently drops it. + const targets = await callTargetsFor( + 'multi.dart', + 'int caller() {\n var f = (int x) => x, g = (int y) => y;\n return f(1) + g(2);\n}\n', + ); + + expect(targets).toEqual(['Function:multi.dart:f', 'Function:multi.dart:g']); + }); + + it('Kotlin: a class-body closure property resolves to its Method node', async () => { + // The class-body form is the common real-world shape and is the ONLY case + // that exercises the `Method` arm of the callable-label check — narrowing + // that check to `Function` would delete this silently. + const targets = await callTargetsFor( + 'Box.kt', + 'class Box {\n val handler = { x: Int -> x }\n fun caller(): Int {\n return handler(1)\n }\n}\n', + ); + + expect(targets).toEqual(['Method:Box.kt:Box.handler']); }); }); @@ -299,16 +336,84 @@ describeIfWorkerBuilt('the declaration route does not double-emit (#2693)', () = }); }); -describeIfWorkerBuilt('a non-callable value binding stays edge-free', () => { - // The suppression must key on an actual callable value. Widening - // `buildGraphTargetIndex` to admit value bindings whose graph node is a - // `Function` is safe only because a genuine constant keeps its own - // `Const`/`Property`/`Variable` node, so `resolveDefGraphId`'s qualified key - // hits before the label-agnostic `simpleKey` fallback can reach a callable. +describeIfWorkerBuilt('a value binding is never aliased onto a same-named callable', () => { + // These are the regression tests for the defect the first cut of #2693 + // shipped. Admitting a value binding on a same-file NAME match let + // `resolveDefGraphId` fall through to its label-agnostic, first-write-wins + // `simpleKey(filePath, simpleName)` and bind the name to ANY same-named + // callable in the file — a fabricated caller, chosen by declaration order. + // + // The join is positional now: a closure binding IS its callable node (same + // file, same line, same name); an aliasing local is not. Every case below + // pairs a value binding with a same-named callable, which is precisely the + // collision the previous fixtures never created — they used DIFFERENT names + // (`maxSize` vs `size`), so the pre-filter rejected them before the guard + // they were named after could run, and deleting that guard changed nothing. - it('Kotlin: a constant sharing its name with a function mints no CALLS to the constant', async () => { + it('TypeScript: a local aliasing a parameter does not call the same-named top-level function', async () => { const targets = await callTargetsFor( - 'Shadow.kt', + 'alias.ts', + 'export function handler(): number {\n return 1;\n}\n\n' + + 'export function caller(cb: () => number): number {\n const handler = cb;\n return handler();\n}\n', + ); + + expect(targets).toEqual([]); + }); + + it('TypeScript: a local closure does not call a same-named class method', async () => { + // `Svc` is never instantiated. The local arrow has its own Function node, + // which is the only legitimate target. + const targets = await callTargetsFor( + 'svc.ts', + 'export class Svc {\n save(x: number): number {\n return x;\n }\n}\n\n' + + 'export function run(): number {\n const save = (x: number): number => x * 2;\n return save(1);\n}\n', + ); + + expect(targets).toEqual(['Function:svc.ts:save']); + }); + + it('TypeScript: a shadowing local does not also call the shadowed function', async () => { + // `caller` invokes `other` through the shadowing binding; the outer + // `handler` is unreachable from it. + const targets = await callTargetsFor( + 'shadow.ts', + 'export function handler(x: number): number {\n return x;\n}\n' + + 'export function other(x: number): number {\n return x * 2;\n}\n\n' + + 'export function caller(): number {\n const handler = other;\n return handler(1);\n}\n', + ); + + expect(targets).toEqual(['Function:shadow.ts:other']); + }); + + it('Rust: a let binding does not call the same-named function', async () => { + // Rust `let` bindings get no graph node at all, so the simple-name + // fallback was the ONLY route — this is the shape with no value node to + // claim the qualified key first. + const targets = await callTargetsFor( + 'main.rs', + 'fn handler() -> i32 {\n 1\n}\n\n' + + 'fn caller(cb: fn() -> i32) -> i32 {\n let handler = cb;\n handler()\n}\n', + ); + + expect(targets).toEqual([]); + }); + + it('Dart: a local closure does not call a same-named class method', async () => { + // Before the positional join this emitted the WRONG edge and lost the + // right one: the only target was `Svc.save`, while the closure's own node + // got nothing. + const targets = await callTargetsFor( + 'svc.dart', + 'class Svc {\n int save(int x) => x;\n}\n\n' + + 'int run() {\n var save = (int x) => x * 2;\n return save(1);\n}\n', + ); + + expect(targets).toEqual(['Function:svc.dart:save']); + }); + + it('Kotlin: a genuine constant mints no CALLS', async () => { + const targets = await callTargetsFor( + 'Consts.kt', 'val maxSize = 10\n\nfun size(): Int {\n return maxSize\n}\n', ); diff --git a/gitnexus/test/integration/resolvers/callable-value-flow.test.ts b/gitnexus/test/integration/resolvers/callable-value-flow.test.ts index a16a63d51..6605c8e4d 100644 --- a/gitnexus/test/integration/resolvers/callable-value-flow.test.ts +++ b/gitnexus/test/integration/resolvers/callable-value-flow.test.ts @@ -1399,6 +1399,66 @@ invoke(assigned); } }, 120_000); + it('replays closure-binding resolution from the durable warm parse cache (#2693)', async () => { + // The #2693 captures are provider-synthesized (Dart's fieldless + // `initialized_identifier` seed) and the function-local closure `Function` + // node is minted at parse time — both are replayed VERBATIM from the parse + // cache, so a serialization change would surface only on the SECOND + // analyze. Every other test in this PR runs cold and would stay green. + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-closure-warm-repo-')); + const storage = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-closure-warm-store-')); + try { + fs.writeFileSync( + path.join(root, 'app.dart'), + 'var handler = (int x) => x;\n\nint caller() {\n return handler(1);\n}\n', + 'utf8', + ); + fs.writeFileSync( + path.join(root, 'App.kt'), + 'val handler = { x: Int -> x }\n\nfun caller(): Int {\n return handler(1)\n}\n', + 'utf8', + ); + const coldCache: ParseCache = { + version: PARSE_CACHE_VERSION, + entries: new Map(), + usedKeys: new Set(), + storagePath: storage, + onDiskKeys: new Set(), + }; + const cold = await runPipelineFromRepo(root, () => {}, { + skipGraphPhases: true, + parseCache: coldCache, + }); + const savedKeys = await saveParseCache(storage, coldCache); + await pruneAndSaveDurableParsedFileStore( + getDurableParsedFileDir(storage), + PARSE_CACHE_VERSION, + new Set(savedKeys), + ); + const warmCache = await loadParseCache(storage); + const warm = await runPipelineFromRepo(root, () => {}, { + skipGraphPhases: true, + parseCache: warmCache, + }); + const project = (result: Awaited>) => + getRelationships(result, 'CALLS') + .map( + (edge) => + `${edge.sourceFilePath}:${edge.source}->${edge.targetFilePath}:${edge.target}`, + ) + .sort(); + + expect(project(warm)).toEqual(project(cold)); + expect(project(warm)).toEqual([ + 'App.kt:caller->App.kt:handler', + 'app.dart:caller->app.dart:handler', + ]); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + fs.rmSync(storage, { recursive: true, force: true }); + } + }, 120_000); + it('keeps normal/PDG targets identical and stamps calleeIds at the indirect invocation', async () => { const source = ` function target(): void {} diff --git a/gitnexus/test/unit/call-summary-schema-version.test.ts b/gitnexus/test/unit/call-summary-schema-version.test.ts index 24df56b66..b8ef1149c 100644 --- a/gitnexus/test/unit/call-summary-schema-version.test.ts +++ b/gitnexus/test/unit/call-summary-schema-version.test.ts @@ -133,7 +133,12 @@ describe('CALL_SUMMARY incremental reuse gate (U-C5)', () => { // every unchanged TS/JS file, and the incremental write set never touches // those files → must NOT reuse. expect(passesReuseGate(14)).toBe(false); + // A pre-v16 (v15) index predates #2693: calls through a closure-valued + // binding do not resolve in Kotlin/Swift/Dart, and the incremental write + // set never revisits unchanged files, so those symbols would keep reporting + // a zero blast radius → must NOT reuse. + expect(passesReuseGate(15)).toBe(false); // A current-version stamp passes the gate (incremental top-up eligible). - expect(passesReuseGate(15)).toBe(true); + expect(passesReuseGate(16)).toBe(true); }); });