timob / jnigi

Golang Java JNI library
BSD 2-Clause "Simplified" License
163 stars 44 forks source link

Fix calling methods that return void #59

Closed iamcalledrob closed 2 years ago

iamcalledrob commented 2 years ago

As part of #56, a change was introduced that allowed support for Void types in CallStaticMethod. This was somewhat of an oversight in my commit.

https://github.com/iamcalledrob/jnigi/blob/b9076f4b244646ae7818cd22729eb631f0a7bbf4/jnigi.go#L1398-L1402

if v, ok := dest.(ToGoConverter); ok && (rType&Object == Object || rType&Array == Array) {
    // ...
} else if rType.isArray() && rType != Object|Array {
    // ...
} else if rType != Void {                    // <-- Change
    return assignDest(retVal, dest)      // <--
} else {
    return nil
}

This addressed an issue I was experiencing where I couldn't call a method with a void dest.

When I tried passing in jnigi.Void as the call dest, the following error occurred:

jnigi.CallStaticMethod("com/foo/bar", "myVoidMethod", jnigi.Void /* <- dest */, someArg)
// JNIGI Error: expected dest argument to be <nil>[val] (not jnigi.Type[dest]) or nil)

However when nil was passed in, this error occurred:

jnigi.CallStaticMethod("com/foo/bar", "myVoidMethod", nil /* <- dest */, someArg)
// JNIGI: unknown type <nil> (value = <nil>)):

This led to it being impossible to make calls to void methods, at least as far as I could tell.

What appears to be happening is that if dest was set to jnigi.Void, the method signature would be calculated correctly, however assignDest would fail, since jnigi.Void is not a handled case.

However if the dest was set to nil, calculating the method signature in sigForMethod would fail, because typeOfValue does not handle nil. The resulting assignDest would have succeeded, since it does a nil check, but we never get there because of the signature calculation failure.

The fix in #56 addressed this issue in a single specific case (CallStaticMethod), but not universally.

This PR applies a fix to typeOfValue to map nil to jnigi.Void, so the method signature can be calculated correctly. The existing nil check in assignDest then works as expected. Methods that return void can now be called as follows:

jnigi.CallStaticMethod("com/foo/bar", "myVoidMethod", nil /* <- dest */, someArg)
timob commented 2 years ago

LGTM. Thanks for fixing that. In V1 of the package the return type was specified by a Type value.