feat(resolver): resolve .call()/.apply() this-rebinding and add fun fixture (JS)#1405
Conversation
The test was using auto engine (native-preferred), causing it to pick the published npm native binary which predates the prototype-method fixes. WASM correctly extracts Dog.prototype.bark and resolves all call edges. Fixes #1381
…lookup When a method is called without a receiver inside a class-qualified method (e.g. `IsValidEmail()` inside `Validators.ValidateUser`), both the WASM and native engines now try the class-qualified name as a fallback. Root cause: the same-class method lookup in `resolveByMethodOrGlobal` was gated on `call.receiver && callerName`, which excluded no-receiver calls. Static sibling calls in C#/Java (e.g. `IsValidEmail()` inside a static class) have no receiver — the guard prevented the `Validators.IsValidEmail` lookup. Fixes: - WASM (call-resolver.ts): `if (call.receiver && callerName)` → `if (callerName)` - Native (edge_builder.rs): moves class-scoped exact lookup outside the `call.receiver.is_some()` guard; suffix scan remains gated on receiver-present to avoid false positives on global function calls inside class methods. Also fixes a latent CHA re-classification bug exposed by this change: the Rust orchestrator classifies roles before the CHA post-pass, so the global fan-out median was computed from pre-CHA edges. After CHA added edges, the median shifted but Validators.cs (not directly connected to CHA-affected files) was excluded from the incremental re-classification, leaving stale roles. Fixed by switching the post-CHA re-classification from incremental to full. C# same-file recall: 0/2 → 2/2 (100%). Overall C# recall: 73.9% → 82.6% (19/23 expected edges). Remaining gap: receiver-typed (0/4) tracked in #1402.
…ixture (JS) - add fun Jelly micro-test fixture (fun.js + expected-edges.json) with correct <root> source names; scores 4/4 named edges (baz/baz2/baz3/baz4 → bar) - add ThisCallBinding type: extracts fn.call(namedCtx,...)/fn.apply(namedCtx,...) and seeds fn::this → namedCtx in the PTS map; when this() is called inside fn, the scoped key lookup resolves to namedCtx - add extractThisCallsWalk: captures this(args) call expressions (where this is the callee, not a receiver) so they participate in PTS resolution - fix extractCallbackReferenceCalls: skip identifier args for .call()/.apply()/.bind() invocations; those are the this-context and function arguments flowing into the delegated function, not callbacks for the current scope (was producing FPs) - add buildThisCallBindingsPtsPostPass: native-engine post-pass that resolves this() calls the same way as WASM, mirroring buildFnRefBindingsPtsPostPass - update JS benchmark fixture: add invoker.call(handler, 10) test case and two new expected edges (runCallThis→invoker dynamic, invoker→handler points-to); JS precision holds at 100%, total expected now 37 Closes #1380
Greptile SummaryThis PR adds
Confidence Score: 5/5Safe to merge — the new this-rebinding logic is gated behind a All changed code paths produce correct call-graph edges. The interaction between The dual-maintenance extraction code in Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["fn.call(namedCtx, ...)"] --> B{Code path}
B -->|Query path| C["extractThisCallAndBindingsWalk\n→ thisCallBindings[]"]
B -->|Walk path| D["handleCallExpr\n→ ctx.thisCallBindings[]"]
C --> E["buildPointsToMapForFile\nInjects fn::this → namedCtx\ninto PTS map"]
D --> E
E --> F["buildParamFlowPtsPostPass\n(runs first)\nResolves invoker::this\nif file has paramBindings"]
E --> G["buildThisCallBindingsPtsPostPass\n(runs after)\nResolves this() calls\nvia fn::this PTS key"]
F -->|seenByPair dedup| H["Edge: invoker → handler"]
G -->|seenByPair dedup| H
style F fill:#ffe0b2
style G fill:#c8e6c9
style H fill:#bbdefb
Reviews (9): Last reviewed commit: "fix: resolve merge conflicts with main" | Re-trigger Greptile |
Codegraph Impact Analysis15 functions changed → 18 callers affected across 8 files
|
|
Addressed Greptile's review feedback: Comment 1 — Broad skip in Comment 2 — No-receiver class-scoped lookup JS false-positive risk (lines 185-194): The JS benchmark maintains 100% precision after the receiver-guard removal. A dedicated fixture to make the regression boundary explicit has been filed as #1407. Comment 3 — Full role re-classification performance (lines 282-283): Added a comment to |
…duce O(n) passes In extractSymbolsQuery, replace two separate extractThisCallsWalk + extractThisCallBindingsWalk full-tree traversals with a single combined extractThisCallAndBindingsWalk pass, halving the per-file traversal cost for the query (WASM) path. In extractSymbolsWalk, inline the this() call detection and .call()/.apply() binding extraction into handleCallExpr, which is already called during the main walkJavaScriptNode traversal, eliminating two extra O(n) passes from the walk path entirely.
|
Pushed a perf optimization (commit e502d39) to address the benchmark regression caught by the regression guard: Pre-publish benchmark gate failure: The original implementation added two separate full-AST recursive traversals ( Fix:
Behavioral output is identical — verified by 104 JS parser tests + cross-engine parity tests all passing. |
|
Merged origin/main to resolve conflicts — only change was removing a stale TODO comment from |
|
Merged origin/main to resolve conflicts with 5 commits that landed on main since the previous merge:
Conflict resolutions:
|
1 participant
Summary
funfixture (fun.js+expected-edges.json) with corrected<root>source names for module-level edges; the 4 named edges (baz/baz2/baz3/baz4 → bar) score 100% recall.call()/.apply()this-rebinding: whenfn.call(namedCtx, ...)is seen, seedsfn::this → namedCtxin the PTS map so thatthis()calls insidefnresolve tonamedCtxextractCallbackReferenceCalls: identifier arguments to.call()/.apply()/.bind()were incorrectly emitted as dynamic callback calls; now they are skipped since this-context and delegated function arguments should not be attributed to the outer scopebuildThisCallBindingsPtsPostPassso the native engine benefits from this-rebinding via the same JS post-pass pattern used forfnRefBindingsandparamBindingsTest plan
npx vitest run tests/benchmarks/resolution/resolution-benchmark.test.ts— JS benchmark 100% precision, all 171 tests passnpx vitest run tests/benchmarks/resolution/jelly-micro.test.ts—funscores recall=100% (4/4), all 10 tests passnpx vitest run tests/parsers/javascript.test.ts tests/unit/points-to.test.ts tests/engines/parity.test.ts— passesnpm run lint— no errorsCloses #1380