Coercing undefined to java Object

[tonygermano]

Is there a reason why when coercing undefined to a java object it becomes the string "undefined" rather than null?

This is very confusing and unexpected, for example, when placing it into a Collection or Map,

var value;
var list = new java.util.ArrayList();
list.add(value);
list.get(0) === undefined; // false
list.get(0) == null; // false
list.get(0) == 'undefined'; // true
list.get(0).getClass().toString() == 'class java.lang.String'; // true

This also means the the following won't work as expected

var value;
var optional = java.util.Optional.ofNullable(value);
optional.isPresent(); // true

Yet it works correctly with null

var value = null;
var optional = java.util.Optional.ofNullable(value);
optional.isPresent(); // false

It defeats the purpose of using an Optional if you are required to check for undefined before creating it.

If there's a reason for the current behavior, perhaps it could be altered with a flag? I wouldn't think this is what most people want.

[tonygermano]

To add to this, JSON.stringify converts undefined to null when found in an array.

[gbrail]

I agree that it's weird to have JSON.stringify convert undefined in an array to null, but JavaScript is full of weird things ;-) In this case, V8 seems to do the same thing so that is usually a good sign that Rhino is following the spec. As for changing the way that conversions to Java are handled, if you can think of a specific change that can be done in a backward-compatible way, or via a flag or a new class, then please open an issue -- small, well-defined issues seem to be things that people on this list can implement. OTOH, there is a gigantic amount of code out there that depends on Rhino Java library behavior going back decades, so a big change in this area without a flag is not something that we can do.

…

On Wed, Sep 16, 2020 at 11:33 AM tonygermano ***@***.***> wrote: To add to this, JSON.stringify converts undefined to null when found in an array. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#760 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAD7I24HWDJAUEXHQDDZJQTSGEAHXANCNFSM4QCVDU6A> .

[tonygermano]

Actually, converting undefined to null when found in an array was the part I thought was normal 😁. I was pointing that out in contrast to undefined becoming a string in Java, which is not expected behavior. In java a declared unset variable is null. In javascript a declared unset variable is undefined. You would think they would be roughly equivalent. Another indicator that they should be is that undefined == null is true in javascript.

This just doesn't seem right.

js> var x;
js> x == null
true
js> var Context = org.mozilla.javascript.Context
js> Context.javaToJS(Context.jsToJava(x, java.lang.Object), this) == null
false
js> Context.javaToJS(Context.jsToJava(x, java.lang.Object), this) == 'undefined'
true
js> var y = null;
js> y == null
true
js> Context.javaToJS(Context.jsToJava(y, java.lang.Object), this) === null
true

I think you would agree that it would be quite odd if this,

js> JSON.stringify([,,1])
[null,null,1]

returned ["undefined", "undefined", 1] instead.

Funnily enough, it appears org.mozilla.javascript.NativeArray#get(long) does convert undefined to null, so it's not even consistent.

js> new java.util.ArrayList([,,1])
[null, null, 1.0]
js> new java.util.ArrayList([,,1]).get(0) === null
true

I understand the need for a flag if someone may be depending on the current behavior. I'd argue that the correct behavior should be converting undefined to null (as JSON.stringify and NativeArray#get both do) and the current behavior should be enabled by the flag.

Where would a flag such as this typically be set? I believe org.mozilla.javascript.NativeJavaObject#coerceTypeImpl(Class, Object) is where the conversion typically happens.

[p-bakker]

Note about the background of this: that JavaScript undefined is converted to the string 'undefined' comes from the LiveConnect 3, see section 3.3.1 of https://www-archive.mozilla.org/js/liveconnect/lc3_method_overloading

(However section 2.3.7 of https://www.oracle.com/java/technologies/javase/liveconnect-docs.html#JS_NULL suggests it ought to be null)

I guess they messed things up a bit back then and we're here to live with the consequenses :-)

[kopfsport]

We had some more Tests with rhino 1.7.14 and found scripts returning "undefined" instead of null. With rhino 1.7.9 we used the sun/oracle phobos RhinoScriptEngine code for integration of rhino and there is this little method called for the return value after a script is evaluated, converting Undefined to null

 Object unwrapReturnValue(Object result) {
    if (result instanceof Wrapper) {
      result = ((Wrapper) result).unwrap();
    }
    return result instanceof Undefined ? null : result;
 }

where now there is a call to Context.jsToJava() calling NativeJavaObject.coerceTypeImpl() and thus returning "undefined".

[tberne]

This issue leads to very strange behaviors.

Like:

Rhino 1.7.14
js> var y;
js> var x = new java.util.HashMap()
js> x.put('yyy', y)
null
js> if(x.get('yyy')) print('Oppsss')
Oppsss
js> x.get('yyy').length()
9

I know it was defined at some point that "undefined" was the best choice, but I believe it leads to misunderstandings.

[p-bakker]

Would be pretty straightforward I think to add a Context flag that would alter the behaviour of NativeJavaObject.coerceTypeImpl() to return the Undefined instance value if type == ScriptRuntime.ObjectClass, instead of returning a "undefined" string.

Or allow to choose between returning "undefined", null or Undefined.instance

I don't think we can change the default behaviour, so some config option it must be. Need to think how users of the Rhino Shell or through the JSR-232 Script Engine interface can opt into these different behaviours

[tonygermano]

When it comes to java interop, I'm of the opinion of evolving the defaults to keep up with the language spec and put the odd legacy behaviors behind the feature flags. The most logical behaviors and common use cases should not be behind feature flags.

js undefined -> java null is the most logical behavior
js undefined -> java String "undefined" is not expected, and should be the option requiring a feature flag for people that need this behavior.

Another example of this that I was planning on bringing up is that a java long should map to a js bigint instead of a number. Obviously, this was not possible when the LiveConnect spec was written, because javascript had no bigint type at the time. As shown here #1410, bigint is now the type that the javascript spec prefers for handling 64-bit integers. There is also discussion here #1502.

Yet another questionable default... why does the rhino shell still use language version 180 by default instead of 200?

I think it's ok to change default behaviors to make the library more usable for the majority of people, and put the onus of making configuration changes on the legacy users (assuming they even upgrade the library regularly.)

[p-bakker]

Got no problem changing the default behaviour of something, for example how undefined values are passed to Java, as long as:

• nobody has valid reasons not to do it
• all active participants on this project are on what the default als be
• there's an option to opt-in to the old behaviour (in the various places like shell, JSR-232 script engine and when embedding Rhino(
• the case/PR it tagged with the potentially breaking change label, so it'll be clearly documented in the (eventual) release notes

With regards to the language version in the shell: see #1279, we just need someone to create the PR to do it

Wrt to long > BigInt mapping: don't think we have an explicit case for this, so maybe create one? And a PR would be great as well 🙂 Dunno if whether that conversion occurs or not ought to be configurable or not and what the default should be, but once we have an actual PR, it'll likely get more people involved in the discussion