joeferner/node-java

varargs ambiguity with generics

henridf opened this issue · 7 comments

> var Arrays = java.import('java.util.Arrays');  
> var list = Arrays.asList(['a', 'b']);
> list.toArray();
[ [ 'a', 'b' ] ] // should be ['a', 'b'] !!

I haven't looked at the varargs handling code, but I'm assuming it's because of the ambiguity due to the asList method being generic (List<T> asList(T... a)) - there's no way to distinguish if the javascript Array being passed into Arrays.asList() is intended as a single value of type T or if it's intended as a T[].

Assuming that's the issue, we could detect this ambiguity and throw an error, requiring the javascript programmer to use apply when they want a javascript array to be 'spread' into a varargs generic function.

Does this make sense?

(Btw, ES6 compiled by babel works just fine here with Arrays.asList(...arr);).

You are probably correct. We rely on a fork of commons-lang to find which method matches using MethodUtils#getMatchingAccessibleMethod. We have already patched some of the ambiguity issues that commons-lang was not handling this is just another missing case. When I have some time I'll take a look.

I'm happy to work on this if that's helpful -- let me know.

Absolutely that would be great.

Here's my suggestion for the desired behavior: the javascript call Arrays.asList([a, b, c]) would result in the underlying Java call Arrays.asList(a, b, c).

If you want a single array arg to be passed to the java method, then you'd do Arrays.asList([[a, b, c]]), which results in the underlying Java call Arrays.asList([a, b, c]).

Does this behavior make sense?

Can we make the rules the same as Java? How does it handle these cases?

Based on the little experiment below, I'd say the existing behavior is correct if we want to make things the same as Java. So this is not a bug, though we might want to add something to the README to explain this behavior.

import java.util.Arrays;                                                                                               
import java.util.List;                                                                                                 

public class testvarargs {                                                                                                   
    public static void varargs1(int... numbers) {                                                                      
        System.out.println(numbers[0]);                                                                                
    }                                                                                                                  
    public static void varargs2(Object... numbers) {                                                                   
        System.out.println(numbers[0]);                                                                                
    }                                                                                                                  
    public static <T> void varargs3(T... numbers) {                                                                    
        System.out.println(numbers[0]);                                                                                
    }                                                                                                                  

    public static void main(String[] args) {                                                                           
        int[] ints = new int[] {1, 2, 3};                                                                              

        varargs1(ints);   // prints 1                                                                                             
        varargs2(ints);   // prints [I@7852e922                                                                                
        varargs3(ints);   // prints [I@7852e922                                                           
    }                                                                                                                  
}