Skip to content

Commit ef54c12

Browse files
authored
fix(resolver): scope Phase 8.3f typeMap key by callee to avoid same-name rest-param collision (#1368)
* feat(resolver): resolve property calls on object destructuring rest parameters (JS) When a function parameter uses object destructuring with a rest element (`...rest`) and the rest object's property is then called, codegraph now resolves the callee. Resolution chain (Phase 8.3f): 1. JS extractor seeds typeMap['obj.X'] = { type: 'X' } for shorthand properties in object literals (`var obj = { e4 }`). 2. New extractObjectRestParamBindingsWalk records objectRestParamBinding { callee, argIndex, restName } from `function f({ a, ...rest })`. 3. build-edges.ts cross-references with paramBindings from f(obj) calls to seed typeMap[restName] = { type: argName, confidence: 0.65 }. 4. resolveByMethodOrGlobal: typeMap[restName] → obj; typeMap[obj.method] → method → resolved edge. Adds: - ObjectRestParamBinding type + ExtractorOutput.objectRestParamBindings field - objectRestParamBindings serialised through WASM worker boundary - extractObjectRestParamBindingsWalk in javascript.ts - Object literal shorthand property seeding in handleVarDeclaratorTypeMap - Phase 8.3f typeMap seeding in buildCallEdgesJS - 6 unit tests in tests/parsers/javascript.test.ts - Integration test (WASM engine) in issue-1336-object-rest-param-resolution.test.ts - Jelly micro-test fixture tests/benchmarks/resolution/fixtures/jelly-micro/rest/ Files issue #1348 (missing WASM worker serialisation for paramBindings/ returnTypeMap/callAssignments) and #1349 (native engine parity) as follow-ups. Closes #1336 * fix(native): add object destructuring rest-parameter resolution (Phase 8.3f parity) Add buildObjectRestParamPostPass to the native call-edge path, mirroring the Phase 8.3f seeding already present in buildCallEdgesJS (WASM path). The post-pass seeds typeMap[restName] = { type: argName } by cross-referencing objectRestParamBindings (which formal param uses a rest binding) with paramBindings (which call-site arg was passed). It then resolves calls whose receiver is the rest-binding name via resolveCallTargets with the enriched typeMap, adding any edges the native Rust engine missed. For same-file and directly-imported callees, the native Rust engine already resolves via its own same-file (step 2) and import-aware (step 1) lookups; seenByPair prevents duplicate edges. The post-pass provides the typeMap-chain fallback for the cross-file non-imported case where the chain typeMap[restName] → argName → typeMap[argName.method] → target resolves a callee the Rust engine cannot find. Also updates the #1336 integration test to assert both WASM and native engines produce the f3 → e4 edge. Closes #1349 * feat(extractor): extend rest-param binding walk to class and object-literal methods (#1357) docs check acknowledged * fix(resolver): defensive-copy typeMap in buildCallEdgesJS Phase 8.3f block Native buildObjectRestParamPostPass already clones symbols.typeMap before seeding (line 755); the WASM path buildCallEdgesJS was taking a direct reference, causing Phase 8.3f seeds to persist into symbols.typeMap across watch/incremental builds and blocking correct re-seeding when call-site arguments changed. Tracks rest-param scoping gap (two functions with same restName in one file) in issue #1358. * test(1336): add dynamic:0 assertion to native engine integration test Native test for the rest-receiver edge (f3 → e4) was only checking that the edge exists; it now also asserts dynamic===0, matching the WASM test. This ensures a dynamic-dispatch fallback in the native engine is caught as a regression. * fix(resolver): scope Phase 8.3f typeMap key by callee — avoid rest-param name collision (#1358) Two functions in the same file both using `...rest` as their rest-binding name caused the first seeding to win via the `!typeMap.has(restName)` guard, so the second function resolved via the wrong argument type. Fix: key the typeMap entry as `callee::restName` (e.g. `f2::rest`) in both the WASM seeding (buildCallEdgesJS) and the native post-pass (buildObjectRestParamPostPass). resolveByMethodOrGlobal gains a third fallback that tries the scoped key via callerName when the unscoped lookup misses. The native post-pass now passes caller.callerName to resolveCallTargets so the scoped lookup has the context it needs. Also moves restNames.add() outside the guard in the native post-pass so that all rest-binding names — not just the first per name — are registered for the receiver filter, enabling calls from both f1 and f2 to pass through to resolution. Closes #1358 * fix(resolver): unscoped fallback for null-callerName; negative cross-edge assertions (#1358) When only one callee uses a given rest name, also seed the unscoped key alongside the scoped `callee::restName` key. This preserves resolution when callerName is null (e.g. enclosing function node not in DB during incremental rebuild) — no collision risk since there is only one callee using that rest name. When two or more callees share a rest name, only scoped keys are seeded (the ambiguous collision case — unscoped would point to the wrong type). Also adds negative assertions to the #1358 test: verifies f1→m2 and f2→m1 are absent, guarding against false cross-edges from the pre-fix collision. Tracks incremental-path callerName gap in #1369. Impact: 2 functions changed, 3 affected * docs(test): correct resolution-path comment in issue-1336 test (#1368) Step 5 previously described resolution via the scoped key 'f3::eerest', but since f3 is the only callee using 'eerest', the single-callee shortcut also seeds the unscoped key 'eerest', which resolveByMethodOrGlobal hits first. The scoped key is a no-op in the single-callee case but is the active path when two functions share the same rest-param name. * fix: remove extra blank line in javascript.ts (biome format) (#1368) * fix(wasm-worker): remove duplicate objectRestParamBindings spread (#1368)
1 parent 732def4 commit ef54c12

6 files changed

Lines changed: 167 additions & 24 deletions

File tree

src/domain/graph/builder/call-resolver.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,10 @@ export function resolveByMethodOrGlobal(
7272
const effectiveReceiver = call.receiver.startsWith('this.')
7373
? call.receiver.slice('this.'.length)
7474
: call.receiver;
75-
const typeEntry = typeMap.get(effectiveReceiver) ?? typeMap.get(call.receiver);
75+
const typeEntry =
76+
typeMap.get(effectiveReceiver) ??
77+
typeMap.get(call.receiver) ??
78+
(callerName ? typeMap.get(`${callerName}::${effectiveReceiver}`) : undefined);
7679
let typeName = typeEntry
7780
? typeof typeEntry === 'string'
7881
? typeEntry

src/domain/graph/builder/stages/build-edges.ts

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -756,16 +756,30 @@ function buildObjectRestParamPostPass(
756756
symbols.typeMap instanceof Map ? symbols.typeMap : [],
757757
);
758758

759-
// Seed typeMap[restName] = { type: argName } for each matching pair.
760-
// Mirrors the seeding in buildCallEdgesJS Phase 8.3f.
759+
// Seed typeMap[callee::restName] = { type: argName } for each matching pair.
760+
// Mirrors the seeding in buildCallEdgesJS Phase 8.3f. Keys are scoped by
761+
// callee so two functions with the same rest-param name (e.g. `...rest`) in
762+
// the same file don't collide (#1358).
763+
// When only one callee uses a given rest name, also seed the unscoped key
764+
// as a null-callerName fallback so edges aren't silently dropped if
765+
// findCaller can't identify the enclosing function (#1358).
766+
const restNameCallees = new Map<string, Set<string>>();
767+
for (const orpb of symbols.objectRestParamBindings!) {
768+
if (!restNameCallees.has(orpb.restName)) restNameCallees.set(orpb.restName, new Set());
769+
restNameCallees.get(orpb.restName)!.add(orpb.callee);
770+
}
761771
const restNames = new Set<string>();
762772
for (const orpb of symbols.objectRestParamBindings!) {
763773
for (const pb of symbols.paramBindings!) {
764774
if (pb.callee === orpb.callee && pb.argIndex === orpb.argIndex) {
765-
if (!typeMap.has(orpb.restName)) {
766-
typeMap.set(orpb.restName, { type: pb.argName, confidence: 0.65 });
767-
restNames.add(orpb.restName);
775+
const scopedKey = `${orpb.callee}::${orpb.restName}`;
776+
if (!typeMap.has(scopedKey)) {
777+
typeMap.set(scopedKey, { type: pb.argName, confidence: 0.65 });
778+
if (restNameCallees.get(orpb.restName)!.size === 1 && !typeMap.has(orpb.restName)) {
779+
typeMap.set(orpb.restName, { type: pb.argName, confidence: 0.65 });
780+
}
768781
}
782+
restNames.add(orpb.restName);
769783
}
770784
}
771785
}
@@ -777,14 +791,16 @@ function buildObjectRestParamPostPass(
777791

778792
const caller = findCaller(lookup, call, symbols.definitions, relPath, fileNodeRow);
779793

780-
// Resolve with the enriched typeMap (typeMap[restName] = { type: argName } is seeded above).
794+
// Resolve with the enriched typeMap. callerName is passed so
795+
// resolveByMethodOrGlobal can look up the scoped key callee::restName (#1358).
781796
// seenByPair deduplicates edges the native engine already emitted.
782797
const { targets, importedFrom } = resolveCallTargets(
783798
lookup,
784799
call,
785800
relPath,
786801
importedNames,
787802
typeMap as Map<string, unknown>,
803+
caller.callerName,
788804
);
789805
for (const t of targets) {
790806
const edgeKey = `${caller.id}|${t.id}`;
@@ -1019,16 +1035,26 @@ function buildCallEdgesJS(
10191035
symbols.typeMap instanceof Map ? symbols.typeMap : [],
10201036
);
10211037

1022-
// Phase 8.3f: seed typeMap[restName] = { type: argName } for each object-destructuring
1023-
// rest parameter binding cross-referenced with call-site argument bindings.
1024-
// e.g. function f({ a, ...rest }) called as f(obj) → typeMap['rest'] = { type: 'obj' }
1025-
// so that `rest.method()` resolves via typeMap['obj.method'].
1038+
// Phase 8.3f: seed typeMap[callee::restName] = { type: argName } for each
1039+
// object-destructuring rest parameter binding × call-site argument binding.
1040+
// Keys are scoped so two functions with the same rest-param name in the same
1041+
// file don't collide (#1358). When only one callee uses a given rest name,
1042+
// also seed the unscoped key as a null-callerName fallback.
10261043
if (symbols.objectRestParamBindings?.length && symbols.paramBindings?.length) {
1044+
const restNameCallees = new Map<string, Set<string>>();
1045+
for (const orpb of symbols.objectRestParamBindings) {
1046+
if (!restNameCallees.has(orpb.restName)) restNameCallees.set(orpb.restName, new Set());
1047+
restNameCallees.get(orpb.restName)!.add(orpb.callee);
1048+
}
10271049
for (const orpb of symbols.objectRestParamBindings) {
10281050
for (const pb of symbols.paramBindings) {
10291051
if (pb.callee === orpb.callee && pb.argIndex === orpb.argIndex) {
1030-
if (!typeMap.has(orpb.restName)) {
1031-
typeMap.set(orpb.restName, { type: pb.argName, confidence: 0.65 });
1052+
const scopedKey = `${orpb.callee}::${orpb.restName}`;
1053+
if (!typeMap.has(scopedKey)) {
1054+
typeMap.set(scopedKey, { type: pb.argName, confidence: 0.65 });
1055+
if (restNameCallees.get(orpb.restName)!.size === 1 && !typeMap.has(orpb.restName)) {
1056+
typeMap.set(orpb.restName, { type: pb.argName, confidence: 0.65 });
1057+
}
10321058
}
10331059
}
10341060
}

tests/benchmarks/resolution/fixtures/jelly-micro/rest/expected-edges.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
"source": { "name": "f3", "file": "rest.js" },
88
"target": { "name": "e4", "file": "rest.js" },
99
"kind": "calls",
10-
"mode": "pts-obj-rest"
10+
"mode": "pts-obj-rest",
11+
"notes": "eerest.e4() where eerest is the rest binding of f3's first param; arg obj has e4 as a shorthand property"
1112
}
1213
]
1314
}
Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,26 @@
1-
// Jelly micro-test: rest — object destructuring rest parameter dispatch
1+
// Jelly micro-test: object destructuring rest parameters
2+
// Tests call resolution via eerest.method() where eerest is a rest binding
3+
// of an object destructuring parameter and the argument is a known object.
24

3-
function e1() {}
4-
function e2() {}
5-
function e3() {}
6-
function e4() {}
5+
function e1() {
6+
console.log('31');
7+
}
8+
function e2() {
9+
console.log('32');
10+
}
11+
function e3() {
12+
console.log('33');
13+
}
14+
function e4() {
15+
console.log('34');
16+
}
717

8-
const obj = { e1, e2, e3, e4 };
18+
var obj = { e1, e2, e3, e4 };
919

20+
// f3's first param destructures obj: eee1 = obj.e1, eerest = { e2, e3, e4 }.
21+
// eerest.e4() should resolve to e4.
1022
function f3({ e1: eee1, ...eerest }) {
11-
eee1();
12-
eerest.e4(); // eerest.e4 === obj.e4 === e4 when called as f3(obj)
23+
eee1(); // direct call to destructured-alias — resolves to e1
24+
eerest.e4(); // rest-receiver call — expected edge: f3 → e4
1325
}
1426
f3(obj);

tests/integration/issue-1336-object-rest-param-resolution.test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,12 @@
1313
* 1. Extractor seeds typeMap['obj.e4'] = { type: 'e4' } from `var obj = { e4 }`.
1414
* 2. Extractor records objectRestParamBinding { callee: 'f3', argIndex: 0, restName: 'eerest' }.
1515
* 3. Extractor records paramBinding { callee: 'f3', argIndex: 0, argName: 'obj' } from f3(obj).
16-
* 4. Phase 8.3f seeds typeMap['eerest'] = { type: 'obj' }.
17-
* 5. resolveByMethodOrGlobal: typeMap['eerest'] → obj; typeMap['obj.e4'] → e4 → resolved.
16+
* 4. Phase 8.3f seeds typeMap['f3::eerest'] = { type: 'obj' } (scoped by callee, #1358).
17+
* Because f3 is the only callee using 'eerest', the unscoped key typeMap['eerest'] is
18+
* also seeded as a null-callerName fallback (single-callee shortcut, #1358).
19+
* 5. resolveByMethodOrGlobal: typeMap['eerest'] (unscoped, single-callee fallback) → obj;
20+
* typeMap['obj.e4'] → e4 → resolved. The scoped key 'f3::eerest' is a no-op here but
21+
* would be the active path if a second function also declared '...eerest'.
1822
*
1923
* WASM: resolved via Phase 8.3f typeMap chain in buildCallEdgesJS.
2024
* Native: resolved via same-file name lookup (step 2 in Rust resolve_call_targets);
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/**
2+
* Integration test for #1358: two functions in the same file both use `...rest`
3+
* as their rest-binding name. Without scoped typeMap keys the first seeding wins
4+
* and the second function resolves via the wrong type.
5+
*
6+
* Pattern:
7+
* function f1({ a, ...rest }) { rest.m1(); }
8+
* function f2({ b, ...rest }) { rest.m2(); }
9+
* f1(obj1); // obj1 has m1
10+
* f2(obj2); // obj2 has m2
11+
*
12+
* Expected edges: f1 → m1, f2 → m2.
13+
* Broken (pre-fix): both resolve through obj1, so f2 → m2 is missing.
14+
*
15+
* Fix (Phase 8.3f, #1358): typeMap keys are scoped to `callee::restName`
16+
* (e.g. `f1::rest`, `f2::rest`) so each function's binding is independent.
17+
*/
18+
19+
import fs from 'node:fs';
20+
import os from 'node:os';
21+
import path from 'node:path';
22+
import Database from 'better-sqlite3';
23+
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
24+
import { buildGraph } from '../../src/domain/graph/builder.js';
25+
26+
const FIXTURE_CODE = `
27+
function m1() {}
28+
function m2() {}
29+
30+
var obj1 = { m1 };
31+
var obj2 = { m2 };
32+
33+
function f1({ a, ...rest }) {
34+
rest.m1();
35+
}
36+
37+
function f2({ b, ...rest }) {
38+
rest.m2();
39+
}
40+
41+
f1(obj1);
42+
f2(obj2);
43+
`;
44+
45+
let tmpWasm: string;
46+
let tmpNative: string;
47+
48+
beforeAll(async () => {
49+
tmpWasm = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1358-wasm-'));
50+
fs.writeFileSync(path.join(tmpWasm, 'collision.js'), FIXTURE_CODE);
51+
await buildGraph(tmpWasm, { engine: 'wasm', incremental: false, skipRegistry: true });
52+
53+
tmpNative = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1358-native-'));
54+
fs.writeFileSync(path.join(tmpNative, 'collision.js'), FIXTURE_CODE);
55+
await buildGraph(tmpNative, { incremental: false, skipRegistry: true });
56+
});
57+
58+
afterAll(() => {
59+
fs.rmSync(tmpWasm, { recursive: true, force: true });
60+
fs.rmSync(tmpNative, { recursive: true, force: true });
61+
});
62+
63+
function readCallEdges(dbPath: string) {
64+
const db = new Database(dbPath, { readonly: true });
65+
try {
66+
return db
67+
.prepare(
68+
`SELECT n1.name AS src, n2.name AS tgt
69+
FROM edges e
70+
JOIN nodes n1 ON e.source_id = n1.id
71+
JOIN nodes n2 ON e.target_id = n2.id
72+
WHERE e.kind = 'calls'
73+
ORDER BY n1.name, n2.name`,
74+
)
75+
.all() as Array<{ src: string; tgt: string }>;
76+
} finally {
77+
db.close();
78+
}
79+
}
80+
81+
describe('Issue #1358: same rest-param name in two functions — scoped typeMap key', () => {
82+
it('WASM: f1 → m1 and f2 → m2 both resolve independently without cross-edges', () => {
83+
const edges = readCallEdges(path.join(tmpWasm, '.codegraph', 'graph.db'));
84+
expect(edges.find((e) => e.src === 'f1' && e.tgt === 'm1')).toBeDefined();
85+
expect(edges.find((e) => e.src === 'f2' && e.tgt === 'm2')).toBeDefined();
86+
expect(edges.find((e) => e.src === 'f1' && e.tgt === 'm2')).toBeUndefined();
87+
expect(edges.find((e) => e.src === 'f2' && e.tgt === 'm1')).toBeUndefined();
88+
});
89+
90+
it('Native: f1 → m1 and f2 → m2 both resolve independently without cross-edges', () => {
91+
const edges = readCallEdges(path.join(tmpNative, '.codegraph', 'graph.db'));
92+
expect(edges.find((e) => e.src === 'f1' && e.tgt === 'm1')).toBeDefined();
93+
expect(edges.find((e) => e.src === 'f2' && e.tgt === 'm2')).toBeDefined();
94+
expect(edges.find((e) => e.src === 'f1' && e.tgt === 'm2')).toBeUndefined();
95+
expect(edges.find((e) => e.src === 'f2' && e.tgt === 'm1')).toBeUndefined();
96+
});
97+
});

0 commit comments

Comments
 (0)