382340 - Code contributions: vararg support, type coercion, attributes

Status: RESOLVED INACTIVE

(Rhino Graveyard :: Core, defect)

Description

[Norris Boyd]

Contributions from Ulrike Mueller <[e-mail]>:

Attached a diff file and the source files of all our changes that make sense as contribution. The diff is made against 1.6R4, which I think is fine because 1.6R5 only contains the license change.

 

Here a description of the enhancements:

 

Java5 varg support:

Java5 varg methods can be called with varg style from JavaScript. The relevant changes are in MemberBox, NativeJavaClass and NativeJavaMethod. The call to isVarg() in MemberBox is the one Java5 specific call, which probably needs to be converted into a JRE version dependent call based on reflection.

 

Better type coercion:

    * when converting a JavaScript native object with the Java target class “Object.class” Rhino so far didn’t convert into the objects “natural” Java type. This is changed.
    * Conversion of boolen was missing
    * Conversion of an uninitialized (= elements are undefined) native JavaScript array
    * Conversion into BigDecimal was missing

All changes are in NativeJavaObject.

 

Support for “name” and “constructor” attributes in NativeJavaClass:

By supporting these two attributes the programming model for native JavaScript functions and Java is better aligned. For example “name” can be used regardless of whether it is a native JavaScript function or a function backed by a Java Class.

 

Coercion delay:

There is a little change in NativeJavaObject that delays a number coercion. We had the following issue in the system: When using Rhino for simple expressions on Java objects sometimes the Java object did return an integer, which was immediately converted into a double even so no further processing with a JavaScript expressions did happen. This caused some side effects. With this change the integer stays an integer until used in an expression. This also means that Rhino can return an integer if it is a direct result of a call to a Java method/property.

 

Compile warning:

There are two lines changed in NativeJavaClass to avoid a compiler warning.

Comment 1

[Attila Szegedi]

Please attach the diff -- I'd like to review this very much.

Comment 2

[Norris Boyd]

Attachment: Patch from contributor.

Comment 3

[Norris Boyd]

I've submitted changes for Java5 varg support and Compile warning. 

Comment 4

[Hannes Wallnoefer]

The varargs patch does not handle the case where a varargs method is called with a java array. For example, the following code returns a List containing a string array as single element, instead of a List containing all the strings in the array:

java.util.Arrays.asList(java.util.TimeZone.getAvailableIDs());

I'll post a patch for this shortly.

Comment 5

[Hannes Wallnoefer]

Attachment: patch to support java arrays for varargs

This patch fixes the behaviour when passing a java array as varargs parameter. 

I also unified the behaviour between methods and constructors when passing null as vararg parameter to mean a null array rather than an array of length one containing null.

Comment 6

[Norris Boyd]

Good catch. Committed patch:

Checking in src/org/mozilla/javascript/NativeJavaClass.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/NativeJavaClass.java,v  <--  NativeJavaClass.java
new revision: 1.48.2.2; previous revision: 1.48.2.1
done
Checking in src/org/mozilla/javascript/NativeJavaMethod.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/NativeJavaMethod.java,v  <--  NativeJavaMethod.java
new revision: 1.58.2.1; previous revision: 1.58
done

Comment 7

[Attila Szegedi]

> This patch fixes the behaviour when passing a java array as varargs parameter. 

Does this mean you can actually invoke a varargs method by manually packaging the varargs in an array, and passing it as the last argument? Hm... Honestly, I find this to not be a terribly good idea. You open up venues for ambiguity, like if the method is declared as 

foo(Object...)

now, foo(someArray) could be both foo(someArray) and foo(new Object[] { someArray}) right. In reality, the fact that varargs methods get their arguments packaged in an array is sort of a language implementation detail really, and I wouldn't encourage people to do this directly.

> 
> I also unified the behaviour between methods and constructors when passing null
> as vararg parameter to mean a null array rather than an array of length one
> containing null. 
> 

Comment 8

[Attila Szegedi]

(In reply to comment #5)
> I also unified the behaviour between methods and constructors when passing null
> as vararg parameter to mean a null array rather than an array of length one
> containing null. 
> 

But... A Java call site as emitted by the compiler won't ever pass null for the array parameter for a vararg method -- vararg methods can rely on always getting a non-null array in the last argument, so you'd break them by passing null. null should indeed get converted to a single-element array containing null, as that's how it would work from Java as well, provided the vararg isn't a primitive type, of course, in which case it should fail as you can't really coerce null to any primitive type.

Comment 9

[Hannes Wallnoefer]

(In reply to comment #7)
> 
> Does this mean you can actually invoke a varargs method by manually packaging
> the varargs in an array, and passing it as the last argument? Hm... Honestly, I
> find this to not be a terribly good idea. You open up venues for ambiguity,

That's right, but Java varargs are really just an extended form of passing an array as argument. You should be able to use both forms, passing sequence of arguments or an array. Consider for example java.util.Arrays.asList(T...) which in Java 1.4 is defined as asList(Object[]).

Comment 10

[Attila Szegedi]

(In reply to comment #9)
> 
> That's right, but Java varargs are really just an extended form of passing an
> array as argument. You should be able to use both forms, passing sequence of
> arguments or an array. Consider for example java.util.Arrays.asList(T...) which
> in Java 1.4 is defined as asList(Object[]).
> 

Ouch. I really wish they didn't do that. Anyway, it turns out my reading of the JLS 15.12.2 at <http://java.sun.com/docs/books/jls/third_edition/html/expressions.html#292575> was wrong; I thought that the process described in 15.12.2.2 will only consider fixed-arity methods, but it indeed considers those declared as variable arity too, only it treats them as being fixed-arity for purposes of matching -- that's why they could change asList() method signature and still remain backwards compatible... Well, I guess this means your way of handling this should indeed be considered correct :-)

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Closing. Bug management is now done here:
https://github.com/mozilla/rhino