fix(native): resolve this-dispatch in func-prop methods (#1512) - #1536
Conversation
Sorry, something went wrong.
…ds edges exist
runPostNativeThisDispatch had two bugs that prevented it from resolving
`this.g()` inside `f.h = function() {}` (func-prop methods) on the native path:
1. Guard required at least one `extends` edge — func-prop this-dispatch
(`f.h = function(){ this.g() }`) works by treating the dot-prefix of the
caller name (`f` from `f.h`) as the class prefix; no class inheritance needed.
2. Full-build file selection only unioned files from `extends` edges — files
containing only func-prop method nodes were never re-parsed for this-dispatch.
3. Empty `parents` map caused early return — wrong when func-prop methods exist
without any class hierarchy (`resolveThisDispatch` handles empty parents by
doing direct class-prefix lookup on the first loop iteration).
Fix: expand the guard to also fire when dot-named `method` nodes exist in the DB;
skip the `parents` query entirely when no `extends` edges exist (avoiding the
unnecessary DB round-trip); remove the empty-parents bail-out; add func-prop
method nodes to the full-build file selection query.
Also adds `this-dispatch-func-prop` to TECHNIQUE_MAP in the resolution benchmark
so the mode is correctly bucketed as `cha-rta` if it appears in JS fixture edges.
docs check acknowledged
Closes #1512
Greptile SummaryThis PR fixes three bugs in runPostNativeThisDispatch that prevented this.g() calls inside function-property methods (e.g. f.h = function() { this.g() }) from being resolved on the native engine path. Confidence Score: 5/5Safe to merge — changes are narrowly scoped to removing two incorrect early-returns and adding one UNION arm; existing class-inheritance behaviour is unchanged. Both changed files are surgical: the orchestrator patch removes guards that were provably wrong for the func-prop case and adds an additive UNION arm, and the benchmark change is a single map entry. No data-path regressions are introduced on repos that have no dot-named method nodes. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[runPostNativeThisDispatch] --> B{hasExtends?}
B -- yes --> D[Build parentRows via extends JOIN]
B -- no --> C{hasFuncPropMethod?}
C -- no --> Z[Early return]
C -- yes --> E[parentRows = empty array]
D --> F[Build parents Map]
E --> F
F --> G{isFullBuild?}
G -- yes --> H[UNION: extends src + extends tgt + dot-named methods]
G -- no --> I[relFiles = changedFiles]
H --> J[Re-parse relFiles via WASM]
I --> J
J --> K[resolveThisDispatch per call site]
K --> L{call type?}
L -- class extends --> M[Traverse parents chain]
L -- func-prop --> N[Extract prefix, lookup sibling directly]
M --> O[Insert calls edge]
N --> O
Reviews (2): Last reviewed commit: "fix(native): resolve this-dispatch in fu..." | Re-trigger Greptile |
Sorry, something went wrong.
| UNION | ||
| SELECT file FROM nodes | ||
| WHERE kind = 'method' AND INSTR(name, '.') > 0 AND file IS NOT NULL |
There was a problem hiding this comment.
The new UNION arm uses INSTR(name, '.') > 0 without an index-friendly prefix filter, so on a full build this scans every method node in the DB. In large repos with many dot-named non-func-prop nodes (e.g. module.exports.foo, prototype assignments, namespaced helpers) this could pull in substantially more files than the pure extends-hierarchy arms. Consider adding an index on (kind, name) or switching to a LIKE '%.%' pattern (which SQLite can leverage with a partial index). At minimum, the comment could note that the scan is bounded by kind = 'method'.
| UNION | |
| SELECT file FROM nodes | |
| WHERE kind = 'method' AND INSTR(name, '.') > 0 AND file IS NOT NULL | |
| UNION | |
| SELECT file FROM nodes | |
| WHERE kind = 'method' AND name LIKE '%.%' AND file IS NOT NULL |
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!
Sorry, something went wrong.
Codegraph Impact Analysis1 functions changed → 4 callers affected across 4 files
|
Sorry, something went wrong.
|
Acknowledged — the INSTR(name, '.') > 0 arm could include class-method files without extends edges in very large repos, partially diluting the re-parse optimization. This is a performance concern (not a correctness issue) — the DISTINCT and THIS_DISPATCH_EXTS filter still bound the set, and real-world impact depends on the ratio of class-only vs func-prop files. Tracked in #1550 for a follow-up that narrows the arm (e.g. via NOT EXISTS guard) after profiling on a large repo confirms the actual overhead. |
Sorry, something went wrong.
Sorry, something went wrong.
Summary
Root Cause
runPostNativeThisDispatch was designed around class inheritance (extends edges) and never accounted for the func-prop pattern. resolveThisDispatch already handles it correctly: for caller f.h, it extracts class prefix f, then looks up f.g via lookup.byName() — no parents traversal needed. The post-pass just never gave it the right files or the chance to run.
Fix (src/domain/graph/builder/stages/native-orchestrator.ts)
Also adds this-dispatch-func-prop to TECHNIQUE_MAP in resolution-benchmark.test.ts so the mode is correctly bucketed as cha-rta.
Test plan
Closes #1512