fix(native): kotlin callable-ref prefers class method; suppress spurious invoke sink#1686
Merged
Merged
Conversation
…r Groovy/Java/Scala JVM getMethod/invokeMethod calls with literal method names were emitted by the Rust extractors with dynamicKind='reflection' and keyExpr set, but resolve_call_targets in build_edges.rs had no path to resolve them. Methods in Groovy/Java/Scala are stored as class-qualified names (e.g. DynamicDispatch.greet), so the plain name lookup of 'greet' finds nothing. The TypeScript resolver has a RES-3 fallback in resolveFallbackTargets that handles this via two qualified lookups: 1. typeMap[receiver] -> resolvedType.keyExpr (type-annotated locals) 2. callerName class prefix -> CallerClass.keyExpr (same-class siblings) Add the equivalent block to resolve_call_targets in Rust, scoped to non-JS/TS files to avoid interfering with the existing JS reflection path. Fixes the wasm=1 native=0 divergence for: dynamic-groovy: DynamicDispatch.runInvokeMethod -> DynamicDispatch.greet dynamic-java: Reflection.runGetMethod -> Reflection.greet dynamic-scala: Reflection.runGetMethod -> Reflection.greet Closes #1681
…ous invoke sink
RES-4 pre-pass in resolve_call_targets: when a call has dynamicKind='reflection'
and a receiver but no keyExpr (i.e. Greeter::greet member callable reference),
look up {Receiver}.{name} in the same file before the plain {name} same-file
lookup. Without this, greet matched the top-level free function instead of
Greeter.greet (the class method), diverging from the WASM engine which has an
equivalent RES-4 pre-pass in resolveFallbackTargets.
For fn.invoke() patterns: the WASM grammar models navigation_expression with a
navigation_suffix child, so WASM's lastChild.type check never sees 'simple_identifier'
and emits nothing. Native was emitting an <dynamic:unresolved> call which produced a
spurious sink edge. Suppress the invoke call in native to match WASM behaviour.
Closes #1682
Contributor
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+1022
to
+1068
| // RES-3: reflection with literal method name — JVM getMethod("name") / invokeMethod("name"). | ||
| // Java/Scala/Groovy methods are stored as class-qualified names (e.g. Reflection.greet), | ||
| // so a plain lookup of `keyExpr` finds nothing. When dynamicKind='reflection' and keyExpr | ||
| // is set (a string-literal method name was captured), try two qualified forms: | ||
| // 1. typeMap[receiver] → resolvedType → `resolvedType.keyExpr` (type-annotated local) | ||
| // 2. callerName class prefix → `CallerClass.keyExpr` (same-class sibling — covers Groovy | ||
| // obj.invokeMethod and Java/Scala clazz.getMethod where the class is the caller's own) | ||
| // Scoped to non-JS/TS files to avoid interfering with the JS reflection path. | ||
| // Mirrors `resolveFallbackTargets` RES-3 block in `src/domain/graph/builder/stages/build-edges.ts`. | ||
| if call.dynamic_kind.as_deref() == Some("reflection") | ||
| && call.key_expr.is_some() | ||
| && call.receiver.is_some() | ||
| && !is_module_scoped_language(rel_path) | ||
| { | ||
| let key_expr = call.key_expr.as_deref().unwrap(); | ||
| let receiver = call.receiver.as_deref().unwrap(); | ||
|
|
||
| // RES-3.1: typeMap[receiver] → resolvedType.keyExpr | ||
| if let Some(&(resolved_type, _)) = type_map.get(receiver) { | ||
| let qualified = format!("{}.{}", resolved_type, key_expr); | ||
| let typed: Vec<&NodeInfo> = ctx.nodes_by_name | ||
| .get(qualified.as_str()) | ||
| .map(|v| v.iter() | ||
| .filter(|n| (n.kind == "method" || n.kind == "function") | ||
| && resolve::compute_confidence(rel_path, &n.file, None) >= 0.5) | ||
| .copied().collect()) | ||
| .unwrap_or_default(); | ||
| if !typed.is_empty() { return typed; } | ||
| } | ||
|
|
||
| // RES-3.2: callerName class prefix → CallerClass.keyExpr | ||
| if !caller_name.is_empty() { | ||
| if let Some(last_dot) = caller_name.rfind('.') { | ||
| let seg_start = caller_name[..last_dot].rfind('.').map(|p| p + 1).unwrap_or(0); | ||
| let caller_class = &caller_name[seg_start..last_dot]; | ||
| let qualified = format!("{}.{}", caller_class, key_expr); | ||
| let class_scoped: Vec<&NodeInfo> = ctx.nodes_by_name | ||
| .get(qualified.as_str()) | ||
| .map(|v| v.iter() | ||
| .filter(|n| (n.kind == "method" || n.kind == "function") | ||
| && resolve::compute_confidence(rel_path, &n.file, None) >= 0.5) | ||
| .copied().collect()) | ||
| .unwrap_or_default(); | ||
| if !class_scoped.is_empty() { return class_scoped; } | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
The diff adds a full RES-3 JVM-reflection resolution block (typeMap lookup + caller-class prefix) but the PR description and test plan cover only dynamic-kotlin. The block is guarded by !is_module_scoped_language so it fires for any non-JS/TS file. All 423 cargo test cases pass, so there's no regression, but there's also no explicit fixture exercising the new getMethod/invokeMethod path in this changeset. A dedicated Java or Groovy reflection fixture (or at minimum a unit test) would give future reviewers confidence that the two qualified forms behave as intended.
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!
Comment on lines
385
to
+386
| if name == "invoke" { | ||
| symbols.calls.push(Call { | ||
| name: "<dynamic:unresolved>".to_string(), | ||
| line: start_line(node), | ||
| dynamic: Some(true), | ||
| dynamic_kind: Some("unresolved-dynamic".to_string()), | ||
| receiver, | ||
| ..Default::default() | ||
| }); | ||
| // intentionally emit nothing — mirrors WASM engine behaviour |
Contributor
There was a problem hiding this comment.
invoke suppression is file-wide, not scoped to unresolvable function references
The guard if name == "invoke" drops every .invoke() navigation call regardless of receiver type. A Kotlin class with operator fun invoke(...) called via obj.invoke(...) would also be silently skipped, even if obj's type is statically known. Since this is an intentional WASM-parity decision it may be acceptable, but a short comment noting that operator-invoke edges are also dropped (not just function-reference calls) would help future maintainers distinguish deliberate suppression from an oversight.
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
resolve_call_targets: when a reflection-kind call has a receiver but nokeyExpr(e.g.Greeter::greetmember callable reference), look up{Receiver}.{name}in the same file before the plain{name}same-file lookup. Without this,greetmatched the top-level free function instead ofGreeter.greet(the class method), diverging from the WASM engine which has an equivalent RES-4 pre-pass inresolveFallbackTargetsinbuild-edges.ts.invokesink: the WASM grammar modelsnavigation_expressionwith anavigation_suffixchild so itslastChild.typecheck never sees'simple_identifier'and emits nothing forfn.invoke()patterns. Native was emitting an<dynamic:unresolved>call which produced a spurious sink edge not present in WASM. Suppress theinvokeemission in native to match WASM behaviour.Fixes both parity gaps reported in #1682 —
dynamic-kotlinfixture now shows 0 edge diffs between WASM and native (verified withnode scripts/parity-compare.mjs --langs dynamic-kotlin).Test plan
node scripts/parity-compare.mjs --langs dynamic-kotlin→ PARITY OK (0 diffs, was 3)npx vitest run tests/benchmarks/resolution/resolution-benchmark.test.ts→dynamic-kotlin: precision=100%, recall=100% (3/3 edges)cargo testincrates/codegraph-core→ 423/423 tests passCloses #1682