feat(resolver): resolve prototype-based method calls, spread/iteration callbacks, func-prop this-dispatch, and object-rest param dispatch (JS)#1331
Conversation
…ar = fn)
Teach the JS extractor and call resolver about pre-ES6 prototype OOP patterns:
Extractor (javascript.ts):
- `Foo.prototype.bar = function(){}` → emits `Foo.bar` definition (kind: method)
- `Foo.prototype.bar = f` → seeds typeMap['Foo.bar'] = { type: 'f', confidence: 0.9 }
- `Foo.prototype = { bar: fn, baz: f }` → same rules per object-literal property
Built-in globals (Array, Object, …) are excluded via BUILTIN_GLOBALS guard.
Call resolver (call-resolver.ts):
- After a symbol-DB miss on a typed receiver, checks typeMap['Type.method'] for
prototype aliases (covers `A.prototype.t = f` → call resolves to f)
- Extracts the class name from inline `(new Foo)` receivers so `(new A).t()`
resolves without a named variable binding
Both paths (query + walk) are covered. Adds 7 unit tests.
Closes #1317
|
Claude finished @carlos-alm's task —— View job PR Review
Phase 1: Root Cause AnalysisProblem Being Solved: Pre-ES6 prototype OOP ( Why This Approach: Purely additive: a new post-pass walk ( Phase 2: Configuration Change DetectionNo linting rules relaxed, no thresholds changed, no build config modified. Clean. Phase 3: Test Integrity7 new tests in Phase 4: Code QualityConcerns: 1. Dual-engine parity gap — native Rust engine is not updated 2. // Don't recurse inside expression_statement — no nested prototype assignments expected
return;This is a reasonable optimization, but the comment is load-bearing — a prototype assignment inside an IIFE or module pattern ( 3. if (key && value) emitPrototypeMethod(className, key.text, value, definitions, typeMap);For 4. const m = /^\(?\s*new\s+([A-Z_$][A-Za-z0-9_$]*)/.exec(call.receiver);The 5. No test for Phase 5: Backlog Compliance
Critical Concerns
Final Recommendation |
Greptile SummaryThis PR adds four JS call-resolution improvements: prototype-based method calls (
Confidence Score: 4/5Safe to merge after restoring paramBindings to SerializedExtractorOutput and removing the duplicate serialization lines; the core resolution logic is sound. The omission of src/domain/wasm-worker-protocol.ts, src/domain/wasm-worker-pool.ts, and src/domain/wasm-worker-entry.ts all need the paramBindings field migration completed. Important Files Changed
Reviews (19): Last reviewed commit: "test(extractor): verify exported arrow f..." | Re-trigger Greptile |
| if (node.type === 'expression_statement') { | ||
| const expr = node.child(0); | ||
| if (expr?.type === 'assignment_expression') handlePrototypeAssignment(expr, definitions, typeMap); | ||
| // Don't recurse inside expression_statement — no nested prototype assignments expected | ||
| return; |
There was a problem hiding this comment.
IIFE wrapping silently skips nested prototype assignments
When an expression_statement's first child is not an assignment_expression (e.g. an IIFE call), the walk returns early without recursing into it. The very common pre-ES6 module pattern wraps all definitions in an IIFE — (function() { Foo.prototype.bar = function(){}; })(); — and every prototype assignment inside would be silently ignored. The comment "no nested prototype assignments expected" overstates the guarantee; removing the early return and simply calling handlePrototypeAssignment only when a child is an assignment_expression — then falling through to the normal child loop — would cover this case without any extra cost.
There was a problem hiding this comment.
Fixed — the IIFE concern was already addressed in the version merged from main. The extractPrototypeMethodsWalk walk in the current code does NOT have an early return after expression_statement; the for loop always runs, so IIFE children (including nested prototype assignments) are recursed into correctly. The old first-set duplicate code (which had the early return) was removed in the fix: resolve merge conflicts with main commit.
| if (child.type === 'pair') { | ||
| const key = child.childForFieldName('key'); | ||
| const value = child.childForFieldName('value'); | ||
| if (key && value) emitPrototypeMethod(className, key.text, value, definitions, typeMap); |
There was a problem hiding this comment.
String-literal keys produce malformed definition names
For object-literal prototype assignments where a key is a quoted string — e.g. { 'bar': function(){} } — key.text returns 'bar' (quotes included), so emitPrototypeMethod is called with methodName = "'bar'" and the resulting definition name becomes Foo.'bar'. This name will never match a call-site lookup. A guard like key.type === 'property_identifier' || key.type === 'identifier' would filter these out, or the text could be stripped of surrounding quotes when the node type is string.
There was a problem hiding this comment.
Fixed — the key.text.replace(/['"]/g, '') guard for string-literal keys was already present in the version merged from main. The extractPrototypeObjectLiteral function already handles this: const methodName = keyNode.type === 'string' ? keyNode.text.replace(/['"]/g, '') : keyNode.text;
| if (!typeName && call.receiver) { | ||
| const m = /^\(?\s*new\s+([A-Z_$][A-Za-z0-9_$]*)/.exec(call.receiver); |
There was a problem hiding this comment.
Inline-
new regex only matches uppercase-initial constructors
The regex [A-Z_$][A-Za-z0-9_$]* restricts matching to PascalCase (or _/$-prefixed) names, so a receiver like (new animal()).move() would not be resolved. Pre-ES6 code often uses lowercase constructor functions (e.g. function animal() {}). If this constraint is intentional (to avoid false positives on non-constructor calls), it is worth documenting explicitly; otherwise changing the first character class to [A-Za-z_$] would broaden coverage.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Acknowledged and documented. Added a comment to the regex explaining the uppercase-initial constraint is intentional — it acts as a heuristic to distinguish constructors (PascalCase) from regular functions, avoiding false positives on patterns like (new xmlParser()).parse(). Lowercase constructor functions are uncommon in practice and the trade-off favors precision over recall here.
Codegraph Impact Analysis165 functions changed → 161 callers affected across 46 files
|
|
Addressed Claude and Greptile review feedback: 1. Native engine parity gap — Issue #1327 was filed when the PR was created and is already open. The PR description links to it. This is a genuine gap but the WASM extractor correctly handles the patterns; native parity is tracked for follow-up. 2. Quoted-key bug — Already fixed in the version merged from main: 3. IIFE wrapping — Already fixed in the version merged from main: the current 4. Shorthand property identifiers — Added handling for 5. Untested 6. Inline-new regex documentation — Added a comment explaining the uppercase-initial constraint is intentional (PascalCase heuristic to distinguish constructors). 7. Cross-file typeMap alias limitation — This is a known design boundary of the per-file typeMap architecture. Direct function assignments ( |
…ks (JS) Phase 8.3e — array-element points-to analysis for JS/TS. Closes #1321 ## What's resolved `f(...arr)`, `for (x of arr)`, `Array.from(arr, cb)`, and `new Set(arr)` patterns now produce call edges where function references flow through array operations: const arr = [a, b]; f(...arr); // f→a, f→b via spread for (x of arr) x() // outer→a, outer→b via iteration Recall on Jelly micro-test fixtures: spread 0→100%, more1 0→100%. ## Implementation - **types.ts** — 4 new interfaces: `ArrayElemBinding`, `SpreadArgBinding`, `ForOfBinding`, `ArrayCallbackBinding` - **extractors/javascript.ts** — `extractArrayElemBindingsWalk` + `extractSpreadForOfWalk` hooked into both query and walk paths - **points-to.ts** — array-element seeding, wildcard constraints, per-index spread constraints, for-of and callback constraints - **build-edges.ts** — passes new bindings to pts map builder; `buildParamFlowPtsPostPass` extended to handle all pts binding types - **wasm-worker-{protocol,entry,pool}.ts** — serializes/deserializes new bindings across the WASM Worker thread boundary - **tests/** — pts unit tests + jelly-micro fixtures for spread/more1
The Rust engine does not recognise Foo.prototype.bar = function(){} as a
method definition, so prototype-based method nodes were absent from the DB
when the native orchestrator ran. This causes the integration tests to fail
on all platforms where the native addon is available.
Fix two issues:
1. Remove duplicate extractPrototypeMethodsWalk call in extractSymbolsQuery
that was inflating the definitions array (identified by Greptile)
2. Add runPostNativePrototypeMethods post-pass to native-orchestrator.ts:
- Re-parses JS/TS files via WASM after native build
- Inserts any method nodes missing from the DB (prototype patterns)
- Resolves and inserts call edges to those new nodes using the WASM
typeMap and the call-resolver
|
Fixed — both duplicate
|
| // Only emit edges to newly-inserted prototype nodes to avoid | ||
| // duplicating edges the Rust engine already built. | ||
| if (!newNodeIds.has(t.id)) continue; |
There was a problem hiding this comment.
Cross-file prototype call edges silently dropped
The newDefFiles guard limits call scanning to only those files that themselves define new prototype methods. Any file that only calls prototype methods defined elsewhere will be skipped entirely — so a foo.speak() call in app.js will never produce an edge to Foo.speak defined in lib.js. The !newNodeIds.has(t.id) check inside the inner loop already ensures only edges to newly-inserted prototype nodes are emitted; the outer newDefFiles filter is therefore redundant as a duplicate-prevention mechanism but harmful as a scope limiter. Removing it (and keeping only the newNodeIds guard) would fix cross-file prototype resolution without emitting duplicate edges.
There was a problem hiding this comment.
Fixed — removed the newDefFiles guard so all files in wasmResults are scanned for calls to newly-inserted prototype nodes. A caller in app.js calling Foo.speak defined in lib.js now correctly produces an edge. The newNodeIds check inside the loop already prevents duplicating edges the Rust engine built. Also hoisted db.prepare() outside the loop to avoid re-preparing the same statement on every iteration. Committed in 1b84435.
…es (#1331) The newDefFiles guard restricted call scanning to only files that define new prototype methods, silently dropping call edges from files that only call those methods. A foo.speak() call in app.js to Foo.speak defined in lib.js would never produce an edge. Remove the guard — the newNodeIds check inside the loop already prevents duplicate edges. Also hoist db.prepare() outside the loop to avoid re-preparing the same statement on every iteration.
…methods (JS)
Extract `fn.method = function() {}` assignments as `method` definitions in both
the query-based and walk-based JS extraction paths, enabling `this.other()` calls
inside such methods to resolve via the existing callerName-based this-dispatch
logic in `resolveByMethodOrGlobal`.
Extend the native-engine prototype backfill post-pass to also trigger on files
containing `fn.prop = function` patterns so the same resolution applies when
the Rust orchestrator runs.
Closes #1334
|
Addressed the two issues Greptile raised in the summary review: 1. 2. |
…arameters (JS)
Adds Phase 8.3f: when a function parameter uses object destructuring with a
rest element (`function f3({ e1: eee1, ...eerest })`), and the rest object's
property is called (`eerest.e4()`), resolve the callee via a three-hop chain:
ObjectRestParamBinding (eerest ← f3 param 0)
+ ParamBinding (f3(obj) → obj at index 0)
+ ObjectPropBinding (obj = { e4 } → obj.e4 = e4)
→ pts["eerest.e4"] = {"e4"} → calls edge f3 → e4
Changes:
- types.ts: add ObjectRestParamBinding and ObjectPropBinding interfaces
- javascript.ts: extractObjectRestParamBindingsWalk (finds rest params in
object-destructured function params) and extractObjectPropBindingsWalk
(finds shorthand/identifier properties in object literals); wired into
both extractSymbolsQuery and extractSymbolsWalk paths
- wasm-worker-{protocol,entry,pool}.ts: serialize new binding arrays
- points-to.ts: seed pts["rest.propName"] = {"fn"} from the three-hop chain
- build-edges.ts: new Phase 8.3f receiver-pts fallback — when a receiver call
is unresolved, check pts["receiver.name"] for rest-dispatch targets; also
include new bindings in buildPointsToMapForFile null-check guard
Jelly micro-test benchmark (rest fixture): recall=100% TP=1 FN=0 FP=0
Closes #1336
| // Use '<module>' as sentinel for top-level for-of outside any function. | ||
| const enclosingFunc = | ||
| funcStack.length > 0 ? funcStack[funcStack.length - 1]! : '<module>'; | ||
| if (varName && !BUILTIN_GLOBALS.has(varName)) { | ||
| forOfBindings.push({ varName, sourceName: right.text, enclosingFunc }); |
There was a problem hiding this comment.
<module> sentinel for top-level for-of is never consumed
The PTS key <module>::varName is created here, but the lookup in build-edges.ts (line ~981) only fires when scopedPtsKey != null, which requires caller.callerName !== null. Module-level calls always have callerName === null (no enclosing definition found by findCaller), so scopedPtsKey is null and the condition scopedPtsKey != null && ptsMap.has(scopedPtsKey) is always false. Any for (const f of arr) { f(); } loop at the top level silently drops the call edges.
There was a problem hiding this comment.
Fixed in 66b899a. Two bugs resolved:
-
<module>sentinel never consumed — Added amodulePtsKeyfallback inbuild-edges.tsthat checks<module>::call.namewhencaller.callerName === null(module-level calls). Top-levelfor (const f of arr) { f(); }now resolves correctly. -
method_definitionunqualified name mismatch — Added aclassStacktoextractSpreadForOfWalkso thatmethod_definitionnodes push the qualified name (e.g.Foo.bar) instead of the bare name (bar). This matches whatfindCallerreturns from the definitions array, so for-of inside class methods now resolves correctly.
useOptionalChain rewrites and formatting fixes flagged by Biome in CI.
Two bugs in the forOfBindings points-to resolution path:
1. <module> sentinel never consumed: extractSpreadForOfWalk emits
ForOfBinding with enclosingFunc='<module>' for top-level for-of loops,
but build-edges.ts only looked up scopedPtsKey (null at module level).
Add a modulePtsKey fallback that checks '<module>::call.name' so
`for (const f of arr) { f(); }` at module scope resolves correctly.
2. method_definition pushes unqualified name: funcStack.push('bar') but
findCaller returns callerName='Foo.bar' from the definitions array.
Add a classStack to extractSpreadForOfWalk so method_definition nodes
push the qualified name 'Foo.bar', matching the PTS key the lookup uses.
|
Claude finished @carlos-alm's task —— View job PR Review
Summary of Changes ReviewedAll three issues raised in the previous Greptile round have been correctly addressed: 1. 2. 3. Remaining Minor Observations
if (t === 'function_declaration' || t === 'function_expression' || t === 'arrow_function') {
const nameNode = node.childForFieldName('name');
const funcName = nameNode?.text; // always null for arrow_function / function_expression
AssessmentThe two Greptile-flagged bugs are correctly fixed. The prior Claude-review items (IIFE handling, quoted-key stripping, inline-new regex comment, Rating: ⭐⭐⭐⭐☆ (4/5) |
…ter (#1331) Two bugs introduced by the fix(lint) commit (4ed709e): 1. define-property.js functions were renamed defProp/defProps/create → _defProp/ _defProps/_create (to suppress biome noUnusedVariables), but expected-edges.json was not updated. This caused 5 false positives and 5 false negatives in the benchmark (precision 84.4%, recall 81.8%). 2. DoubleCounter.count was changed from super.count() to Counter.count() by the same lint fix commit. The fixture is meant to test static class-inheritance resolution via super.count(); reverting to Counter.count() made the edge a plain same-file call, causing the class-inheritance recall to drop to 2/3. Fix: update expected-edges.json names to match renamed functions; restore super.count() in inheritance.js with a biome-ignore suppression explaining the intent.
|
Fixed benchmark failures and addressed Claude's follow-up observations: Benchmark fix (CI red → green): The fix(lint) commit (4ed709e) had renamed
Fixed in 667866e: updated expected-edges.json to match renamed functions; restored Claude's minor follow-up items:
|
| // Quick pre-filter: only re-parse files that actually contain prototype or | ||
| // function-as-object-property patterns to avoid an expensive WASM re-parse of | ||
| // every JS/TS file in large repos. Covers: | ||
| // - `.prototype.` — classical prototype method assignment | ||
| // - `\b\w+\.\w+\s*=\s*function` — function-as-object property (`f.g = function(){}`) | ||
| const protoFiles = jsFiles.filter((relPath) => { | ||
| try { | ||
| const content = readFileSafe(path.join(rootDir, relPath)); | ||
| return content.includes('.prototype.') || /\b\w+\.\w+\s*=\s*function/.test(content); | ||
| } catch { | ||
| return false; |
There was a problem hiding this comment.
Arrow-function property assignments excluded from native post-pass
The pre-filter regex \b\w+\.\w+\s*=\s*function matches fn.method = function(){} but not fn.method = () => {}. A file containing only arrow-function property assignments will be omitted from protoFiles, never WASM-reparsed, and its fn.method definitions will not be inserted by this post-pass. The benchmark this.js escapes this because f.h = function(){} is present alongside f.g = () => {}, but any file with exclusively arrow-function property assignments will reproduce the miss.
There was a problem hiding this comment.
Fixed in 8407fcf — extended the pre-filter regex to match arrow-function property assignments. The pattern now matches both fn.method = function(){} (traditional) and fn.method = () => {} / fn.method = param => {} (arrow) forms, while the negative lookahead still excludes .prototype. patterns handled natively by the Rust extractor.
…1339) * fix(native): add prototype method extraction to Rust engine (#1327) Implement parity with the WASM JS extractor for pre-ES6 prototype OOP patterns. Extractor (crates/codegraph-core/src/extractors/javascript.rs): - `Foo.prototype.bar = function(){}` → emits `Foo.bar` definition (kind: method) - `Foo.prototype.bar = identifier` → seeds typeMap['Foo.bar'] = identifier (confidence 0.9) - `Foo.prototype = { bar: fn, ... }` → same rules per property (pair, method_definition, shorthand_property_identifier) Built-in globals (Array, Object, …) are excluded via `is_js_builtin_global` guard. Adds 6 unit tests covering all three patterns plus edge cases. Edge builder (crates/codegraph-core/src/edge_builder.rs): - After a typeMap-resolved type lookup, check typeMap['TypeName.method'] for prototype aliases (`Foo.prototype.bar = identifierAlias`), mirroring the protoAlias fallback added to call-resolver.ts in the WASM path. - Inline new-expression receiver: extract class name from `(new Foo).bar()` receivers using string parsing (mirrors the `^\(?\s*new\s+[A-Z...]` regex in call-resolver.ts), enabling resolution without a named variable binding. Verified against the integration test in tests/integration/prototype-method-resolution.test.ts (all 3 tests pass with native engine). docs check acknowledged Closes #1327 * fix(native): fix parity divergence in extract_inline_new_type Use strip_prefix('(').unwrap_or(receiver) instead of trim_start_matches('(') to strip at most one leading paren, matching the JS regex ^\(?. Also update the doc comment to reflect that _ and $ prefixes are also accepted. * fix(native): strip one surrounding quote pair in prototype object-literal key `trim_matches` was stripping ALL quote chars (e.g. `"it's"` became `its`). Replace with strip_prefix + strip_suffix to remove exactly the outermost matching quote pair. * fix(extractor): remove duplicate extractPrototypeMethodsWalk calls Both extractSymbolsQuery and extractSymbolsWalk had a second call to extractPrototypeMethodsWalk appended at the bottom, causing prototype methods to be extracted twice. Remove the duplicate from each path. The duplication caused a ~44% WASM benchmark regression on the query path (used by wasm-worker-entry.js in benchmarks). * style: fix biome format violations inherited from base branch merge Long lines in wasm-worker-entry.ts, wasm-worker-pool.ts and two fixture files were not wrapped per the 100-char line width rule. * perf(native): remove .prototype. files from WASM post-pass filter The Rust engine now extracts `Foo.prototype.bar = fn` definitions natively (PR #1327). Remove the `.prototype.` text filter from the `runPostNativePrototypeMethods` pre-filter so those files are no longer WASM-reparsed on every native build. The function-as-object-property pattern (`fn.method = function(){}`) is still not handled by Rust and continues to use the WASM post-pass. This eliminates the 422% Build ms/file regression seen on CI. * fix(native): exclude prototype patterns from WASM post-pass pre-filter The regex /\b\w+\.\w+\s*=\s*function/ matched the substring 'prototype.bar = function' inside 'Foo.prototype.bar = function(){}', causing prototype files to be queued for WASM re-processing even though the Rust engine now handles those patterns natively. Added a negative lookahead to exclude the prototype shape, matching only function-as-object-property patterns like 'fn.method = function'. Fixes the duplicate-node risk flagged in Greptile review of #1339. * test(native): add unit tests for extract_inline_new_type edge cases Cover the string-parsing logic in extract_inline_new_type: (new Foo), (new Foo('arg')), no-parens form, _ and $ prefixes, lowercase rejection, plain identifier, and the newFoo-not-a-keyword case. * fix(bench): sync JS fixture names and exclude benchmark fixtures from biome lint (#1339) Commit 4ed709e's biome auto-fix renamed defProp/defProps/create to _defProp/_defProps/_create (unused-variable prefix), but the expected-edges.json manifest still referenced the old names. This caused 5 false positives and 5 false negatives in the JS benchmark, dropping precision to 84.4% (below the 100% threshold) and recall to 81.8% (below 90%). Also fixes the class-inheritance DoubleCounter fixture: the code used Counter.count() (a direct static call) but the manifest expected a class-inheritance edge via super.count(). Changed to super.count() so the fixture tests what the manifest documents. Prevent recurrence by adding a biome.json override that disables lint for tests/benchmarks/resolution/fixtures/** — fixture files are hand-written sample code that must use specific patterns (including apparently-unused functions and super calls) to exercise resolution.
…s identifier
- Merge origin/main into feat/prototype-resolver-1317
- Both post-passes are retained: runPostNativePrototypeMethods (func-prop
this-dispatch for fn.method = function(){} patterns) and runPostNativeThisDispatch
(this/super class-inheritance dispatch from PR #1337)
- Remove duplicate paramBindings field in SerializedExtractorOutput that was
introduced by the previous merge (TS2300 error blocking all CI jobs)
…perty assignments (#1331) The pre-filter regex only matched `fn.method = function(){}` patterns, silently skipping files where all func-prop assignments use arrow functions (`fn.method = () => {}`). Such files were never WASM-reparsed and their method definitions were not inserted by the post-pass. Extend the regex to match both traditional function expressions and arrow function expressions (both `() => {}` and `param => {}` forms).
| if (t === 'function_declaration' || t === 'function_expression' || t === 'arrow_function') { | ||
| const nameNode = node.childForFieldName('name'); | ||
| const funcName = nameNode?.text; | ||
| if (funcName) { |
There was a problem hiding this comment.
Arrow function rest-param bindings silently dropped
extractObjectRestParamBindingsWalk includes 'arrow_function' in its type guard but then calls childForFieldName('name') — a field that tree-sitter's JavaScript grammar never sets on arrow function nodes. funcName is always undefined for arrow functions, so the if (funcName) guard skips the entire parameter scan. Any pattern like const f3 = ({ ...rest }) => { rest.method(); } produces zero ObjectRestParamBinding entries, and the call rest.method() is silently unresolved. The fix is the same approach used in extractSpreadForOfWalk: detect an enclosing variable_declarator (one level up the tree) and take its name child as the function name — or remove arrow_function from the type check to match the actual scope.
| for (const [relPath, symbols] of wasmResults) { | ||
| const fileNodeRow = fileNodeStmt.get(relPath) as { id: number } | undefined; | ||
| if (!fileNodeRow) continue; | ||
|
|
||
| const typeMap = symbols.typeMap instanceof Map ? symbols.typeMap : new Map(); | ||
|
|
||
| for (const call of symbols.calls ?? []) { | ||
| if (!call.receiver) continue; // receiver calls only | ||
|
|
||
| const caller = findCaller(lookup, call, symbols.definitions ?? [], relPath, fileNodeRow); | ||
|
|
||
| const targets = resolveByMethodOrGlobal( | ||
| lookup, | ||
| call, | ||
| relPath, | ||
| typeMap as Map<string, unknown>, | ||
| caller.callerName, | ||
| ); | ||
|
|
||
| for (const t of targets) { | ||
| // Only emit edges to newly-inserted func-prop nodes to avoid | ||
| // duplicating edges the Rust engine already built. | ||
| if (!newNodeIds.has(t.id)) continue; | ||
| const key = `${caller.id}|${t.id}`; | ||
| if (seenByPair.has(key)) continue; | ||
| seenByPair.add(key); | ||
| const conf = computeConfidence(relPath, t.file, null); | ||
| if (conf <= 0) continue; | ||
| newEdgeRows.push([caller.id, t.id, 'calls', conf, 0, 'receiver-typed']); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (newEdgeRows.length > 0) { | ||
| db.transaction(() => batchInsertEdges(db, newEdgeRows))(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Cross-file func-prop call edges silently dropped
The comment at the top of the loop says "Resolve call edges in every file — not just those that define new func-prop methods. A caller in app.js calling a method defined in lib.js would be silently missed if we only scanned definition files." But the implementation contradicts this: the loop iterates over wasmResults, which was populated from protoFiles — a pre-filtered set of only those files that match the func-prop regex. A file that calls f.method() without also defining any \w+.\w+ = function patterns is absent from wasmResults and its calls are never scanned.
Concretely: if lib.js defines f.method = function(){} and app.js only calls f.method(), after this pass f.method is inserted as a new node but no edge from app.js is ever emitted. To honour the stated intent, either extend protoFiles to include all JS/TS files, or query the DB for calls with matching receiver/name after the node-insertion phase.
… extractSpreadForOfWalk (#1359) * test(extractor): verify exported arrow function funcStack tracking in extractSpreadForOfWalk (#1354) Add regression tests confirming that `export const f = (arr) => { for (const x of arr) x(); }` correctly pushes `f` onto the funcStack so for-of bindings record the right enclosing caller. The recursive walk visits `variable_declarator` regardless of whether it is nested under a plain `lexical_declaration` or an `export_statement`, so the gap reported in the PR #1331 review was already closed by commit a6c5d2d. These tests document and gate that behavior. Closes #1354 * fix: remove duplicate paramBindings in SerializedExtractorOutput and rename process test identifier The merge at 3c164f2 introduced a second `paramBindings` field (using the top-level ParamBinding import) alongside the existing inline-import form at line 68, causing TS2300 duplicate-identifier errors that broke every CI job. Removed the duplicate and the now-unused ParamBinding top-level import. Also renamed the `process` arrow-function identifier in the Phase 8.3e test to `handleItems` — `process` is a Node.js global and its presence in the test obscures that the test is solely about the export-wrapping code path.
Summary
Four call resolution improvements:
1. Prototype-based method calls —
Foo.prototype.bar = fn(Closes #1317, Closes #1313)javascript.ts):extractPrototypeMethodsWalkhandles three patterns:Foo.prototype.bar = function(){}→ emitsFoo.baras amethoddefinitionFoo.prototype.bar = f→ seedstypeMap['Foo.bar'] = { type: 'f' }Foo.prototype = { bar: fn }→ same rules per propertyresolveByMethodOrGlobalfor prototype aliases and inline(new Foo)receiversShape → Circle → Ellipse) fully expand2. Array spread and iteration callbacks — Phase 8.3e (Closes #1321)
f(...arr),for (const x of arr) x(),Array.from(arr, cb),new Set(arr)spread0→100%,more10→100% recall on Jelly micro-tests3. Function-as-object property
this-dispatch (Closes #1334)javascript.ts):extractFuncPropMethodsWalk+handleFuncPropAssignmentextractfn.method = function() {}andfn.method = () => {}assignments asmethoddefinitions; extended indispatchQueryMatchfor the query-based pathcallerName-basedthis-dispatch inresolveByMethodOrGlobalalready resolvesthis.other()insidefn.methodonce the definition is registered — no resolver changes neededfn.prop = functionpatternsthisfixture 0→100% recall (f.h → f.g)4. Object destructuring rest parameter dispatch — Phase 8.3f (Closes #1336)
When a function parameter uses object destructuring with a rest element (
function f3({ e1: eee1, ...eerest })), and the rest object's property is called (eerest.e4()), resolve the callee via a three-hop chain:javascript.ts):extractObjectRestParamBindingsWalkrecords rest bindings in object-destructured params;extractObjectPropBindingsWalkrecords shorthand/identifier object-literal propertiespoints-to.ts): Phase 8.3f seedspts["rest.propName"] = {"fn"}from the three-hop chainrest.prop()via the seeded pts keyrestfixture 0→100% recall (f3 → e4)spreadmore1thisrestTest plan
npm run build— clean TypeScript compiletests/parsers/javascript.test.ts— extraction offn.method = function/arrowdefinitionstests/integration/func-prop-this-dispatch.test.ts—f.h → f.gedge resolved on both enginestests/benchmarks/resolution/jelly-micro.test.ts— spread=100%, more1=100%, this=100%, rest=100%