404211 - Multiple competing function definitions leads to chaotic results

Status: RESOLVED FIXED

(Rhino Graveyard :: Core, defect)

Description

[benson margulies]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.9) Gecko/20071025 Firefox/2.0.0.9
Build Identifier: 1.6R7

Defining a JavaScript class in Java:

If you mistakenly try to define multiple overloads of a JavaScript method via multiple jsFunction_foo functions, there is no diagnosis, and unpredictable results. One of the function will end up as the function-of-record, but which one depends on the JVM version or the phase of the moon.

Reproducible: Always

Steps to Reproduce:
1. Define class with jsFunction_send() and jsFunction_send(String x)
2. write code to call it from JavaScript

Actual Results:  
No diagnosis and unpredictable results.

Expected Results:  
A diagnosis. Or at least some rule to determine which function gets the prize.

Comment 1

[Norris Boyd]

Yes, we should report an error.

Comment 2

[Norris Boyd]

Fixed: 

Checking in src/org/mozilla/javascript/ScriptableObject.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/ScriptableObject.java,v  <--  ScriptableObject.java
new revision: 1.133; previous revision: 1.132
done

Comment 3

[Hannes Wallnoefer]

I think the committed fix breaks getter/setter definitions when both jsGet_* and jsSet_* methods are defined for a property. I have methods jsGet_status and jsSet_status to defina a read/write property named status and get the following: 

org.mozilla.javascript.EvaluatorException: Invalid method "jsSet_status": name "status" is already in use. (/home/hannes/svn/helma-ng/modules/helma/rhino.js#24)
        at org.mozilla.javascript.DefaultErrorReporter.runtimeError(DefaultErrorReporter.java:109)
        at org.mozilla.javascript.Context.reportRuntimeError(Context.java:922)
        at org.mozilla.javascript.Context.reportRuntimeError(Context.java:978)
        at org.mozilla.javascript.Context.reportRuntimeError2(Context.java:948)
        at org.mozilla.javascript.ScriptableObject.buildClassCtor(ScriptableObject.java:1159)
        at org.mozilla.javascript.ScriptableObject.defineClass(ScriptableObject.java:998)
        at org.mozilla.javascript.ScriptableObject.defineClass(ScriptableObject.java:931)
        at org.helma.javascript.RhinoEngine.defineHostClass(RhinoEngine.java:108)
        ....

Comment 4

[Hannes Wallnoefer]

Attachment: Fix definition of getters and setters

This patch fixes the problem with jsGet_ and jsSet_ described in comment #3. I just moved the check for conflicts down below the block that handles jsGet_ and jsSet_. Since now the method name and property name are required to coexist for longer, I renamed variable name to methodName and always use propName when referring to the property name.

Comment 5

[Hannes Wallnoefer]

Attachment: A simpler and less obtrusive patch

This patch is much simpler than the previous one. We're simply moving up the continue statement to where the setterPrefix is detected, simplifying the code and avoiding the duplicate method check at the same time.

Comment 6

[Norris Boyd]

Thanks for finding and fixing this. I've submitted your patch.

Comment 7

[Jose Losada]

I'm migrating from rhino 1.6R7 to rhino 1.7R2 and I'm getting the following error:

Invalid method "jsGet_tagName": name "tagName" is already in use

I have a class with the method jsGet_tagName and this getter method is redefined in one subclass.
I'm getting the error while redefining the subclass.

In rhino 1.6R7 everything works fine.

Could be a bug related with this issue?