-
Notifications
You must be signed in to change notification settings - Fork 16
fix(native): kotlin callable-ref prefers class method; suppress spurious invoke sink #1686
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -376,16 +376,14 @@ fn match_kotlin_node(node: &Node, source: &[u8], symbols: &mut FileSymbols, _dep | |
| .unwrap_or_else(|| node_text(&fn_node, source).to_string()); | ||
| let receiver = fn_node.child(0) | ||
| .map(|n| node_text(&n, source).to_string()); | ||
| // fn.invoke(args) — callable ref invocation; flag as unresolved | ||
| // fn.invoke(args) — callable ref invocation without static type | ||
| // info; the WASM grammar represents `navigation_expression` with a | ||
| // `navigation_suffix` child so its `lastChild.type` check misses | ||
| // `invoke` and emits nothing. Match that behaviour here so both | ||
| // engines agree: skip `invoke` calls rather than emitting an | ||
| // unresolvable-dynamic sink that the WASM path never produces. | ||
| 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 | ||
|
Comment on lines
385
to
+386
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The guard |
||
| } else { | ||
| symbols.calls.push(Call { | ||
| name, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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_languageso it fires for any non-JS/TS file. All 423cargo testcases pass, so there's no regression, but there's also no explicit fixture exercising the newgetMethod/invokeMethodpath 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!