diff --git a/gitnexus/src/core/ingestion/languages/php/query.ts b/gitnexus/src/core/ingestion/languages/php/query.ts index f8894ebaf..e24919e02 100644 --- a/gitnexus/src/core/ingestion/languages/php/query.ts +++ b/gitnexus/src/core/ingestion/languages/php/query.ts @@ -107,6 +107,20 @@ const PHP_SCOPE_QUERY = ` (property_element name: (variable_name) @declaration.name)) @declaration.variable +;; ── Declarations — closure bindings (#2693) ─────────────────────────────── +;; A dollar-name bound to a closure (fn(x) => x, or function(x){...}) IS a +;; callable. PHP emitted the callable-flow seed and invoke for it already, but +;; nothing DECLARED the name, so the flow pass had no SymbolDefinition to attach +;; the seed to and the call stayed unresolved. +;; Restricted to a closure value: declaring every PHP assignment would mint defs +;; repo-wide for no resolution benefit. +(assignment_expression + left: (variable_name (name) @declaration.name) + right: (arrow_function)) @declaration.variable +(assignment_expression + left: (variable_name (name) @declaration.name) + right: (anonymous_function)) @declaration.variable + ;; ── Imports — namespace_use_declaration ─────────────────────────────────── ;; ;; Captures ALL forms: plain, alias, function/const qualifiers, and grouped. diff --git a/gitnexus/src/core/ingestion/tree-sitter-queries.ts b/gitnexus/src/core/ingestion/tree-sitter-queries.ts index 197fcb621..2375ab37f 100644 --- a/gitnexus/src/core/ingestion/tree-sitter-queries.ts +++ b/gitnexus/src/core/ingestion/tree-sitter-queries.ts @@ -71,6 +71,33 @@ export const TYPESCRIPT_QUERIES = ` name: (identifier) @name value: (function_expression)))) @definition.function +; \`var\` closure bindings (#2693). The lexical rules above cover const/let; +; \`var\` is a different grammar node, so \`var f = (x) => x\` kept a Variable +; label while const/let got Function — and the CALLS edge that resolved through +; the declaration route therefore pointed at a NON-callable node. Same construct, +; same binding semantics for this purpose, so same label. +(variable_declaration + (variable_declarator + name: (identifier) @name + value: (arrow_function))) @definition.function + +(variable_declaration + (variable_declarator + name: (identifier) @name + value: (function_expression))) @definition.function + +(export_statement + declaration: (variable_declaration + (variable_declarator + name: (identifier) @name + value: (arrow_function)))) @definition.function + +(export_statement + declaration: (variable_declaration + (variable_declarator + name: (identifier) @name + value: (function_expression)))) @definition.function + ; Object-property arrows / function expressions: \`{ addItem: () => ... }\`. ; The pair's key field carries the meaningful name. Without these patterns, ; calls inside the arrow are attributed to the file (issue #1166), and the @@ -409,6 +436,33 @@ export const JAVASCRIPT_QUERIES = ` name: (identifier) @name value: (function_expression)))) @definition.function +; \`var\` closure bindings (#2693). The lexical rules above cover const/let; +; \`var\` is a different grammar node, so \`var f = (x) => x\` kept a Variable +; label while const/let got Function — and the CALLS edge that resolved through +; the declaration route therefore pointed at a NON-callable node. Same construct, +; same binding semantics for this purpose, so same label. +(variable_declaration + (variable_declarator + name: (identifier) @name + value: (arrow_function))) @definition.function + +(variable_declaration + (variable_declarator + name: (identifier) @name + value: (function_expression))) @definition.function + +(export_statement + declaration: (variable_declaration + (variable_declarator + name: (identifier) @name + value: (arrow_function)))) @definition.function + +(export_statement + declaration: (variable_declaration + (variable_declarator + name: (identifier) @name + value: (function_expression)))) @definition.function + ; Object-property arrows / function expressions: \`{ addItem: () => ... }\`. ; See TYPESCRIPT_QUERIES for rationale (issue #1166). (pair @@ -800,6 +854,27 @@ export const JAVA_QUERIES = ` object: (_) @assignment.receiver field: (identifier) @assignment.property) right: (_)) @assignment + +; ── Closure bindings (#2693) ──────────────────────────────────────────────── +; A name bound to a closure literal IS a callable, so it emits Function rather +; than a value label — matching TS/JS and the languages #2687 already covered. +; The callable node is what callable-value-flow joins the binding to (by file, +; line and name), which is what makes handler.apply(1) resolve. Overlap with the value +; rules above is collapsed by the parse-worker dedup, which ranks callable +; highest (#2687). +; Anchored on field_declaration / local_variable_declaration — the SAME nodes +; the value rules above use — so the parse-worker dedup (keyed by definition +; node + name) actually collapses the pair. Anchoring on the inner +; variable_declarator instead produced a Function AND a Property twin, the exact +; double-indexing #2687 removed. +(field_declaration + declarator: (variable_declarator + name: (identifier) @name + value: (lambda_expression))) @definition.function +(local_variable_declaration + declarator: (variable_declarator + name: (identifier) @name + value: (lambda_expression))) @definition.function `; // C queries - works with tree-sitter-c @@ -1152,6 +1227,17 @@ export const CSHARP_QUERIES = ` expression: (_) @assignment.receiver name: (identifier) @assignment.property) right: (_)) @assignment + +; ── Closure bindings (#2693) ──────────────────────────────────────────────── +; A name bound to a closure literal IS a callable, so it emits Function rather +; than a value label — matching TS/JS and the languages #2687 already covered. +; The callable node is what callable-value-flow joins the binding to (by file, +; line and name), which is what makes handler(1) resolve. Overlap with the value +; rules above is collapsed by the parse-worker dedup, which ranks callable +; highest (#2687). +(variable_declarator + (identifier) @name + (lambda_expression)) @definition.function `; // Rust queries - works with tree-sitter-rust @@ -1306,6 +1392,20 @@ export const PHP_QUERIES = ` scope: (_) @assignment.receiver name: (variable_name (name) @assignment.property)) right: (_)) @assignment + +; ── Closure bindings (#2693) ──────────────────────────────────────────────── +; A name bound to a closure literal IS a callable, so it emits Function rather +; than a value label — matching TS/JS and the languages #2687 already covered. +; The callable node is what callable-value-flow joins the binding to (by file, +; line and name), which is what makes $handler(1) resolve. Overlap with the value +; rules above is collapsed by the parse-worker dedup, which ranks callable +; highest (#2687). +(assignment_expression + left: (variable_name (name) @name) + right: (arrow_function)) @definition.function +(assignment_expression + left: (variable_name (name) @name) + right: (anonymous_function)) @definition.function `; // Ruby queries - works with tree-sitter-ruby @@ -1374,6 +1474,17 @@ export const RUBY_QUERIES = ` receiver: (_) @assignment.receiver method: (identifier) @assignment.property) right: (_)) @assignment + +; ── Closure bindings (#2693) ──────────────────────────────────────────────── +; A name bound to a closure literal IS a callable, so it emits Function rather +; than a value label — matching TS/JS and the languages #2687 already covered. +; The callable node is what callable-value-flow joins the binding to (by file, +; line and name), which is what makes handler.call(1) resolve. Overlap with the value +; rules above is collapsed by the parse-worker dedup, which ranks callable +; highest (#2687). +(assignment + left: (identifier) @name + right: (lambda)) @definition.function `; // Kotlin queries - works with tree-sitter-kotlin (fwcd/tree-sitter-kotlin) diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index 9f294f34e..241d3f71d 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -55,8 +55,10 @@ 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). +// v23: closure bindings emit callable nodes in Dart, Ruby, Java, C# and PHP +// (plus JS/TS `var`), and Dart/PHP gain the scope declarations and flow +// captures their forms were missing (#2693). Cached worker results are replayed +// verbatim, so without this bump a warm cache keeps serving the old labels. // 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 diff --git a/gitnexus/src/storage/repo-manager.ts b/gitnexus/src/storage/repo-manager.ts index a0e362a2b..e352b3cea 100644 --- a/gitnexus/src/storage/repo-manager.ts +++ b/gitnexus/src/storage/repo-manager.ts @@ -473,10 +473,12 @@ export interface RepoMeta { * keep its twin and `impact`/`context` would stay ambiguous on those names; * force a full re-analyze instead. * v16: calls through a closure-valued binding (`val f = { }; f()`) now resolve - * in Kotlin, Swift and Dart (#2693). These are NEW `CALLS` edges, and Dart also - * gains `Function` nodes for function-local closures. The incremental write set - * only covers changed files, so unchanged files would keep reporting a zero - * blast radius for those symbols; force a full re-analyze instead. + * in Kotlin, Swift, Dart, Ruby, Java, C# and PHP (#2693). These are NEW `CALLS` + * edges, and those languages also gain callable graph nodes for closure + * bindings that previously carried a value label or no node at all (including + * JS/TS `var f = () => {}`). The incremental write set only covers changed + * files, so unchanged files would keep reporting a zero blast radius for those + * symbols; force a full re-analyze instead. */ export const INCREMENTAL_SCHEMA_VERSION = 16; diff --git a/gitnexus/test/integration/closure-binding-labels.test.ts b/gitnexus/test/integration/closure-binding-labels.test.ts index ad820702f..4dca786d9 100644 --- a/gitnexus/test/integration/closure-binding-labels.test.ts +++ b/gitnexus/test/integration/closure-binding-labels.test.ts @@ -24,7 +24,7 @@ * node this file asserts IS the evidence that admits the binding as a callable * target, so a regression in the labels above now also breaks call resolution. */ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; @@ -32,6 +32,12 @@ import { runPipelineFromRepo } from '../../src/core/ingestion/pipeline.js'; import { DIST_WORKER_URL, distWorkerExists } from '../helpers/worker-parse.js'; import { parseFilesWithWorkers } from '../helpers/worker-parse.js'; +// Every test here spins its own worker pool (see the note above), and the file +// now covers a dozen languages across four describes. Under that contention a +// single case can exceed the 30s default even though it takes ~7s alone, so the +// budget is raised file-wide rather than per-test. +vi.setConfig({ testTimeout: 90_000 }); + const labelsFor = async (path: string, content: string, name: string): Promise => { const { graph } = await parseFilesWithWorkers([{ path, content }]); return graph.nodes @@ -336,6 +342,82 @@ describeIfWorkerBuilt('the declaration route does not double-emit (#2693)', () = }); }); +describeIfWorkerBuilt('closure bindings resolve in the remaining languages (#2693)', () => { + // Ruby, Java, C# and PHP already emitted correct callable-flow seeds and + // invokes; what they lacked was the #2687 piece — a CALLABLE graph node at + // the binding, which is what `buildGraphTargetIndex` joins to by position. + // Ruby and Java invoke through the callable-object protocol (`.call` / + // `.apply`); C# and PHP call the binding directly. + + it('Ruby: handler.call(1) resolves', async () => { + const targets = await callTargetsFor( + 'a.rb', + 'handler = ->(x) { x }\n\ndef caller\n handler.call(1)\nend\n', + ); + + expect(targets).toEqual(['Function:a.rb:handler']); + }); + + it('Java: handler.apply(1) resolves to ONE node, not a Function/Property twin', async () => { + // The rule is anchored on field_declaration — the same node the value rule + // uses — so the parse-worker dedup collapses the pair. Anchoring on the + // inner variable_declarator produced both a Function and a Property node. + const targets = await callTargetsFor( + 'A.java', + 'import java.util.function.Function;\n' + + 'class A {\n' + + ' static Function handler = x -> x;\n' + + ' int caller() { return handler.apply(1); }\n' + + '}\n', + ); + + expect(targets).toEqual(['Function:A.java:A.handler']); + }); + + it('C#: handler(1) resolves', async () => { + const targets = await callTargetsFor( + 'A.cs', + 'using System;\nclass A {\n' + + ' static Func handler = x => x;\n' + + ' int Caller() { return handler(1); }\n}\n', + ); + + expect(targets).toEqual(['Function:A.cs:A.handler']); + }); + + it('PHP: $handler(1) resolves', async () => { + const targets = await callTargetsFor( + 'a.php', + ' $x;\n' + + 'function caller() {\n global $handler;\n return $handler(1);\n}\n', + ); + + expect(targets).toEqual(['Function:a.php:handler']); + }); + + it('PHP: an anonymous function binding resolves too', async () => { + const targets = await callTargetsFor( + 'b.php', + ' { + // `var` is a different grammar node than const/let, so it kept a Variable + // label — and the CALLS edge that resolved through the declaration route + // pointed at a NON-callable node. + const targets = await callTargetsFor( + 'c.js', + 'var handler = (x) => x;\n\nexport function caller() { return handler(1); }\n', + ); + + expect(targets).toEqual(['Function:c.js:handler']); + }); +}); + 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 diff --git a/gitnexus/test/integration/const-function-twin.test.ts b/gitnexus/test/integration/const-function-twin.test.ts index 506b07ef8..ea2f0bfe8 100644 --- a/gitnexus/test/integration/const-function-twin.test.ts +++ b/gitnexus/test/integration/const-function-twin.test.ts @@ -15,8 +15,10 @@ * the twin was emitted first and never suppressed. * * The over-suppression guards below matter as much as the twin assertions: a - * genuine non-callable `const`, an object-literal service (#1718), a `var` - * binding, and the non-function initializers must all keep their value nodes. + * genuine non-callable `const`, an object-literal service (#1718), a plain + * `var` value, and the non-function initializers must all keep their value + * nodes. (A `var` bound to a CLOSURE is a twin case, not a guard case, since + * #2693 — see the pair of `var` tests.) * * Mirrors the sibling suppression case in `c-cpp-typedef-legacy-parse.test.ts`. */ @@ -113,11 +115,24 @@ describe('#2687 export-const function twin', () => { expect(labelsOf(nodes, 'ternary')).toEqual(['Const']); }); - it('keeps the Variable node for a var-bound function-expression', async () => { - // `var` has no matching `@definition.function` pattern, so nothing claims - // the name and the value node must survive untouched. + it('collapses a var-bound function-expression to one Function node', async () => { + // `var` originally had no `@definition.function` pattern, so the value node + // survived unclaimed and this asserted `Variable`. That was a gap, not a + // decision: a call through the binding still resolved via the declaration + // route, so the CALLS edge pointed at a NON-callable node. `var` now claims + // the name like const/let (#2693), and the dedup collapses the pair to ONE + // node — a twin here would mean the rule is anchored on a different node + // than the value rule. const nodes = await parseNodes('src/var.ts', 'var legacy = function () {\n return 3;\n};\n'); + expect(labelsOf(nodes, 'legacy')).toEqual(['Function']); + }); + + it('keeps the Variable node for a var-bound NON-function initializer', async () => { + // The property the previous case used to cover: when nothing claims the + // name, the value node must survive untouched. + const nodes = await parseNodes('src/varvalue.ts', 'var legacy = 3;\n'); + expect(labelsOf(nodes, 'legacy')).toEqual(['Variable']); });