Fixed ScriptRuntime.toPrimitive() and use it for toXXX() (taken from #1611 done by @tonygermano)

rhino/src/main/java/org/mozilla/javascript/NativeArray.java

@@ -1947,10 +1947,9 @@ private static Object js_lastIndexOf(
     */
     private static Boolean js_includes(
             Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
-        Object compareTo = args.length > 0 ? args[0] : Undefined.instance;
 
         Scriptable o = ScriptRuntime.toObject(cx, scope, thisObj);
-        long len = ScriptRuntime.toLength(new Object[] {getProperty(thisObj, "length")}, 0);
+        long len = getLengthProperty(cx, o);
         if (len == 0) return Boolean.FALSE;
 
         long k;
@@ -1964,6 +1963,8 @@ private static Boolean js_includes(
             }
             if (k > len - 1) return Boolean.FALSE;
         }
+
+        Object compareTo = args.length > 0 ? args[0] : Undefined.instance;
         if (o instanceof NativeArray) {
             NativeArray na = (NativeArray) o;
             if (na.denseOnly) {

rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java

@@ -436,15 +436,10 @@ public static double toNumber(Object val) {
             if (val instanceof String) return toNumber((String) val);
             if (val instanceof CharSequence) return toNumber(val.toString());
             if (val instanceof Boolean) return ((Boolean) val).booleanValue() ? 1 : +0.0;
-            if (val instanceof Symbol) throw typeErrorById("msg.not.a.number");
-            if (val instanceof Scriptable) {
-                val = ((Scriptable) val).getDefaultValue(NumberClass);
-                if ((val instanceof Scriptable) && !isSymbol(val))
-                    throw errorWithClassName("msg.primitive.expected", val);
-                continue;
-            }
-            warnAboutNonJSObject(val);
-            return NaN;
+            if (isSymbol(val)) throw typeErrorById("msg.not.a.number");
+            // Assert: val is an Object
+            val = toPrimitive(val, NumberClass);
+            // Assert: val is a primitive
         }
     }
 
@@ -713,56 +708,45 @@ public static double toNumber(String s) {
 
     /** Convert the value to a BigInt. */
     public static BigInteger toBigInt(Object val) {
-        for (; ; ) {
-            if (val instanceof BigInteger) {
-                return (BigInteger) val;
-            }
-            if (val instanceof BigDecimal) {
-                return ((BigDecimal) val).toBigInteger();
-            }
-            if (val instanceof Number) {
-                if (val instanceof Long) {
-                    return BigInteger.valueOf(((Long) val));
-                } else {
-                    double d = ((Number) val).doubleValue();
-                    if (Double.isNaN(d) || Double.isInfinite(d)) {
-                        throw rangeErrorById(
-                                "msg.cant.convert.to.bigint.isnt.integer", toString(val));
-                    }
-                    BigDecimal bd = new BigDecimal(d, MathContext.UNLIMITED);
-                    try {
-                        return bd.toBigIntegerExact();
-                    } catch (ArithmeticException e) {
-                        throw rangeErrorById(
-                                "msg.cant.convert.to.bigint.isnt.integer", toString(val));
-                    }
+        val = toPrimitive(val, NumberClass);
+        if (val instanceof BigInteger) {
+            return (BigInteger) val;
+        }
+        if (val instanceof BigDecimal) {
+            return ((BigDecimal) val).toBigInteger();
+        }
+        if (val instanceof Number) {
+            if (val instanceof Long) {
+                return BigInteger.valueOf(((Long) val));
+            } else {
+                double d = ((Number) val).doubleValue();
+                if (Double.isNaN(d) || Double.isInfinite(d)) {
+                    throw rangeErrorById("msg.cant.convert.to.bigint.isnt.integer", toString(val));
                 }
-            }
-            if (val == null || Undefined.isUndefined(val)) {
-                throw typeErrorById("msg.cant.convert.to.bigint", toString(val));
-            }
-            if (val instanceof String) {
-                return toBigInt((String) val);
-            }
-            if (val instanceof CharSequence) {
-                return toBigInt(val.toString());
-            }
-            if (val instanceof Boolean) {
-                return ((Boolean) val).booleanValue() ? BigInteger.ONE : BigInteger.ZERO;
-            }
-            if (val instanceof Symbol) {
-                throw typeErrorById("msg.cant.convert.to.bigint", toString(val));
-            }
-            if (val instanceof Scriptable) {
-                val = ((Scriptable) val).getDefaultValue(BigIntegerClass);
-                if ((val instanceof Scriptable) && !isSymbol(val)) {
-                    throw errorWithClassName("msg.primitive.expected", val);
+                BigDecimal bd = new BigDecimal(d, MathContext.UNLIMITED);
+                try {
+                    return bd.toBigIntegerExact();
+                } catch (ArithmeticException e) {
+                    throw rangeErrorById("msg.cant.convert.to.bigint.isnt.integer", toString(val));
                 }
-                continue;
             }
-            warnAboutNonJSObject(val);
-            return BigInteger.ZERO;
         }
+        if (val == null || Undefined.isUndefined(val)) {
+            throw typeErrorById("msg.cant.convert.to.bigint", toString(val));
+        }
+        if (val instanceof String) {
+            return toBigInt((String) val);
+        }
+        if (val instanceof CharSequence) {
+            return toBigInt(val.toString());
+        }
+        if (val instanceof Boolean) {
+            return ((Boolean) val).booleanValue() ? BigInteger.ONE : BigInteger.ZERO;
+        }
+        if (isSymbol(val)) {
+            throw typeErrorById("msg.cant.convert.to.bigint", toString(val));
+        }
+        throw errorWithClassName("msg.primitive.expected", val);
     }
 
     /** ToBigInt applied to the String type */
@@ -841,6 +825,7 @@ public static BigInteger toBigInt(String s) {
      * <p>See ECMA 7.1.3 (v11.0).
      */
     public static Number toNumeric(Object val) {
+        val = toPrimitive(val, NumberClass);
         if (val instanceof Number) {
             return (Number) val;
         }
@@ -1027,24 +1012,22 @@ public static String toString(Object val) {
                 return val.toString();
             }
             if (val instanceof BigInteger) {
-                return val.toString();
+                return ((BigInteger) val).toString(10);
             }
             if (val instanceof Number) {
                 // XXX should we just teach NativeNumber.stringValue()
                 // about Numbers?
                 return numberToString(((Number) val).doubleValue(), 10);
             }
-            if (val instanceof Symbol) {
-                throw typeErrorById("msg.not.a.string");
+            if (val instanceof Boolean) {
+                return val.toString();
             }
-            if (val instanceof Scriptable) {
-                val = ((Scriptable) val).getDefaultValue(StringClass);
-                if ((val instanceof Scriptable) && !isSymbol(val)) {
-                    throw errorWithClassName("msg.primitive.expected", val);
-                }
-                continue;
+            if (isSymbol(val)) {
+                throw typeErrorById("msg.not.a.string");
             }
-            return val.toString();
+            // Assert: val is an Object
+            val = toPrimitive(val, StringClass);
+            // Assert: val is a primitive
         }
     }
 
@@ -3552,25 +3535,48 @@ public static Object toPrimitive(Object input) {
     }
 
     /**
-     * 1. If input is an Object, then a. Let exoticToPrim be ? GetMethod(input, @@toPrimitive). b.
-     * If exoticToPrim is not undefined, then i. If preferredType is not present, then 1. Let hint
-     * be "default". ii. Else if preferredType is string, then 1. Let hint be "string". iii. Else,
-     * 1. Assert: preferredType is number. 2. Let hint be "number". iv. Let result be ?
-     * Call(exoticToPrim, input, « hint »). v. If result is not an Object, return result. vi. Throw
-     * a TypeError exception. c. If preferredType is not present, let preferredType be number. d.
-     * Return ? OrdinaryToPrimitive(input, preferredType). 2. Return input.
+     * The abstract operation ToPrimitive takes argument input (an ECMAScript language value) and
+     * optional argument preferredType (string or number) and returns either a normal completion
+     * containing an ECMAScript language value or a throw completion. It converts its input argument
+     * to a non-Object type. If an object is capable of converting to more than one primitive type,
+     * it may use the optional hint preferredType to favour that type.
      *
      * @param input
      * @param preferredType
      * @return
      * @see <a href="https://262.ecma-international.org/15.0/index.html#sec-toprimitive"></a>
      */
     public static Object toPrimitive(Object input, Class<?> preferredType) {
-        if (!isObject(input)) {
+        // 1. If input is an Object, then
+        //    a. Let exoticToPrim be ? GetMethod(input, @@toPrimitive).
+        //    b. If exoticToPrim is not undefined, then
+        //        i. If preferredType is not present, then
+        //            1. Let hint be "default".
+        //        ii. Else if preferredType is string, then
+        //            1. Let hint be "string".
+        //        iii. Else,
+        //            1. Assert: preferredType is number.
+        //            2. Let hint be "number".
+        //        iv. Let result be ? Call(exoticToPrim, input, « hint »).
+        //        v.  If result is not an Object, return result.
+        //        vi. Throw a TypeError exception.
+        //    c. If preferredType is not present, let preferredType be number.
+        //    d. Return ? OrdinaryToPrimitive(input, preferredType).
+        // 2. Return input.
+
+        // do not return on Scriptable's here; we like to fall back to our
+        // default impl getDefaultValue() for them
+        if (!(input instanceof Scriptable) && !isObject(input)) {
             return input;
         }
+
         final Scriptable s = (Scriptable) input;
-        final Object exoticToPrim = ScriptableObject.getProperty(s, SymbolKey.TO_PRIMITIVE);
+        // to be backward compatible: getProperty(Scriptable obj, Symbol key)
+        // throws if obj is not a SymbolScriptable
+        Object exoticToPrim = null;
+        if (s instanceof SymbolScriptable) {
+            exoticToPrim = ScriptableObject.getProperty(s, SymbolKey.TO_PRIMITIVE);
+        }
         if (exoticToPrim instanceof Function) {
             final Function func = (Function) exoticToPrim;
             final Context cx = Context.getCurrentContext();
@@ -3589,10 +3595,12 @@ public static Object toPrimitive(Object input, Class<?> preferredType) {
             }
             return result;
         }
-        if (!Undefined.isUndefined(exoticToPrim) && exoticToPrim != Scriptable.NOT_FOUND) {
+        if (exoticToPrim != null
+                && exoticToPrim != Scriptable.NOT_FOUND
+                && !Undefined.isUndefined(exoticToPrim)) {
             throw notFunctionError(exoticToPrim);
         }
-        final Class<?> defaultValueHint = preferredType == null ? preferredType : NumberClass;
+        final Class<?> defaultValueHint = preferredType == null ? NumberClass : preferredType;
         final Object result = s.getDefaultValue(defaultValueHint);
         if ((result instanceof Scriptable) && !isSymbol(result))
             throw typeErrorById("msg.bad.default.value");

rhino/src/test/java/org/mozilla/javascript/tests/IterableTest.java

@@ -288,6 +288,9 @@ public SymbolFooBoilerplate(final Scriptable scope) {
 
         @Override
         public Object get(Symbol key, Scriptable start) {
+            if (SymbolKey.TO_PRIMITIVE == key) {
+                return null;
+            }
             throw new UnsupportedOperationException(
                     "Not supported yet."); // To change body of generated methods, choose Tools |
             // Templates.

tests/src/test/java/org/mozilla/javascript/tests/es6/NativeArray2Test.java

@@ -1,32 +1,11 @@
 package org.mozilla.javascript.tests.es6;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-
-import org.junit.After;
-import org.junit.Before;
 import org.junit.Test;
-import org.mozilla.javascript.Context;
-import org.mozilla.javascript.ScriptableObject;
+import org.mozilla.javascript.tests.Utils;
 
 /** Test for NativeArray. */
 public class NativeArray2Test {
 
-    private Context cx;
-    private ScriptableObject scope;
-
-    @Before
-    public void setUp() {
-        cx = Context.enter();
-        cx.setLanguageVersion(Context.VERSION_ES6);
-        scope = cx.initStandardObjects();
-    }
-
-    @After
-    public void tearDown() {
-        Context.exit();
-    }
-
     @Test
     public void concatLimitSpreadable() {
         String js =
@@ -39,8 +18,9 @@ public void concatLimitSpreadable() {
                         + " '' + e;\n"
                         + "};";
 
-        String result = (String) cx.evaluateString(scope, js, "test", 1, null);
-        assertTrue(result.endsWith("exceeds supported capacity limit."));
+        Utils.assertWithAllOptimizationLevelsES6(
+                "TypeError: Array length 9,007,199,254,740,992 exceeds supported capacity limit.",
+                js);
     }
 
     @Test
@@ -59,7 +39,6 @@ public void concatLimitSpreadable2() {
                         + " '' + e;\n"
                         + "};";
 
-        String result = (String) cx.evaluateString(scope, js, "test", 1, null);
-        assertEquals(result, "Error: get failed", result);
+        Utils.assertWithAllOptimizationLevelsES6("Error: get failed", js);
     }
 }