jruby · Jul 21, 2015 · Jul 21, 2015 · Jul 21, 2015 · Jul 21, 2015 · Jul 21, 2015
diff --git a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java
@@ -166,15 +166,29 @@ private static <T extends ParameterTypes> T findMatchingCallableForArgs(final Ru
                 Class<?>[] msTypes = mostSpecific.getParameterTypes();
                 boolean ambiguous = false;
 
+                final IRubyObject lastArg = args.length > 0 ? args[ args.length - 1 ] : null;
+                int mostSpecificArity = Integer.MIN_VALUE;
+
+                final int procArity;
+                if ( lastArg instanceof RubyProc ) {
+                    final Method implMethod; final int last = msTypes.length - 1;
+                    if ( last >= 0 && msTypes[last].isInterface() && ( implMethod = getFunctionalInterfaceMethod(msTypes[last]) ) != null ) {
+                        mostSpecificArity = implMethod.getParameterTypes().length;
+                    }
+                    procArity = ((RubyProc) lastArg).getBlock().arity().getValue();
+                }
+                else {
+                    procArity = Integer.MIN_VALUE;
+                }
+
                 OUTER: for ( int c = 1; c < size; c++ ) {
                     final T candidate = candidates.get(c);
                     final Class<?>[] cTypes = candidate.getParameterTypes();
 
                     for ( int i = 0; i < msTypes.length; i++ ) {
                         final Class<?> msType = msTypes[i], cType = cTypes[i];
                         if ( msType != cType && msType.isAssignableFrom(cType) ) {
-                            mostSpecific = candidate;
-                            msTypes = cTypes;
+                            mostSpecific = candidate; msTypes = cTypes;
                             ambiguous = false; continue OUTER;
                         }
                     }
@@ -192,26 +206,68 @@ else if ( cType.isPrimitive() && msType.isAssignableFrom(getBoxType(cType)) ) {
                         }
                     }
 
+                    // special handling if we're dealing with Proc#impl :
+                    if ( procArity != Integer.MIN_VALUE ) { // lastArg instanceof RubyProc
+                        // cases such as (both ifaces - differ in arg count) :
+                        // java.io.File#listFiles(java.io.FileFilter) ... accept(File)
+                        // java.io.File#listFiles(java.io.FilenameFilter) ... accept(File, String)
+                        final Method implMethod; final int last = cTypes.length - 1;
+                        if ( last >= 0 && cTypes[last].isInterface() && ( implMethod = getFunctionalInterfaceMethod(cTypes[last]) ) != null ) {
+                            // we're sure to have an interface in the end - match arg count :
+                            // NOTE: implMethod.getParameterCount() on Java 8 would do ...
+                            final int methodArity = implMethod.getParameterTypes().length;
+                            if ( methodArity == procArity ) {
+                                if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity
+                                // TODO we could try to match parameter types with arg types
+                                else {
+                                    mostSpecific = candidate; msTypes = cTypes;
+                                    mostSpecificArity = procArity; ambiguous = false;
+                                }
+                                continue; /* OUTER */ // we want to check all
+                            }
+                            else if ( mostSpecificArity != procArity ) {
+                                if ( methodArity < procArity ) { // not ideal but still usable
+                                    if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity
+                                    else if ( mostSpecificArity < methodArity ) { // candidate is "better" match
+                                        mostSpecific = candidate; msTypes = cTypes;
+                                        mostSpecificArity = methodArity; ambiguous = false;
+                                    }
+                                    continue; /* OUTER */
+                                }
+                                else if ( procArity < 0 && methodArity >= -(procArity + 1) ) { // *splat that fits
+                                    if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity
+                                    else {
+                                        final int msa = mostSpecificArity + procArity;
+                                        final int ma = methodArity + procArity;
+                                        if ( ( msa < 0 && ma < 0 && msa < ma ) ||
+                                             ( msa >= 0 && ma >= 0 && msa > ma ) ||
+                                             ( msa > ma ) ) {
+                                            mostSpecific = candidate; msTypes = cTypes;
+                                            mostSpecificArity = methodArity; // ambiguous = false;
+                                        }
+                                    }
+                                    ambiguous = false;
+                                    continue; /* OUTER */
+                                }
+                            }
+                            else { // we're not a match and if there's something else matched than it's not really ambiguous
+                                ambiguous = false; /* continue; /* OUTER */
+                            }
+                        }
+                    }
+
                     // somehow we can still decide e.g. if we got a RubyFixnum
                     // then (int) constructor should be preferred over (float)
                     if ( ambiguous ) {
-                        // special handling if we're dealing with Proc#impl :
-                        final IRubyObject lastArg = args.length > 0 ? args[ args.length - 1 ] : null;
-                        final T procToIfaceMatch = matchProcToInterfaceCandidate(lastArg, candidates);
-                        if ( procToIfaceMatch != null ) {
-                            mostSpecific = procToIfaceMatch; ambiguous = false;
-                        }
-                        else {
-                            int msPref = 0, cPref = 0;
-                            for ( int i = 0; i < msTypes.length; i++ ) {
-                                final Class<?> msType = msTypes[i], cType = cTypes[i];
-                                msPref += calcTypePreference(msType, args[i]);
-                                cPref += calcTypePreference(cType, args[i]);
-                            }
-                            // for backwards compatibility we do not switch to cType as
-                            // the better fit - we seem to lack tests on this front ...
-                            if ( msPref > cPref ) ambiguous = false; // continue OUTER;
+                        int msPref = 0, cPref = 0;
+                        for ( int i = 0; i < msTypes.length; i++ ) {
+                            final Class<?> msType = msTypes[i], cType = cTypes[i];
+                            msPref += calcTypePreference(msType, args[i]);
+                            cPref += calcTypePreference(cType, args[i]);
                         }
+                        // for backwards compatibility we do not switch to cType as
+                        // the better fit - we seem to lack tests on this front ...
+                        if ( msPref > cPref ) ambiguous = false; // continue OUTER;
                     }
                 }
                 method = mostSpecific;
@@ -224,55 +280,28 @@ else if ( cType.isPrimitive() && msType.isAssignableFrom(getBoxType(cType)) ) {
 
         // fall back on old ways
         if (method == null) {
-            method = findCallable(methods, Exact, args);
-            if (method == null) {
-                method = findCallable(methods, AssignableAndPrimitivable, args);
-                if (method == null) {
-                    method = findCallable(methods, AssignableOrDuckable, args);
-                    if (method == null) {
-                        method = findCallable(methods, AssignableOrDuckable, args);
-                        if (method == null) {
-                            method = findCallable(methods, AssignableAndPrimitivableWithVarargs, args);
-                        }
-                    }
-                }
-            }
+            method = findMatchingCallableForArgsFallback(runtime, methods, args);
         }
 
         return method;
     }
 
-    private static <T extends ParameterTypes> T matchProcToInterfaceCandidate(
-            final IRubyObject lastArg, final List<T> candidates) {
-        if ( lastArg instanceof RubyProc ) {
-            // cases such as (both ifaces - differ in arg count) :
-            // java.io.File#listFiles(java.io.FileFilter) ... accept(File)
-            // java.io.File#listFiles(java.io.FilenameFilter) ... accept(File, String)
-            final int arity = ((RubyProc) lastArg).getBlock().arity().getValue();
-            T match = null;
-            for ( int i = 0; i < candidates.size(); i++ ) {
-                final T method = candidates.get(i);
-
-                final Class<?>[] params = method.getParameterTypes();
-
-                if ( params.length == 0 ) return null; // can not match (no args)
-                final Class<?> lastParam = params[ params.length - 1 ];
-
-                if ( ! lastParam.isInterface() ) return null; // can not match
-
-                final Method implMethod = getFunctionalInterfaceMethod(lastParam);
-                if ( implMethod != null ) {
-                    // we're sure to have an interface in the end - match arg count :
-                    // NOTE: implMethod.getParameterCount() on Java 8 would do ...
-                    if ( implMethod.getParameterTypes().length == arity ) {
-                        if ( match != null ) return null; // 2 with same arity (can not match)
-                        match = method; // do not break here we want to check all
+    private static <T extends ParameterTypes> T findMatchingCallableForArgsFallback(final Ruby runtime,
+        final T[] methods, final IRubyObject... args) {
+        T method = findCallable(methods, Exact, args);
+        if (method == null) {
+            method = findCallable(methods, AssignableAndPrimitivable, args);
+            if (method == null) {
+                method = findCallable(methods, AssignableOrDuckable, args);
+                if (method == null) {
+                    method = findCallable(methods, AssignableOrDuckable, args);
+                    if (method == null) {
+                        method = findCallable(methods, AssignableAndPrimitivableWithVarargs, args);
                     }
                 }
             }
-            return match;
         }
-        return null;
+        return method;
     }
 
     private static <T extends ParameterTypes> T findCallable(T[] callables, CallableAcceptor acceptor, IRubyObject[] args) {
@@ -304,17 +333,42 @@ private static <T extends ParameterTypes> List<T> findCallableCandidates(final T
         ParameterTypes[] incoming = callables.clone();
 
         for ( int i = 0; i < args.length; i++ ) {
-            retained.clear();
-            for ( final Matcher matcher : NON_EXACT_MATCH_SEQUENCE ) {
+            retained.clear(); // non-exact match sequence :
+            // PRIMITIVABLE :
+            for ( int c = 0; c < incoming.length; c++ ) {
+                ParameterTypes callable = incoming[c];
+                if ( callable == null ) continue; // removed (matched)
+
+                Class[] types = callable.getParameterTypes();
+
+                if ( PRIMITIVABLE.match( types[i], args[i] ) ) {
+                    retained.add((T) callable);
+                    incoming[c] = null; // retaining - remove
+                }
+            }
+            // ASSIGNABLE :
+            for ( int c = 0; c < incoming.length; c++ ) {
+                ParameterTypes callable = incoming[c];
+                if ( callable == null ) continue; // removed (matched)
+
+                Class[] types = callable.getParameterTypes();
+
+                if ( ASSIGNABLE.match( types[i], args[i] ) ) {
+                    retained.add((T) callable);
+                    incoming[c] = null; // retaining - remove
+                }
+            }
+            if ( retained.isEmpty() ) {
+                // DUCKABLE :
                 for ( int c = 0; c < incoming.length; c++ ) {
                     ParameterTypes callable = incoming[c];
                     if ( callable == null ) continue; // removed (matched)
 
                     Class[] types = callable.getParameterTypes();
 
-                    if ( matcher.match( types[i], args[i] ) ) {
+                    if ( DUCKABLE.match( types[i], args[i] ) ) {
                         retained.add((T) callable);
-                        incoming[c] = null; // retaining - remove
+                        //incoming[c] = null; // retaining - remove
                     }
                 }
             }
@@ -412,9 +466,6 @@ public boolean match(Class type, IRubyObject arg) {
         @Override public String toString() { return "DUCKABLE"; } // for debugging
     };
 
-    //private static final Matcher[] MATCH_SEQUENCE = new Matcher[] { EXACT, PRIMITIVABLE, ASSIGNABLE, DUCKABLE };
-    private static final Matcher[] NON_EXACT_MATCH_SEQUENCE = new Matcher[] { PRIMITIVABLE, ASSIGNABLE, DUCKABLE };
-
     private static boolean exactMatch(ParameterTypes paramTypes, IRubyObject... args) {
         Class[] types = paramTypes.getParameterTypes();
 

diff --git a/core/src/main/java/org/jruby/javasupport/JavaMethod.java b/core/src/main/java/org/jruby/javasupport/JavaMethod.java
@@ -46,7 +46,6 @@
 import org.jruby.Ruby;
 import org.jruby.RubyBoolean;
 import org.jruby.RubyClass;
-import org.jruby.RubyInstanceConfig;
 import org.jruby.RubyModule;
 import org.jruby.RubyString;
 import org.jruby.anno.JRubyClass;
@@ -57,17 +56,13 @@
 import org.jruby.javasupport.proxy.JavaProxyMethod;
 import org.jruby.runtime.ObjectAllocator;
 import org.jruby.runtime.builtin.IRubyObject;
-import org.jruby.util.log.Logger;
-import org.jruby.util.log.LoggerFactory;
 
 import static org.jruby.util.CodegenUtils.getBoxType;
 import static org.jruby.util.CodegenUtils.prettyParams;
 
 @JRubyClass(name="Java::JavaMethod")
 public class JavaMethod extends JavaCallable {
 
-    //private static final Logger LOG = LoggerFactory.getLogger("JavaMethod");
-
     //private final static boolean USE_HANDLES = RubyInstanceConfig.USE_GENERATED_HANDLES;
     //private final static boolean HANDLE_DEBUG = false;
 
@@ -188,7 +183,7 @@ public static JavaMethod getMatchingDeclaredMethod(Ruby runtime, Class<?> javaCl
 
     @Override
     public final boolean equals(Object other) {
-        return other instanceof JavaMethod && this.method.equals( ((JavaMethod)other).method );
+        return other instanceof JavaMethod && this.method.equals( ((JavaMethod) other).method );
     }
 
     @Override
@@ -594,7 +589,7 @@ public String toGenericString() {
     }
 
     @Override
-    public AccessibleObject accessibleObject() {
+    public final AccessibleObject accessibleObject() {
         return method;
     }
 

diff --git a/core/src/test/java/org/jruby/java/dispatch/CallableSelectorTest.java b/core/src/test/java/org/jruby/java/dispatch/CallableSelectorTest.java
@@ -0,0 +1,171 @@
+/*
+ * Copyright (c) 2015 JRuby.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.jruby.java.dispatch;
+
+import java.lang.reflect.Method;
+import org.jruby.Ruby;
+import org.jruby.RubyProc;
+import org.jruby.javasupport.JavaMethod;
+import org.jruby.runtime.Arity;
+import org.jruby.runtime.Binding;
+import org.jruby.runtime.Block;
+import org.jruby.runtime.BlockBody;
+import org.jruby.runtime.Frame;
+import org.jruby.runtime.NullBlockBody;
+import org.jruby.runtime.backtrace.BacktraceElement;
+import org.jruby.runtime.builtin.IRubyObject;
+import org.jruby.util.collections.IntHashMap;
+
+import org.junit.Test;
+import static org.junit.Assert.*;
+
+/**
+ * @author kares
+ */
+public class CallableSelectorTest {
+
+    @Test
+    public void testCallableProcToIfaceMatchIsNotOrderSensitive() throws Exception {
+        final Ruby runtime = Ruby.newInstance();
+
+        final Method list1 = java.io.File.class.getMethod("listFiles", java.io.FileFilter.class);
+        final Method list2 = java.io.File.class.getMethod("listFiles", java.io.FilenameFilter.class);
+
+        IntHashMap cache;
+        JavaMethod[] methods;
+        Binding binding = new Binding(new Frame(), null, null, new BacktraceElement());
+        JavaMethod result; IRubyObject[] args;
+
+        // arity 1 :
+
+        BlockBody body1 = new NullBlockBody() {
+            @Override public Arity arity() { return Arity.ONE_ARGUMENT; }
+        };
+        RubyProc dummyProc = RubyProc.newProc(runtime, new Block(body1, binding), Block.Type.PROC);
+
+        cache = IntHashMap.nullMap();
+        methods = new JavaMethod[] {
+            new JavaMethod(runtime, list1), new JavaMethod(runtime, list2)
+        };
+        result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
+        assertEquals(new JavaMethod(runtime, list1), result);
+
+        cache = IntHashMap.nullMap();
+        args = new IRubyObject[] { dummyProc };
+        result = CallableSelector.matchingCallableArityN(runtime, cache, methods, args);
+        assertEquals(new JavaMethod(runtime, list1), result);
+
+        cache = IntHashMap.nullMap();
+        methods = new JavaMethod[] { // "reverse" method order
+            new JavaMethod(runtime, list2), new JavaMethod(runtime, list1)
+        };
+        result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
+        assertEquals(new JavaMethod(runtime, list1), result);
+
+        cache = IntHashMap.nullMap();
+        args = new IRubyObject[] { dummyProc };
+        result = CallableSelector.matchingCallableArityN(runtime, cache, methods, args);
+        assertEquals(new JavaMethod(runtime, list1), result);
+
+        // arity 2 :
+
+        BlockBody body2 = new NullBlockBody() {
+            @Override public Arity arity() { return Arity.TWO_ARGUMENTS; }
+        };
+        dummyProc = RubyProc.newProc(runtime, new Block(body2, binding), Block.Type.PROC);
+
+        cache = IntHashMap.nullMap();
+        methods = new JavaMethod[] {
+            new JavaMethod(runtime, list1), new JavaMethod(runtime, list2)
+        };
+        result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
+        assertEquals(new JavaMethod(runtime, list2), result);
+
+        cache = IntHashMap.nullMap();
+        args = new IRubyObject[] { dummyProc };
+        result = CallableSelector.matchingCallableArityN(runtime, cache, methods, args);
+        assertEquals(new JavaMethod(runtime, list2), result);
+
+        cache = IntHashMap.nullMap();
+        methods = new JavaMethod[] { // "reverse" method order
+            new JavaMethod(runtime, list2), new JavaMethod(runtime, list1)
+        };
+        result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
+        assertEquals(new JavaMethod(runtime, list2), result);
+
+        cache = IntHashMap.nullMap();
+        args = new IRubyObject[] { dummyProc };
+        result = CallableSelector.matchingCallableArityN(runtime, cache, methods, args);
+        assertEquals(new JavaMethod(runtime, list2), result);
+
+        // arity -1 :
+
+        BlockBody body_1 = new NullBlockBody() { // arity -1
+            @Override public Arity arity() { return Arity.OPTIONAL; }
+        };
+        dummyProc = RubyProc.newProc(runtime, new Block(body_1, binding), Block.Type.PROC);
+
+        cache = IntHashMap.nullMap();
+        methods = new JavaMethod[] {
+            new JavaMethod(runtime, list1), new JavaMethod(runtime, list2)
+        };
+        result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
+        assertEquals(new JavaMethod(runtime, list1), result);
+
+        cache = IntHashMap.nullMap();
+        methods = new JavaMethod[] {
+            new JavaMethod(runtime, list2), new JavaMethod(runtime, list1)
+        };
+        result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
+        assertEquals(new JavaMethod(runtime, list1), result);
+
+        // arity -3 :
+
+        BlockBody body_3 = new NullBlockBody() { // arity -3
+            @Override public Arity arity() { return Arity.TWO_REQUIRED; }
+        };
+        dummyProc = RubyProc.newProc(runtime, new Block(body_3, binding), Block.Type.PROC);
+
+        cache = IntHashMap.nullMap();
+        methods = new JavaMethod[] {
+            new JavaMethod(runtime, list1), new JavaMethod(runtime, list2)
+        };
+        result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
+        assertEquals(new JavaMethod(runtime, list2), result);
+
+        cache = IntHashMap.nullMap();
+        methods = new JavaMethod[] {
+            new JavaMethod(runtime, list2), new JavaMethod(runtime, list1)
+        };
+        result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
+        assertEquals(new JavaMethod(runtime, list2), result);
+
+        // arity -2 :
+
+        BlockBody body_2 = new NullBlockBody() { // arity -2 (arg1, *rest) should prefer (single)
+            @Override public Arity arity() { return Arity.ONE_REQUIRED; }
+        };
+        dummyProc = RubyProc.newProc(runtime, new Block(body_2, binding), Block.Type.PROC);
+
+        cache = IntHashMap.nullMap();
+        methods = new JavaMethod[] {
+            new JavaMethod(runtime, list1), new JavaMethod(runtime, list2)
+        };
+        result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
+        assertEquals(new JavaMethod(runtime, list1), result);
+
+        cache = IntHashMap.nullMap();
+        methods = new JavaMethod[] {
+            new JavaMethod(runtime, list2), new JavaMethod(runtime, list1)
+        };
+        result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
+        assertEquals(new JavaMethod(runtime, list1), result);
+
+    }
+
+}
diff --git a/spec/java_integration/interfaces/implementation_spec.rb b/spec/java_integration/interfaces/implementation_spec.rb
@@ -218,6 +218,42 @@ def callIt
       dir.should be_kind_of(java.io.File)
       name.should be_kind_of(String)
     end
+
+    java.io.File.new('.').listFiles do |dir, name, invalid|
+      # should choose FilenameFilter#accept(File, String)
+      dir.should be_kind_of(java.io.File)
+      name.should be_kind_of(String)
+    end
+
+    java.io.File.new('.').listFiles do |pathname, *args|
+      pathname.should be_kind_of(java.io.File)
+      args.should be_empty
+    end
+
+    java.io.File.new('.').listFiles do |dir, name, *args|
+      dir.should be_kind_of(java.io.File)
+      name.should be_kind_of(String)
+      args.should be_empty
+    end
+
+    java.io.File.new('.').listFiles do |*args|
+      args[0].should be_kind_of(java.io.File)
+      args.size.should eql 1
+    end
+
+    #executor = java.util.concurrent.Executors.newSingleThreadExecutor
+    #executor.execute { |*args| args.should be_empty }; sleep 0.1
+    #executor.shutdown
+
+    work_queue = java.util.concurrent.LinkedBlockingQueue.new
+    executor = java.util.concurrent.ThreadPoolExecutor.new(0, 2, 0, java.util.concurrent.TimeUnit::SECONDS, work_queue) do
+      |*args| # newThread(Runnable)
+      args[0].should be_kind_of(java.lang.Runnable)
+      args.size.should eql 1
+      java.lang.Thread.new(args[0])
+    end
+    executor.execute { |*args| args.should be_empty }; sleep 0.1
+    executor.shutdown
   end
 
   it "should maintain Ruby object equality when passed through Java and back" do