feat(dynamic-calls): phase 2 — JVM dynamic dispatch (Java/Kotlin/Scala/Groovy) - #1646
feat(dynamic-calls): phase 2 — JVM dynamic dispatch (Java/Kotlin/Scala/Groovy)#1646carlos-alm merged 12 commits into
Conversation
Sorry, something went wrong.
Before this change, any call that couldn't be statically resolved (eval, obj[key](), .call/.apply/.bind, computed subscripts) was silently dropped from the graph — a confidence-0 blind spot with no user-visible signal. This introduces a DynamicKind taxonomy (computed-literal, computed-key, reflection, eval, unresolved-dynamic) that classifies JS call sites at extraction time. Flag-only kinds (eval, computed-key, unresolved-dynamic) now emit sink edges at confidence=0.0 (caller → file node, invisible to normal queries but queryable via `codegraph roles --dynamic`) instead of being dropped. Key changes: - types.ts: DynamicKind union + Call.dynamicKind/keyExpr + EdgeRow.dynamic_kind - migrations v20: edges.dynamic_kind column + index - extractors/javascript.ts: classify eval/new Function/computed-key/reflection - call-resolver.ts: short-circuit <dynamic:*> names to prevent spurious lookups - build-edges.ts: sink-edge emission for FLAG_ONLY_KINDS, dynamic_kind back-fill - roles CLI: --dynamic flag shows flagged call counts by kind - Rust mirror: CallInfo/ComputedEdge structs updated, short-circuit + sink-edge - Tests: 7-test FFI round-trip, dynamic-javascript benchmark (100% precision/recall) README docs update (README:919) is deferred to the Docs/Parity PR. docs check acknowledged
… path
The WASM worker uses the query-based extractor path (extractSymbolsQuery)
when a TreeSitter query is available, bypassing extractSymbolsWalk.
The callfn_node branch (plain identifier calls) was pushing
{ name: fn.text } directly without going through extractCallInfo, so
eval() detection was never triggered. Similarly, newfn_node was pushing
{ name: 'Function' } for new Function() without flagging it.
Fix: route callfn_node through extractCallInfo (same as callmem_node and
callsub_node) and add explicit new Function() detection in newfn_node.
Result: eval:2 computed-key:1 flagged in the dynamic-javascript benchmark
(up from computed-key:1 only).
docs check acknowledged
…k filter - Rust tests: add dynamic_kind/key_expr fields to two CallInfo literals that were missing them (caused build failure on all platforms) - Rust sink-edge dedup: replace unsafe u64 bit-pack with a typed (caller_id, file_node_id, kind_byte) tuple set; the old approach corrupted bits 56-63 for caller IDs >= 16 M - TS back-fill UPDATE: add dynamic_kind to WHERE clause so two sink edges from the same caller to the same file with different kinds are updated independently - Benchmark extractResolvedEdges: tighten the confidence filter to exclude only intentional sink edges (confidence=0 AND dynamic_kind IS NOT NULL) instead of all confidence=0 edges globally
…ator detection Extends the dynamic-call taxonomy to JS/TS-specific reflection idioms: - Reflect.apply(fn, ctx, args) / Reflect.call(fn, ...) — extracts fn as the actual callee with dynamicKind='reflection'. Works for both identifier and member-expression targets. - Reflect.construct(Target, args) — same pattern, extracts Target. - Reflect.get(target, 'prop') — string key → computed-literal kind (resolvable); identifier key → computed-key kind (flagged sink edge). - @foo / @Foo.bar TypeScript decorators — bare identifier and member-expression decorators emit reflection-kind calls. @foo() call-expression decorators are already handled by the existing call_expression walk. Both the walk path (walkJavaScriptNode/handleDecorator) and the query path (dispatchQueryMatch/runCollectorWalk) are updated. Rust extractor mirrored. Also fixes Phase 0 parity gaps in the Rust extractor: - eval() detection (was only in TS/WASM query path; Rust now emits <dynamic:eval>) - new Function() detection (Rust now emits <dynamic:eval>) - obj["method"]() and obj[key]() now set dynamic_kind in Rust (computed-literal and computed-key respectively) matching the TS extractor New fixture: tests/benchmarks/resolution/fixtures/dynamic-typescript/ - reflect.ts: Reflect.apply/construct/get patterns - decorator.ts: @log bare decorator patterns - expected-edges.json: 3 edges (100% P/R), 1 flagged (computed-key sink) New tests: 6 additional FFI tests for Reflect and decorator patterns (14 total) docs check acknowledged
…otlin/Scala/Groovy)
Adds detection of reflection and dynamic dispatch patterns in the JVM language family.
Both TS/WASM extractors and Rust mirrors updated.
Java: Method.invoke → unresolved-dynamic; getMethod("name")/getDeclaredMethod → reflection
with keyExpr; Class.forName → unresolved-dynamic.
Kotlin: ::greet callable ref → reflection, resolves to greet() (100% recall);
fn.invoke() → unresolved-dynamic sink edge.
Scala: method.invoke → unresolved-dynamic; getMethod("name") → reflection.
Groovy: obj.invokeMethod("name", args) → reflection if literal; obj."${dyn}"() → unresolved-dynamic.
Fixtures: dynamic-java (0% recall, qualified names), dynamic-kotlin (100% P/R),
dynamic-scala (0% recall), dynamic-groovy (0% recall).
docs check acknowledged
Impact: 37 functions changed, 16 affected
Greptile SummaryThis PR adds JVM dynamic-dispatch detection (Phase 2) across four languages — Java, Kotlin, Scala, and Groovy — in both the TypeScript WASM extractors and their Rust native mirrors. Each extractor gains handlers for reflection patterns (Method.invoke, getMethod/getDeclaredMethod, Class.forName, Kotlin callable refs ::fn, Groovy invokeMethod/GString method names) that emit typed call edges with dynamicKind set appropriately.
Confidence Score: 4/5Mostly safe — the Java, Scala, and Groovy paths are well-guarded; the one real defect is in the Kotlin extractor where bare invoke() with no receiver is flagged as dynamic dispatch, producing false sink edges for the common operator fun invoke() pattern. The Java extractor had its receiver-guard fix correctly backported from the previous review. The Kotlin navigation_expression branch (which handles fn.invoke()) is properly guarded. However, the simple_identifier branch flags any bare invoke() call — no receiver required — as unresolved-dynamic. Inside a Kotlin class that defines operator fun invoke(), writing invoke() is a fully resolvable self-call that incorrectly becomes a sink edge, inflating codegraph roles --dynamic with noise. The identical root cause was fixed for Java in an earlier round; the Kotlin path was added without the same guard. src/extractors/kotlin.ts (line 278) and the parallel crates/codegraph-core/src/extractors/kotlin.rs (line 347) — both need the bare invoke check either removed or given a receiver guard to match the treatment applied to the Java extractor. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[AST node visit] --> B{Node type?}
B -->|callable_reference| C[handleKotlinCallableRef]
C --> C1[Emit reflection edge\nname = member after ::\nreceiver = qualifier]
B -->|call_expression / simple_identifier| D{name == 'invoke'?}
D -->|Yes - no receiver| D1[⚠️ Emit unresolved-dynamic\nno receiver guard]
D -->|No| D2[Emit normal call edge]
B -->|call_expression / navigation_expression| E{method name?}
E -->|invoke - has receiver| E1[Emit unresolved-dynamic sink edge]
E -->|other| E2[Emit normal call edge]
B -->|method_invocation Java/Scala| F{method name?}
F -->|invoke + receiver present| F1[Emit unresolved-dynamic]
F -->|getMethod / getDeclaredMethod| G{string literal arg?}
G -->|Yes| G1[Emit reflection edge\nkeyExpr = literal]
G -->|No| G2[Emit computed-key edge]
F -->|forName + receiver == Class| H[Emit unresolved-dynamic\nkeyExpr = class name]
F -->|other| F2[Emit normal call edge]
B -->|method_invocation Groovy| I{method name?}
I -->|invokeMethod| J{string literal arg?}
J -->|Yes| J1[Emit reflection edge]
J -->|No| J2[Emit computed-key edge]
I -->|field is gstring| I1[Emit unresolved-dynamic]
I -->|other| I2[Emit normal call edge]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[AST node visit] --> B{Node type?}
B -->|callable_reference| C[handleKotlinCallableRef]
C --> C1[Emit reflection edge\nname = member after ::\nreceiver = qualifier]
B -->|call_expression / simple_identifier| D{name == 'invoke'?}
D -->|Yes - no receiver| D1[⚠️ Emit unresolved-dynamic\nno receiver guard]
D -->|No| D2[Emit normal call edge]
B -->|call_expression / navigation_expression| E{method name?}
E -->|invoke - has receiver| E1[Emit unresolved-dynamic sink edge]
E -->|other| E2[Emit normal call edge]
B -->|method_invocation Java/Scala| F{method name?}
F -->|invoke + receiver present| F1[Emit unresolved-dynamic]
F -->|getMethod / getDeclaredMethod| G{string literal arg?}
G -->|Yes| G1[Emit reflection edge\nkeyExpr = literal]
G -->|No| G2[Emit computed-key edge]
F -->|forName + receiver == Class| H[Emit unresolved-dynamic\nkeyExpr = class name]
F -->|other| F2[Emit normal call edge]
B -->|method_invocation Groovy| I{method name?}
I -->|invokeMethod| J{string literal arg?}
J -->|Yes| J1[Emit reflection edge]
J -->|No| J2[Emit computed-key edge]
I -->|field is gstring| I1[Emit unresolved-dynamic]
I -->|other| I2[Emit normal call edge]
Reviews (7): Last reviewed commit: "fix(dynamic-calls): add receiver guard t..." | Re-trigger Greptile |
Sorry, something went wrong.
| if (methodName === 'invoke') { | ||
| pushCall(ctx, node, '<dynamic:unresolved>', { | ||
| dynamic: true, | ||
| dynamicKind: 'unresolved-dynamic', | ||
| receiver, | ||
| }); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
invoke match is name-only — false positives on user-defined methods
Any method call named invoke is flagged as unresolved-dynamic, regardless of receiver type. In Java codebases this affects common patterns: executor services, command/strategy objects, Mockito's verify(mock).invoke(), Apache-Commons MethodInvoker.invoke(), and any domain method a developer chose to name invoke. These generate sink edges that inflate codegraph roles --dynamic with noise unrelated to reflection. The Java Reflection API specifically uses method.invoke(target, args) where method is a java.lang.reflect.Method object — without type information the extractor cannot distinguish that from task.invoke(). The same issue exists in src/extractors/scala.ts (line 177) and the corresponding Rust extractors. A minimal guard would be to require a non-null receiver (since Method.invoke is always invoked on an object), though type-aware resolution would be needed for full precision.
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed. Added a receiver non-null guard to the invoke check in both the WASM extractors (src/extractors/java.ts, src/extractors/scala.ts) and the native Rust extractors (crates/codegraph-core/src/extractors/java.rs, crates/codegraph-core/src/extractors/scala.rs). Bare invoke() calls with no receiver now fall through to the normal method invocation path.
Sorry, something went wrong.
| if (methodName === 'forName') { | ||
| const literal = getFirstStringArgJava(node); | ||
| pushCall(ctx, node, '<dynamic:unresolved>', { | ||
| dynamic: true, | ||
| dynamicKind: 'unresolved-dynamic', | ||
| keyExpr: literal ?? undefined, | ||
| receiver, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| // Normal method invocation | ||
| pushCall(ctx, node, methodName, receiver ? { receiver } : {}); |
There was a problem hiding this comment.
forName match is name-only — misclassifies unrelated methods
Any call to a method named forName is classified as dynamic class loading regardless of receiver type. User-defined or framework classes that expose a forName() factory method (e.g., Currency.forName("USD"), Enum.forName(...), generated DSL builders) would be incorrectly emitted as unresolved-dynamic sink edges. Class.forName() is always invoked on the class literal Class — checking that receiver === 'Class' would eliminate most false positives here.
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed. Added a receiver === 'Class' guard to the forName check in src/extractors/java.ts and crates/codegraph-core/src/extractors/java.rs. Calls like Currency.forName("USD") or Enum.forName(...) now fall through to normal method invocation instead of being flagged as unresolved-dynamic.
Sorry, something went wrong.
| if targets.is_empty() { | ||
| if let Some(ref dk) = call.dynamic_kind { | ||
| if dk == "eval" || dk == "computed-key" || dk == "unresolved-dynamic" { | ||
| let sink_key = (caller_id, fc.file_node_id, dk.as_bytes()[0]); | ||
| if !seen_sink_edges.contains(&sink_key) { | ||
| seen_sink_edges.insert(sink_key); | ||
| edges.push(ComputedEdge { | ||
| source_id: caller_id, |
There was a problem hiding this comment.
First-byte discriminator is fragile for future DynamicKind values
dk.as_bytes()[0] is used as the third component of the dedup key (caller_id, file_node_id, u8). The current three flag-only kinds start with different bytes (eval=0x65, computed-key=0x63, unresolved-dynamic=0x75), so there is no collision today. However, the key silently aliases any future kind that shares a leading byte with an existing one — e.g., adding eval-strict would collide with eval. Storing the full dynamic_kind string as the key (a HashSet<(u32, u32, String)>) would be unambiguous at negligible cost.
| if targets.is_empty() { | |
| if let Some(ref dk) = call.dynamic_kind { | |
| if dk == "eval" || dk == "computed-key" || dk == "unresolved-dynamic" { | |
| let sink_key = (caller_id, fc.file_node_id, dk.as_bytes()[0]); | |
| if !seen_sink_edges.contains(&sink_key) { | |
| seen_sink_edges.insert(sink_key); | |
| edges.push(ComputedEdge { | |
| source_id: caller_id, | |
| let mut seen_sink_edges: HashSet<(u32, u32, String)> = HashSet::new(); |
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.
There was a problem hiding this comment.
Fixed. Changed seen_sink_edges from HashSet<(u32, u32, u8)> to HashSet<(u32, u32, String)> in build_edges.rs. The dedup key now uses the full dynamic_kind string instead of the first byte, preventing silent aliasing for any future DynamicKind values that share a leading byte with an existing kind (e.g. eval-strict would have collided with eval).
Sorry, something went wrong.
… dedup key - `invoke` now only flagged as unresolved-dynamic when called on an object (receiver non-null), avoiding false positives on user-defined methods (executor services, strategy objects, Mockito stubs). Applied in both WASM (java.ts, scala.ts) and native (java.rs, scala.rs) extractors. - `forName` now only flagged when receiver === 'Class', eliminating false positives from user-defined forName() factory methods (Currency.forName, Enum.forName, etc.). Applied in both WASM and native Java extractors. - seen_sink_edges in build_edges.rs changed from HashSet<(u32, u32, u8)> to HashSet<(u32, u32, String)> to use the full dynamic_kind string as the dedup key, preventing silent aliasing for future DynamicKind values that share a leading byte with an existing kind. Impact: 5 functions changed, 22 affected
Sorry, something went wrong.
Impact: 24 functions changed, 158 affected
Codegraph Impact Analysis31 functions changed → 11 callers affected across 4 files
|
Sorry, something went wrong.
| // Back-fill dynamic_kind for flagged sink edges emitted by the native engine. | ||
| // Include dynamic_kind in the WHERE clause so two sink edges from the same caller | ||
| // to the same file node with different kinds don't clobber each other. | ||
| if (dynamicKindRows.length > 0) { | ||
| const stmt = db.prepare( | ||
| "UPDATE edges SET dynamic_kind = ? WHERE kind = 'calls' AND source_id = ? AND target_id = ? AND (dynamic_kind IS NULL OR dynamic_kind = ?)", | ||
| ); | ||
| for (const r of dynamicKindRows) stmt.run(r[6], r[0], r[1], r[6]); | ||
| } |
There was a problem hiding this comment.
New UPDATE block drops the dynamic = 1 guard — can corrupt normal call edges
Block 1 (lines 1665–1670) already covers unresolved sink edges with AND confidence = 0.0 AND dynamic = 1. Block 2 (added here) drops both guards. After Block 1 runs, Block 2 will also match any regular call edge with dynamic = 0 and dynamic_kind IS NULL that shares (source_id, target_id) with a dynamicKindRows entry — e.g., a caller that both directly calls and reflectively targets the same method ends up with dynamic_kind = 'reflection' on its direct-call edge. At minimum AND dynamic = 1 must be added; add AND confidence > 0 if the intent is to cover resolved dynamic edges that Block 1 misses.
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed. The second UPDATE block (the one without the guards) has been removed entirely. Block 1 with AND confidence = 0.0 AND dynamic = 1 is the only UPDATE now — it correctly scopes to sink edges only, preventing any spillover onto normal call edges that share the same (source_id, target_id) pair.
Sorry, something went wrong.
…ate dynamic_kind UPDATE
- groovy.rs: add gstring/template_string branch in the field_expression
fallback path of handle_call_expr, matching the WASM check in
handleGroovyCallExpr (groovy.ts). obj."\${dyn}"() calls on the native
path now emit unresolved-dynamic sink edges instead of falling through
as normal calls.
- build-edges.ts: remove the second dynamic_kind UPDATE block in
applyEdgeTechniquesAfterNativeInsert that was missing the
confidence = 0.0 AND dynamic = 1 scope guards. The duplicate block
could silently overwrite dynamic_kind on normal call edges whenever a
caller made both a reflection call and a regular call to the same
target. The first (correctly-scoped) block is sufficient.
Impact: 1 functions changed, 4 affected
|
Fixed both defects identified in the review summary:
|
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
… in Java and Scala extractors Bare getMethod() / getDeclaredMethod() calls (no receiver) were being flagged as reflection edges, producing false positives for gRPC ServiceDescriptor.getMethod(), Spring AnnotationUtils.getDeclaredMethod(), and proto-generated descriptors. Added receiver !== undefined (TS) and receiver.is_some() (Rust) guards, consistent with the existing invoke and forName guards in the same files. Bare calls now fall through to normal call edge handling. Impact: 2 functions changed, 4 affected
Sorry, something went wrong.
…n invoke false positives Impact: 1 functions changed, 2 affected
|
Fixed — added receiver guard to the simple_identifier invoke branch in kotlin.ts and kotlin.rs. Bare invoke() calls with no receiver now fall through to normal call-edge handling, preventing false sink edges for operator fun invoke() patterns. |
Sorry, something went wrong.
Sorry, something went wrong.
Summary
Stacked on PRs #1629 (Phase 0) and #1637 (Phase 1). Adds detection + flagging of JVM-family dynamic dispatch patterns.
All patterns mirrored in Rust extractors.
Benchmark results
Test plan
Known gap
Java/Scala/Groovy getMethod("name") detection works but doesn't resolve because methods are extracted as ClassName.methodName in the DB. Correct resolution needs the receiver type (clazz → Reflection) to construct the qualified lookup. Filed for RES-3.