feat(dynamic-calls): RES-2 — closed dispatch table resolution ({a:fnA,b:fnB}[k]())#1677
Merged
Conversation
…ix hasActiveFileSiblings parity The Rust orchestrator classifies roles before JS post-passes and does not implement the hasActiveFileSiblings heuristic. This causes functions with fan_in=0 and fan_out>0 (e.g. main, square in sample-project) to be classified as dead-unresolved by Rust but leaf by the JS classifier. Previously, JS classifyNodeRoles only ran when CHA or this-dispatch post-passes added new edges. For projects with no inheritance (like sample-project), those passes add zero edges, so Rust's stale roles were never overridden. Fix: always run a full JS classifyNodeRoles(db, null) after native full builds so the JS classifier's hasActiveFileSiblings heuristic takes effect. Incremental builds keep the existing scoped re-classification for post-pass edges only, since the heuristic gap was corrected on the preceding full build. Closes #1659
const { Calculator } = require('./utils') creates a kind='function' shadow
node for Calculator in the importing file. The JS resolveReceiverEdge logic
uses importedNames.has(effectiveReceiver) to detect import artifacts — but
CJS require bindings were not in importedNames (only ES module imports were),
so the shadow node appeared to be a locally-defined symbol and blocked the
global class lookup. Result: no receiver edge from main to Calculator even
though calc.compute() is a typed receiver call.
Fix:
- Add cjsRequireBindings field to ExtractorOutput + SerializedExtractorOutput
to carry const { X } = require('…') name→source mappings from the extractor
without adding them to symbols.imports (which would create spurious DB edges)
- extractDestructuredBindingsWalk and handleVariableDecl now populate
cjsRequireBindings when require() is the RHS of a destructured const
- buildImportArtifactNames merges importedNames with cjsRequireBindings into
a combined map passed exclusively to resolveReceiverEdge
- buildFileCallEdges receives importArtifactNames as a separate param so CJS
names affect only receiver-edge resolution, not call-target resolution or roles
Closes #1661
…etection ObjC (objc.ts + objc.rs): - [obj performSelector:@selector(greet)] → unresolved-dynamic sink edge - [obj performSelector:@selector(greet) withObject:arg] → unresolved-dynamic - objc_msgSend(obj, sel, ...) → unresolved-dynamic sink edge Dart (dart.ts + dart.rs): - Function.apply(fn, positional, named) → unresolved-dynamic sink edge Handles layout B (method selector and argument_part in adjacent selector nodes) by scanning sibling selectors for the method name. Tests added to both Rust (#[test]) and TS (vitest) for each new pattern. Closes #1664
…,b:fnB}[k]()) When a subscript call's object is an inline object literal (or parenthesized object literal), extract all identifier values as arrayElemBindings under a synthetic table name <dt_L_C>, and emit a call with name='<dt_L_C>[*]' and dynamicKind='dispatch-table'. The existing pts wildcard solver then resolves that call to each value at confidence=baseline-0.1 (PROPAGATION_HOP_PENALTY). 'dispatch-table' is intentionally excluded from FLAG_ONLY_KINDS so no sink edge is emitted when pts resolves the targets. Changes: - types.ts: add 'dispatch-table' to DynamicKind union - javascript.ts: extractSubscriptCallInfo accepts optional arrayElemBindings; detects inline object (and parenthesized object) as dispatch table, seeds bindings, returns wildcard call; extractCallInfoWithBindings helper threads bindings through; dispatchQueryMatch and handleCallExpr both use it - dispatch-table.js fixture + expected-edges.json entries for pts-javascript - points-to.test.ts: 3 new unit tests for dispatch-table pts constraints Closes #1665
Contributor
Codegraph Impact Analysis13 functions changed → 20 callers affected across 8 files
|
Contributor
Greptile SummaryThis PR implements RES-2 of the dynamic-call resolution roadmap: detecting inline object-literal dispatch tables ( Confidence Score: 5/5Safe to merge; dispatch-table detection and CJS parity fix are well-tested and logically contained. The dispatch-table extraction logic is correct and narrowly scoped. The CJS parity fix mirrors a well-understood WASM path. Test coverage is thorough. The misplaced JSDoc in src/features/dataflow.ts and the unexplained ADR deletions are worth a quick author confirmation before merge. Important Files Changed
Reviews (8): Last reviewed commit: "fix(native): use empty file for CJS impo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+2039
to
+2042
| debug( | ||
| scopedFiles | ||
| ? `Post-pass role re-classification complete (${scopedFiles.length} file(s))` | ||
| : 'Post-pass role re-classification complete (full graph — null-file endpoints)', | ||
| : 'Post-pass role re-classification complete (full graph)', |
Contributor
There was a problem hiding this comment.
The previous log message 'full graph — null-file endpoints' distinguished the emergency fallback (null-file nodes) from a normal scoped run. The replacement 'full graph' is now used for both the new full-build reclassification and the old null-file-endpoint fallback, making it impossible to tell from logs alone which case triggered the full-graph pass. Consider adding a qualifier like 'full graph (full build)' vs. 'full graph (null-file endpoints)' to preserve that signal.
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!
Contributor
Author
There was a problem hiding this comment.
Fixed — the log message now distinguishes between the two full-graph paths: 'full graph (full build)' when needsFullReclassify is true (i.e., result.isFullBuild), and 'full graph (null-file endpoints)' when the scoped path fell back due to empty affectedFiles.
…rs (#1677) docs check acknowledged
carlos-alm
force-pushed
the
fix/issue-1665
branch
from
71583e9 to
b385e3b
Compare
…1677) docs check acknowledged
Contributor
Author
docs check acknowledged
…solution (#1678) const { X } = require('…') creates a kind='function' shadow node in the importing file. The Rust emit_receiver_edge checks is_local_definition via imported_names.contains_key(name) — but CJS require bindings were not added to symbols.imports, so the name appeared as a local definition and blocked the cross-file class fallback. Fix: when the RHS of a destructured const is a require() call, also push the names into symbols.imports so collect_imported_names_for_file includes them in the imported_names map used by the receiver-edge resolver. Mirrors the WASM cjsRequireBindings fix (commit 066d7d0, issue #1661). Closes #1678
…thout spurious import edges The previous fix (1d4e2d8) added CJS require names to symbols.imports which caused the pipeline to create 3 extra DB import edges (for each destructured name), making native produce 42 edges vs WASM's 39. Proper fix: mark CJS require imports with cjs_require=Some(true) and skip them in the import-resolution batch that feeds DB import edge creation. collect_imported_names_for_file still processes them, so imported_names.contains_key('Calculator') returns true and the is_local_definition check in emit_receiver_edge correctly falls back to the cross-file class. Changes: - types.rs: add cjs_require: Option<bool> to Import struct - javascript.rs: set cjs_require=Some(true) on CJS require imports - pipeline.rs: skip cjs_require imports in import-resolution batch Closes #1678
…t spurious edges The previous fix filtered batch_inputs in pipeline.rs but build_import_edges iterates ctx.file_symbols[file].imports directly, bypassing that filter. Add the skip at the emission site in import_edges.rs so CJS require bindings never produce DB import edges.
…rrect call targets Previously CJS require entries were filtered from batch_inputs, so imported_names had empty file paths. This caused resolve_call_targets to find shadow function nodes (in index.js) instead of the real definitions (in utils.js), resulting in wrong call edges and wrong roles (sumOfSquares: dead-unresolved instead of core). Allow CJS entries through path resolution to get correct target files. DB import edge creation is still blocked via the cjs_require flag in build_import_edges — correct separation of concerns.
…-resolution behavior CJS require bindings in imported_names now use an empty target_file. This makes resolve_call_targets' import-aware lookup fail for CJS names (file=""), falling through to same-file shadow nodes — exactly matching WASM's behavior where CJS names are absent from importedNamesMap. Without this, step 3's real-file paths routed calls to real nodes (math.js) instead of shadows (index.js), causing shadow add/sumOfSquares/square to have fan_in=0 → dead-unresolved instead of core. The CJS entries still flow through batch_inputs so propagate_return_types and other pipeline functions work correctly — which is what fixed main/square/sumOfSquares roles in step 3.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
'dispatch-table'toDynamicKind— excluded fromFLAG_ONLY_KINDSso the call is resolved via pts rather than sunkextractSubscriptCallInfoto detect inline object literals (including parenthesized({a:fn})[k]()) as dispatch tables: seedsarrayElemBindingswith all identifier values under a synthetic<dt_L_C>name and returns a wildcard call<dt_L_C>[*]; both query and walk extractor paths use the newextractCallInfoWithBindingshelperarr[*]wildcard solver resolves the call to each value atconfidence - PROPAGATION_HOP_PENALTY(0.1), matching the CHA-style over-approximation described in the issueTest plan
tests/unit/points-to.test.ts— 3 new describe blocks cover dispatch-table pts wildcard seeding (26 tests total, all pass)tests/parsers/javascript.test.ts— 141 tests pass (no regressions)tests/engines/query-walk-parity.test.ts— 14 tests pass (both extraction paths produce identical results)tests/unit/builder.test.ts,call-resolver.test.ts,constants.test.ts— all passdispatch-table.jsfixture added topts-javascript;expected-edges.jsonextended with 2 edges (runDispatch → dtFn1,runDispatch → dtFn2)runDispatchtodtFn1anddtFn2Closes #1665