Skip to content

Commit 38cf57b

Browse files
committed
fix: resolve merge conflicts with main (bind/call/apply PR #1330)
- Keep both define-property.js edges (5, from this PR) and bind-call-apply.js edges (3, from main #1330) in expected-edges.json -- total now 33 - Preserve handleDefinePropertyTypeMap, findDescriptorValue, seedProtoProperties, and Object.create composite-key seeding (auto-merged correctly) - Incorporate bind/call/apply tracking code from main into javascript.ts - Update JS edge count comment in resolution-benchmark.test.ts (30 -> 33)
2 parents eae19de + a233110 commit 38cf57b

5 files changed

Lines changed: 185 additions & 9 deletions

File tree

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

Lines changed: 122 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,7 @@ function buildParamFlowPtsPostPass(
552552
ctx: PipelineContext,
553553
getNodeIdStmt: NodeIdStmt,
554554
allEdgeRows: EdgeRowTuple[],
555+
sharedLookup?: CallNodeLookup,
555556
): void {
556557
// Only process files that actually have paramBindings (avoid useless work).
557558
const filesWithParams = [...ctx.fileSymbols].filter(
@@ -567,7 +568,7 @@ function buildParamFlowPtsPostPass(
567568
}
568569

569570
const { barrelOnlyFiles, rootDir } = ctx;
570-
const lookup = makeContextLookup(ctx, getNodeIdStmt);
571+
const lookup = sharedLookup ?? makeContextLookup(ctx, getNodeIdStmt);
571572

572573
for (const [relPath, symbols] of filesWithParams) {
573574
if (barrelOnlyFiles.has(relPath)) continue;
@@ -620,6 +621,94 @@ function buildParamFlowPtsPostPass(
620621
}
621622
}
622623

624+
/**
625+
* bind/alias pts post-pass for the native call-edge path.
626+
*
627+
* The native Rust engine has no knowledge of JS-layer fnRefBindings (e.g.
628+
* `const f = fn.bind(ctx)`), so calls to bind-created aliases are not resolved
629+
* to their original function on the native path. This JS post-pass runs after
630+
* the native edge pass and adds only the fnRefBindings-seeded pts edges that the
631+
* native engine missed.
632+
*
633+
* Uses the same seenByPair dedup guard as buildParamFlowPtsPostPass to avoid
634+
* duplicating edges already emitted by the native engine.
635+
*/
636+
function buildFnRefBindingsPtsPostPass(
637+
ctx: PipelineContext,
638+
getNodeIdStmt: NodeIdStmt,
639+
allEdgeRows: EdgeRowTuple[],
640+
sharedLookup?: CallNodeLookup,
641+
): void {
642+
// Only process files that actually have fnRefBindings.
643+
const filesWithBindings = [...ctx.fileSymbols].filter(
644+
([, symbols]) => symbols.fnRefBindings && symbols.fnRefBindings.length > 0,
645+
);
646+
if (filesWithBindings.length === 0) return;
647+
648+
// Seed seenByPair from the existing rows so we don't duplicate native edges.
649+
const seenByPair = new Set<string>();
650+
for (const [srcId, tgtId] of allEdgeRows) {
651+
seenByPair.add(`${srcId}|${tgtId}`);
652+
}
653+
654+
const { barrelOnlyFiles, rootDir } = ctx;
655+
const lookup = sharedLookup ?? makeContextLookup(ctx, getNodeIdStmt);
656+
657+
for (const [relPath, symbols] of filesWithBindings) {
658+
if (barrelOnlyFiles.has(relPath)) continue;
659+
const fileNodeRow = getNodeIdStmt.get(relPath, 'file', relPath, 0);
660+
if (!fileNodeRow) continue;
661+
662+
const importedNames = buildImportedNamesMap(ctx, relPath, symbols, rootDir);
663+
const typeMap: Map<string, TypeMapEntry | string> = symbols.typeMap || new Map();
664+
const ptsMap = buildPointsToMapForFile(symbols, importedNames);
665+
if (!ptsMap) continue;
666+
667+
// Only resolve calls whose name is an lhs in fnRefBindings — the same
668+
// narrowed guard used in buildFileCallEdges case (c).
669+
const fnRefBindingLhs = new Set(symbols.fnRefBindings!.map((b) => b.lhs));
670+
671+
for (const call of symbols.calls) {
672+
if (call.receiver || call.dynamic) continue; // bind aliases are flat-keyed, never dynamic
673+
if (!fnRefBindingLhs.has(call.name)) continue;
674+
if (!ptsMap.has(call.name)) continue;
675+
676+
const caller = findCaller(lookup, call, symbols.definitions, relPath, fileNodeRow);
677+
678+
// Only resolve calls that had no direct targets (same guard as buildFileCallEdges).
679+
const { targets } = resolveCallTargets(
680+
lookup,
681+
call,
682+
relPath,
683+
importedNames,
684+
typeMap as Map<string, unknown>,
685+
);
686+
if (targets.length > 0) continue;
687+
688+
for (const alias of resolveViaPointsTo(call.name, ptsMap)) {
689+
const { targets: aliasTargets, importedFrom: aliasFrom } = resolveCallTargets(
690+
lookup,
691+
{ name: alias },
692+
relPath,
693+
importedNames,
694+
typeMap as Map<string, unknown>,
695+
);
696+
for (const t of aliasTargets) {
697+
const edgeKey = `${caller.id}|${t.id}`;
698+
if (t.id !== caller.id && !seenByPair.has(edgeKey)) {
699+
const conf =
700+
computeConfidence(relPath, t.file, aliasFrom ?? null) - PROPAGATION_HOP_PENALTY;
701+
if (conf > 0) {
702+
seenByPair.add(edgeKey);
703+
allEdgeRows.push([caller.id, t.id, 'calls', conf, 0, 'points-to']);
704+
}
705+
}
706+
}
707+
}
708+
}
709+
}
710+
}
711+
623712
/**
624713
* Phase 8.5: CHA + RTA post-pass for the native call-edge path.
625714
*
@@ -889,6 +978,12 @@ function buildFileCallEdges(
889978
// no longer tracked here.
890979
const ptsEdgeRows = new Map<string, number>();
891980

981+
// Pre-compute the set of names that appear as lhs in fnRefBindings so that
982+
// case (c) of the pts gate below only fires for names that are genuine
983+
// bind/alias entries, not for every locally-defined function or import that
984+
// buildPointsToMap seeds with a self-pointing entry.
985+
const fnRefBindingLhs = new Set(symbols.fnRefBindings?.map((b) => b.lhs) ?? []);
986+
892987
for (const call of symbols.calls) {
893988
if (call.receiver && BUILTIN_RECEIVERS.has(call.receiver)) continue;
894989

@@ -950,26 +1045,39 @@ function buildFileCallEdges(
9501045
}
9511046
}
9521047

953-
// Phase 8.3 / 8.3c: points-to fallback for unresolved calls.
954-
// Fires for two cases:
1048+
// Phase 8.3 / 8.3c / bind: points-to fallback for unresolved calls.
1049+
// Fires for three cases:
9551050
// (a) dynamic=true: alias calls emitted by extractCallbackReferenceCalls.
9561051
// Looks up `call.name` directly (alias entries are flat-keyed).
9571052
// (b) non-dynamic: parameter variable calls (fn() where fn is a param).
9581053
// Looks up the scoped key `callerName::call.name` to avoid spurious
9591054
// edges from same-named parameters across different functions.
1055+
// (c) non-dynamic: module-level alias bindings — `f = fn.bind(ctx)` or
1056+
// `const f = handler` — where pts('f') was seeded by fnRefBindings.
1057+
// Checked against fnRefBindingLhs (the pre-computed set of lhs names from
1058+
// fnRefBindings) rather than the full ptsMap, so case (c) only fires for
1059+
// genuine bind/alias entries and never for self-seeded local definitions.
9601060
// Confidence is penalised by one hop to reflect the extra indirection.
9611061
//
9621062
// Note: pts edges are added to ptsEdgeRows (not seenCallEdges) so that a later
9631063
// direct call to the same target in the same function body can upgrade confidence
9641064
// rather than being silently dropped by the dedup guard.
9651065
const scopedPtsKey = caller.callerName != null ? `${caller.callerName}::${call.name}` : null;
1066+
const flatPtsKey =
1067+
!call.dynamic && fnRefBindingLhs.has(call.name) && ptsMap?.has(call.name) ? call.name : null;
9661068
if (
9671069
targets.length === 0 &&
9681070
!call.receiver &&
9691071
ptsMap &&
970-
(call.dynamic || (scopedPtsKey != null && ptsMap.has(scopedPtsKey)))
1072+
(call.dynamic || (scopedPtsKey != null && ptsMap.has(scopedPtsKey)) || flatPtsKey != null)
9711073
) {
972-
const ptsLookupName = call.dynamic ? call.name : (scopedPtsKey ?? call.name);
1074+
const ptsLookupName = call.dynamic
1075+
? call.name
1076+
: scopedPtsKey != null && ptsMap.has(scopedPtsKey)
1077+
? scopedPtsKey
1078+
: // flatPtsKey != null is guaranteed by the outer if condition: if neither
1079+
// call.dynamic nor scopedPtsKey matched, flatPtsKey != null must be true.
1080+
flatPtsKey!;
9731081
for (const alias of resolveViaPointsTo(ptsLookupName, ptsMap)) {
9741082
// Resolve the concrete alias target. Only `name` is needed here — receiver
9751083
// and line are not relevant for alias resolution (we are looking up the
@@ -1360,12 +1468,20 @@ export async function buildEdges(ctx: PipelineContext): Promise<void> {
13601468
(ctx.isFullBuild || ctx.fileSymbols.size > ctx.config.build.smallFilesThreshold);
13611469
if (useNativeCallEdges) {
13621470
buildCallEdgesNative(ctx, getNodeIdStmt, allEdgeRows, allNodesBefore, native!);
1471+
// Build the shared lookup once — both pts post-passes use it, avoiding
1472+
// redundant construction of the same context closure.
1473+
const sharedLookup = makeContextLookup(ctx, getNodeIdStmt);
13631474
// Phase 8.3c post-pass: augment native call edges with parameter-flow pts
13641475
// edges. The native Rust engine has no knowledge of paramBindings, so any
13651476
// `fn()` call inside a higher-order function would be missed. This JS pass
13661477
// runs on top of the native edges and adds only the pts-resolved edges that
13671478
// the native engine could not produce.
1368-
buildParamFlowPtsPostPass(ctx, getNodeIdStmt, allEdgeRows);
1479+
buildParamFlowPtsPostPass(ctx, getNodeIdStmt, allEdgeRows, sharedLookup);
1480+
// bind/alias post-pass: augment native call edges with fnRefBindings-seeded
1481+
// pts edges. The native Rust engine has no knowledge of JS fnRefBindings
1482+
// (e.g. `const f = fn.bind(ctx)`), so calls to bind-created aliases are
1483+
// not resolved to their original function on the native path.
1484+
buildFnRefBindingsPtsPostPass(ctx, getNodeIdStmt, allEdgeRows, sharedLookup);
13691485
// Phase 8.5 post-pass: augment native call edges with CHA-resolved dispatch.
13701486
// The native Rust engine has no knowledge of the CHA context, so this/self
13711487
// calls and interface dispatch are not expanded to concrete implementations.

src/extractors/javascript.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1456,8 +1456,7 @@ function handleVarDeclaratorTypeMap(
14561456

14571457
// Phase 8.3: record function-reference bindings before any type-analysis early returns.
14581458
// Captures `const fn = handler` (identifier) and `const fn = obj.method` (member_expression).
1459-
// call_expression and new_expression are intentionally excluded — those are handled by
1460-
// Phase 8.2 callAssignments and the constructor type-map respectively.
1459+
// Also handles `const f = fn.bind(ctx)` — bind returns a new function aliasing fn.
14611460
if (fnRefBindings && valueN) {
14621461
if (valueN.type === 'identifier' && !BUILTIN_GLOBALS.has(valueN.text)) {
14631462
fnRefBindings.push({ lhs: nameN.text, rhs: valueN.text });
@@ -1475,6 +1474,21 @@ function handleVarDeclaratorTypeMap(
14751474
) {
14761475
fnRefBindings.push({ lhs: nameN.text, rhs: prop.text, rhsReceiver: obj.text });
14771476
}
1477+
} else if (valueN.type === 'call_expression') {
1478+
// `const f = fn.bind(ctx)` — bind returns a bound copy of fn; track f → fn so
1479+
// pts(f) ⊇ pts(fn) and subsequent `f(args)` calls resolve to fn.
1480+
// Note: only flat-identifier binds (fn.bind) are tracked here; method-receiver
1481+
// binds like `obj.method.bind(ctx)` are not captured (boundFn must be an identifier).
1482+
const callFn = valueN.childForFieldName('function');
1483+
if (callFn?.type === 'member_expression') {
1484+
const bindProp = callFn.childForFieldName('property');
1485+
if (bindProp?.text === 'bind') {
1486+
const boundFn = callFn.childForFieldName('object');
1487+
if (boundFn?.type === 'identifier' && !BUILTIN_GLOBALS.has(boundFn.text)) {
1488+
fnRefBindings.push({ lhs: nameN.text, rhs: boundFn.text });
1489+
}
1490+
}
1491+
}
14781492
}
14791493
}
14801494

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Patterns for Function.prototype.bind / .call / .apply resolution.
2+
3+
function greet(greeting) {
4+
return greeting + ' ' + this.name;
5+
}
6+
7+
var user = { name: 'Alice' };
8+
9+
// bind: var f = fn.bind(ctx) — f() should resolve to fn()
10+
var greetUser = greet.bind(user);
11+
12+
export function runBind() {
13+
return greetUser('Hello');
14+
}
15+
16+
// call: fn.call(ctx, args) — resolved as a direct call to fn
17+
export function runCall() {
18+
return greet.call(user, 'Hi');
19+
}
20+
21+
// apply: fn.apply(ctx, argsArray) — resolved as a direct call to fn
22+
export function runApply() {
23+
return greet.apply(user, ['Hey']);
24+
}

tests/benchmarks/resolution/fixtures/javascript/expected-edges.json

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,27 @@
164164
"mode": "pts-create-prototype",
165165
"notes": "obj.f2() — resolved via Object.create({ f1, f2 })"
166166
},
167+
{
168+
"source": { "name": "runBind", "file": "bind-call-apply.js" },
169+
"target": { "name": "greet", "file": "bind-call-apply.js" },
170+
"kind": "calls",
171+
"mode": "points-to",
172+
"notes": "greetUser() — greetUser = greet.bind(user), pts tracks greetUser → greet"
173+
},
174+
{
175+
"source": { "name": "runCall", "file": "bind-call-apply.js" },
176+
"target": { "name": "greet", "file": "bind-call-apply.js" },
177+
"kind": "calls",
178+
"mode": "dynamic",
179+
"notes": "greet.call(user, 'Hi') — .call() extracts greet as the callee"
180+
},
181+
{
182+
"source": { "name": "runApply", "file": "bind-call-apply.js" },
183+
"target": { "name": "greet", "file": "bind-call-apply.js" },
184+
"kind": "calls",
185+
"mode": "dynamic",
186+
"notes": "greet.apply(user, ['Hey']) — .apply() extracts greet as the callee"
187+
},
167188
{
168189
"source": { "name": "Dog.speak", "file": "inheritance.js" },
169190
"target": { "name": "Animal.speak", "file": "inheritance.js" },

tests/benchmarks/resolution/resolution-benchmark.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,8 @@ const THRESHOLDS: Record<string, { precision: number; recall: number }> = {
116116
// the correct fix is to add it to expected-edges — not to lower the threshold.
117117
// JS recall 0.9: Phase 8.3e adds Object.defineProperty/defineProperties/create composite pts keys
118118
// (5 new edges in define-property.js) + Phase 8.5 adds class-inheritance and prototype edges
119-
// (inheritance.js, prototypes.js, prototypes2.js), lifting total expected to 30.
119+
// (inheritance.js, prototypes.js, prototypes2.js), lifting total expected to 30. Phase 8.3f
120+
// adds bind/call/apply resolution (3 new edges in bind-call-apply.js), total expected now 33.
120121
javascript: { precision: 1.0, recall: 0.9 },
121122
// TS 0.72: Phase 8.3e adds this.method() same-class resolution (Shape.describe → Shape.area),
122123
// lifting recall from 69.4% to 72.2%. Remaining gap (interface-dispatch, CHA) is tracked

0 commit comments

Comments
 (0)