Non deterministic behaviour with vararg methods

[rPraml]

Summary
If there are two methods, one with varargs and an other without, for example

• String args(String arg1, String... args)
• String args(String arg1)
  rhino considers these two methods as PREFERENCE_EQUAL when it is invoked with args('foo') from javascript. This may result in a non deterministic behaviour as one of the two was taken.

Details
In the case described above, all methods are compared and it is tried to find the best matchin one. So after preferSignaturewe will run into this code part in
NativeJavaMethod

                            if (preference != PREFERENCE_EQUAL) Kit.codeBug();
                            // This should not happen in theory
                            // but on some JVMs, Class.getMethods will return all
                            // static methods of the class hierarchy, even if
                            // a derived class's parameters match exactly.
                            // We want to call the derived class's method.
                            if (bestFit.isStatic()
                                    && bestFit.getDeclaringClass()
                                            .isAssignableFrom(member.getDeclaringClass())) {
                                // ... 
                            } else {
                                if (debug)
                                    printDebug("Ignoring same signature member ", member, args); // CASE 2
                            }

In our application, where the bug occured, we run in the Ignoring same signature member code path and one (not really deterministic - because of HashMap) method was taken. So the code sometimes works and sometimes not.

It was also not easy to provide a deterministic failing test, that's why (and maybe to add more determinism) I've changed the map in discoverAccessibleMethods to a LinkedHashMap (and yet the test depends heavily on the order of the JVM's getDeclaredMethods implementation)

Fix
When there are two candidates, the no-vararg method is taken. This is the same behaviour, as it is in Java.

[rPraml]

same test, but different order. If using HashMap instead of LinkedHashMap, I cannot provide stable failing tests

[rPraml]

To discuss: Should we make this change? It would help to add determinism (as long as the JVM's getDeclaredMethods returns them in the same order as in source code)

[gbrail]

I think that this change makes sense and fixes a bug, and I'm fine with using LinkedHashMap here so that it's deterministic. However, I don't currently work on anything that uses all of the Java reflection stuff in Rhino, and I'd love an opinion by another regular user who works with Rhino this way to make sure that we're not missing anything.

Thanks!