scriptoriumadmin/GitNexus
1
0
Fork 0
mirror of https://github.com/abhigyanpatwari/GitNexus.git synced 2026-10-09 03:17:54 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

feat(scope-resolution): resolve closure bindings in Ruby, Java, C#, PHP and JS/TS var (#2693)

Browse source 
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. PHP additionally had
no scope declaration for the bound name, so the flow pass had nothing to attach
its seed to.

  ruby    handler = ->(x) { x }        handler.call(1)   -> Function:a.rb:handler
  java    Function<..> handler = x->x  handler.apply(1)  -> Function:A.java:A.handler
  csharp  Func<int,int> handler = ...  handler(1)        -> Function:A.cs:A.handler
  php     $handler = fn($x) => $x      $handler(1)       -> Function:a.php:handler

Ruby and Java invoke through the callable-object protocol; C# and PHP call the
binding directly. Locals work in all four, and a binding whose name collides
with a same-named method resolves to the CLOSURE, not the method.

Two things the sweep caught:

JAVA TWIN. Anchoring the rule on the inner variable_declarator produced BOTH a
Function and a Property node — the exact double-indexing #2687 removed. The
parse-worker dedup keys on (definition node, name), and Java's value rule
anchors on field_declaration, so the keys never matched. Re-anchored on
field_declaration / local_variable_declaration.

JS/TS `var`. `var f = (x) => x` kept a Variable label while const/let got
Function, because `var` is a different grammar node (variable_declaration vs
lexical_declaration) that no closure rule covered. A call through the binding
still resolved via the declaration route, so the CALLS edge pointed at a
NON-callable node. Now consistent across const/let/var.

That last one flipped an existing assertion in const-function-twin.test.ts,
which expected `Variable` for a var-bound function-expression. Its comment
explained why — "var has no matching @definition.function pattern, so nothing
claims the name" — i.e. it documented the gap rather than defending it. The
property it was really protecting (an UNCLAIMED value node survives) now has
its own case with a non-function initializer, and the var-closure case asserts
the collapse to one node, which is also the twin guard for the new rule.

Known limits, both pre-existing and both failing safe:

  - A PHP local closure whose name collides with a top-level function gets no
    edge: both want id Function:<file>:<name>, so the closure never gets its own
    node. This is the file-scoped node-identity convention — TypeScript, Python
    and Dart collapse identically at base.
  - TS/JS class-field arrows stay Property (Kotlin's equivalent emits Method).
    They already resolve; changing the label risks the HAS_PROPERTY ownership
    regression #2687 hit once.

The invalidation constants already bumped in this PR (INCREMENTAL_SCHEMA_VERSION
16, SCHEMA_BUMP 23) cover these additional languages; their notes now say so.

Tests: one case per newly-resolving language plus the PHP anonymous-function
form and the JS var form, in closure-binding-labels.test.ts. The file now spins
a worker pool per test across a dozen languages, so its timeout is raised
file-wide — a case that takes ~7s alone was exceeding the 30s default under
that contention.
This commit is contained in:
Gergo Magyar 2026-07-25 22:45:51 +00:00
parent 3b631c9cac
commit 6ba8c52a20
6 changed files with 238 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
gitnexus/src/core/ingestion/languages/php/query.ts
View file

@ -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.

111
gitnexus/src/core/ingestion/tree-sitter-queries.ts
View file

@ -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)

6
gitnexus/src/storage/parse-cache.ts
View file

@ -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 = <arrow | function-expression>` 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

10
gitnexus/src/storage/repo-manager.ts
View file

@ -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;

84
gitnexus/test/integration/closure-binding-labels.test.ts
View file

@ -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<string[]> => {
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<Integer,Integer> 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<int,int> 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',
'<?php\n$handler = fn($x) => $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',
'<?php\n$handler = function ($x) { return $x; };\n' +
'function caller() {\n global $handler;\n return $handler(1);\n}\n',
);
expect(targets).toEqual(['Function:b.php:handler']);
});
it('JavaScript: a `var` closure binding is a Function, like const/let', async () => {
// `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

25
gitnexus/test/integration/const-function-twin.test.ts
View file

@ -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']);
});