Bug 557371 - Make JSON.stringify behavior on Boolean, String, and Number objects to-spec. r=jorendorff

--HG--
extra : rebase_source : f329e9274b79abd264cf254d6d95a1b97f466ab9

js/src/jsnum.cpp

@@ -944,7 +944,6 @@ static JSFunctionSpec number_methods[] = {
     JS_TN(js_toString_str,       num_toString,          1, 0, &num_toString_trcinfo),
     JS_FN(js_toLocaleString_str, num_toLocaleString,    0, 0),
     JS_FN(js_valueOf_str,        js_num_valueOf,        0, 0),
-    JS_FN(js_toJSON_str,         js_num_valueOf,        0, 0),
     JS_FN("toFixed",             num_toFixed,           1, 0),
     JS_FN("toExponential",       num_toExponential,     1, 0),
     JS_FN("toPrecision",         num_toPrecision,       1, 0),

js/src/json.cpp

@@ -490,43 +490,66 @@ CallReplacerFunction(JSContext *cx, jsid id, JSObject *holder, StringifyContext
     return JS_TRUE;
 }
 
+/* ES5 15.12.3 Str. */
 static JSBool
 Str(JSContext *cx, jsid id, JSObject *holder, StringifyContext *scx, Value *vp, bool callReplacer)
 {
-    JS_CHECK_RECURSION(cx, return JS_FALSE);
+    JS_CHECK_RECURSION(cx, return false);
 
+    /*
+     * This method implements the Str algorithm in ES5 15.12.3, but we move
+     * property retrieval into the caller to stream the stringification process
+     * and avoid constantly copying strings.
+     */
+
+    /* Step 2. */
     if (vp->isObject() && !js_TryJSON(cx, vp))
-        return JS_FALSE;
+        return false;
 
+    /* Step 3. */
     if (callReplacer && !CallReplacerFunction(cx, id, holder, scx, vp))
-        return JS_FALSE;
+        return false;
 
-    // catches string and number objects with no toJSON
+    /* Step 4. */
     if (vp->isObject()) {
         JSObject *obj = &vp->toObject();
         Class *clasp = obj->getClass();
-        if (clasp == &js_StringClass || clasp == &js_NumberClass)
+        if (clasp == &js_NumberClass) {
+            double d;
+            if (!ValueToNumber(cx, *vp, &d))
+                return false;
+            vp->setNumber(d);
+        } else if (clasp == &js_StringClass) {
+            JSString *str = js_ValueToString(cx, *vp);
+            if (!str)
+                return false;
+            vp->setString(str);
+        } else if (clasp == &js_BooleanClass) {
             *vp = obj->getPrimitiveThis();
         }
+    }
 
+    /* Step 8. */
     if (vp->isString()) {
         JSString *str = vp->toString();
         size_t length = str->length();
         const jschar *chars = str->getChars(cx);
         if (!chars)
-            return JS_FALSE;
+            return false;
         return write_string(cx, scx->cb, chars, length);
     }
 
-    if (vp->isNull()) {
+    /* Step 5. */
+    if (vp->isNull())
         return js_AppendLiteral(scx->cb, "null");
-    }
 
+    /* Steps 6-7. */
     if (vp->isBoolean()) {
         return vp->toBoolean() ? js_AppendLiteral(scx->cb, "true")
                                : js_AppendLiteral(scx->cb, "false");
     }
 
+    /* Step 9. */
     if (vp->isNumber()) {
         if (vp->isDouble()) {
             jsdouble d = vp->toDouble();
@@ -536,11 +559,12 @@ Str(JSContext *cx, jsid id, JSObject *holder, StringifyContext *scx, Value *vp,
 
         JSCharBuffer cb(cx);
         if (!js_NumberValueToCharBuffer(cx, *vp, cb))
-            return JS_FALSE;
+            return false;
 
         return scx->cb.append(cb.begin(), cb.length());
     }
 
+    /* Step 10. */
     if (vp->isObject() && !IsFunctionObject(*vp) && !IsXML(*vp)) {
         JSBool ok;
 
@@ -551,8 +575,9 @@ Str(JSContext *cx, jsid id, JSObject *holder, StringifyContext *scx, Value *vp,
         return ok;
     }
 
+    /* Step 11. */
     vp->setUndefined();
-    return JS_TRUE;
+    return true;
 }
 
 JSBool

js/src/jsstr.cpp

@@ -3062,7 +3062,6 @@ static JSFunctionSpec string_methods[] = {
     /* Java-like methods. */
     JS_FN(js_toString_str,     js_str_toString,       0,0),
     JS_FN(js_valueOf_str,      js_str_toString,       0,0),
-    JS_FN(js_toJSON_str,       js_str_toString,       0,0),
     JS_FN("substring",         str_substring,         2,JSFUN_GENERIC_NATIVE),
     JS_FN("toLowerCase",       str_toLowerCase,       0,JSFUN_GENERIC_NATIVE),
     JS_FN("toUpperCase",       str_toUpperCase,       0,JSFUN_GENERIC_NATIVE),

js/src/tests/ecma_5/JSON/jstests.list

@@ -3,3 +3,4 @@ script cyclic-stringify.js
 script small-codepoints.js
 script trailing-comma.js
 script stringify-gap.js
+script stringify-boxed-primitives.js

js/src/tests/ecma_5/JSON/stringify-boxed-primitives.js

@@ -0,0 +1,127 @@
+// Any copyright is dedicated to the Public Domain.
+// http://creativecommons.org/licenses/publicdomain/
+
+var gTestfile = 'stringify-boxed-primitives.js';
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 584909;
+var summary = "Stringification of Boolean/String/Number objects";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+function redefine(obj, prop, fun)
+{
+  var desc =
+    { value: fun, writable: true, configurable: true, enumerable: false };
+  Object.defineProperty(obj, prop, desc);
+}
+
+assertEq(JSON.stringify(new Boolean(false)), "false");
+
+assertEq(JSON.stringify(new Number(5)), "5");
+
+assertEq(JSON.stringify(new String("foopy")), '"foopy"');
+
+
+var numToString = Number.prototype.toString;
+var numValueOf = Number.prototype.valueOf;
+var objToString = Object.prototype.toString;
+var objValueOf = Object.prototype.valueOf;
+var boolToString = Boolean.prototype.toString;
+var boolValueOf = Boolean.prototype.valueOf;
+
+redefine(Boolean.prototype, "toString", function() { return 17; });
+assertEq(JSON.stringify(new Boolean(false)), "false")
+delete Boolean.prototype.toString;
+assertEq(JSON.stringify(new Boolean(false)), "false");
+delete Object.prototype.toString;
+assertEq(JSON.stringify(new Boolean(false)), "false");
+delete Boolean.prototype.valueOf;
+assertEq(JSON.stringify(new Boolean(false)), "false");
+delete Object.prototype.valueOf;
+assertEq(JSON.stringify(new Boolean(false)), "false");
+
+
+redefine(Boolean.prototype, "toString", boolToString);
+redefine(Boolean.prototype, "valueOf", boolValueOf);
+redefine(Object.prototype, "toString", objToString);
+redefine(Object.prototype, "valueOf", objValueOf);
+
+redefine(Number.prototype, "toString", function() { return 42; });
+assertEq(JSON.stringify(new Number(5)), "5");
+redefine(Number.prototype, "valueOf", function() { return 17; });
+assertEq(JSON.stringify(new Number(5)), "17");
+delete Number.prototype.toString;
+assertEq(JSON.stringify(new Number(5)), "17");
+delete Number.prototype.valueOf;
+assertEq(JSON.stringify(new Number(5)), "null"); // isNaN(Number("[object Number]"))
+delete Object.prototype.toString;
+try
+{
+  JSON.stringify(new Number(5));
+  throw new Error("didn't throw");
+}
+catch (e)
+{
+  assertEq(e instanceof TypeError, true,
+           "ToNumber failure, should throw TypeError");
+}
+delete Object.prototype.valueOf;
+try
+{
+  JSON.stringify(new Number(5));
+  throw new Error("didn't throw");
+}
+catch (e)
+{
+  assertEq(e instanceof TypeError, true,
+           "ToNumber failure, should throw TypeError");
+}
+
+
+redefine(Number.prototype, "toString", numToString);
+redefine(Number.prototype, "valueOf", numValueOf);
+redefine(Object.prototype, "toString", objToString);
+redefine(Object.prototype, "valueOf", objValueOf);
+
+
+redefine(String.prototype, "valueOf", function() { return 17; });
+assertEq(JSON.stringify(new String(5)), '"5"');
+redefine(String.prototype, "toString", function() { return 42; });
+assertEq(JSON.stringify(new String(5)), '"42"');
+delete String.prototype.toString;
+assertEq(JSON.stringify(new String(5)), '"[object String]"');
+delete Object.prototype.toString;
+assertEq(JSON.stringify(new String(5)), '"17"');
+delete String.prototype.valueOf;
+try
+{
+  JSON.stringify(new String(5));
+  throw new Error("didn't throw");
+}
+catch (e)
+{
+  assertEq(e instanceof TypeError, true,
+           "ToString failure, should throw TypeError");
+}
+delete Object.prototype.valueOf;
+try
+{
+  JSON.stringify(new String(5));
+  throw new Error("didn't throw");
+}
+catch (e)
+{
+  assertEq(e instanceof TypeError, true,
+           "ToString failure, should throw TypeError");
+}
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("All tests passed!");