Skip to content

Escape generated JNI for @JavaImplementation methods#575

Open
colemancda wants to merge 2 commits intoswiftlang:mainfrom
PureSwift:feature/jni-callback-escaping
Open

Escape generated JNI for @JavaImplementation methods#575
colemancda wants to merge 2 commits intoswiftlang:mainfrom
PureSwift:feature/jni-callback-escaping

Conversation

@colemancda
Copy link
Contributor

Fix generated JNI for @JavaImplementation methods by escaping underscores and other characters correctly.

@colemancda colemancda requested a review from ktoso as a code owner February 26, 2026 18:54

// JNI identifier escaping per the JNI specification:
// https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#resolving_native_method_names
private extension String {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private extension String {
extension String {

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to resolve the CI failure, we put access control on members, not extensions

/// - `;` → `_2`
/// - `[` → `_3`
/// - Non-ASCII → `_0XXXX` (UTF-16 hex)
var escapedJNIIdentifier: String {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var escapedJNIIdentifier: String {
private var escapedJNIIdentifier: String {

Copy link
Collaborator

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for linking the relevant JNI docs

Needs to get the CI failure fixed, sadly didn't grant me maintainer edit rights so wasn't able to apply myself :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants