305323 - Rhino fails to select the appropriate overloaded method

Status: RESOLVED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Attila Szegedi]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0; en) Opera 8.02
Build Identifier: 

Note: this might be related to Bug 13415.

When Rhino needs to select from overloaded methods, and the declared types of 
methods are are (String, String, String) and (String, Object, Object), and the 
actual types are (java.lang.String, java.lang.String, org.mozilla.javascript.
NativeJavaObject wrapping java.lang.String), it will find the choice ambigous.

Reproducible: Always

Steps to Reproduce:
Run this example:

public class RhinoReflectionTest
{
    public static void assertEquals(String s1, String s2, String s3)
    {
        System.err.println("s");
    }
    
    public static void assertEquals(String s1, Object o2, Object o3)
    {
        System.err.println("o");
    }
    
    public static void main(String[] args) throws Exception
    {
        Context cx = Context.enter();
        Script s = cx.compileString(
                "Packages.test.RhinoReflectionTest.assertEquals(\"\", \"\", new 
java.lang.String());", "", 1, null);
        ScriptableObject scope = cx.initStandardObjects();
        s.exec(cx, scope);
    }
}
Actual Results:  
Exception was thrown:

org.mozilla.javascript.EvaluatorException: The choice of Java constructor 
assertEquals matching JavaScript argument types (string,string,java.lang.String) 
is ambiguous; candidate constructors are: 
    class java.lang.String assertEquals(java.lang.String,java.lang.Object,java.
lang.Object)
    class java.lang.String assertEquals(java.lang.String,java.lang.String,java.
lang.String) (#1)


Expected Results:  
Succesfully execute, printing "s".

Comment 1

[Attila Szegedi]

The problem is in NativeJavaObject.getConversionWeight that deviates from LC3 
method overloading specification <http://www.mozilla.org/js/liveconnect/
lc3_method_overloading.html> in point 3.3.6.1. Namely, if the method's declared 
argument type is java.lang.String, it will return "2" even if the passed 
NativeJavaObject is an instance of java.lang.String, where LC3 3.3.6.1 says that 
the first in the order of preference of conversions for NativeJavaObject should 
be "Any interface or class that is assignment-compatible with the Java object 
obtained by unwrapping the JS object, i.e. the unwrapped JavaObject is an 
instanceof() the Java argument type."

Comment 2

[Attila Szegedi]

Attachment: fix for the bug

Attached a fix for the bug. Basically, I moved the instanceOf check to be
earlier than the if(to == String.class) check, so it's compliant with LC3. I'm
not aware if this will break anything, nor am I aware if it was written
intentionally this way in the first place. So, I'd appreciate some peer review
before committing the patch.

Comment 3

[Attila Szegedi]

I was reviewing potential side-effects of this patch, but the NativeJavaObject.
getConversionWeight() method I modified is only used from NativeJavaMethod.
preferSignature() [it was fixed exactly for it] and from NativeJavaMethod.
canConvert(). canConvert() only checks if the returned value is less than 
CONVERSION_NONE, so the patch can have no effect on it. So I'm fairly confident 
this shouldn't break anything.

Comment 4

[Attila Szegedi]

Committed the patch to CVS.