Merge pull request #1139 from svaarala/es6-function-tostring

Change Function.prototype.toString() to be ES6 compatible

RELEASES.rst

@@ -2000,6 +2000,10 @@ Planned
   bound function internal prototype is copied from the target function
   instead of always being Function.prototype (GH-1135)
 
+* Change Function.prototype.toString() output to match ES6 requirements;
+  the output no longer parses with eval() but causes a SyntaxError instead
+  (GH-1141)
+
 * Add a fastint check for duk_put_number_list() values (GH-1086)
 
 * Remove an unintended fastint downgrade check for unary minus executor

doc/release-notes-v2-0.rst

@@ -1062,6 +1062,10 @@ Other incompatible changes
   ES6 requirements; in ES5 (and Duktape 1.x) bound function internal prototype
   is always set to Function.prototype.
 
+* Function.prototype.toString() output has been changed to match ES6
+  requirements.  For example ``function foo() {"ecmascript"}`` is now
+  ``function foo() { [ecmascript code] }``.
+
 Known issues
 ============
 

src-input/duk_api_stack.c

@@ -4843,7 +4843,7 @@ DUK_INTERNAL void duk_push_lightfunc_tostring(duk_context *ctx, duk_tval *tv) {
 
 	duk_push_string(ctx, "function ");
 	duk_push_lightfunc_name_raw(ctx, func, lf_flags);
-	duk_push_string(ctx, "() {\"light\"}");
+	duk_push_string(ctx, "() { [lightfunc code] }");
 	duk_concat(ctx, 3);
 }
 

src-input/duk_bi_function.c

@@ -95,35 +95,38 @@ DUK_INTERNAL duk_ret_t duk_bi_function_prototype_to_string(duk_context *ctx) {
 
 	/*
 	 *  E5 Section 15.3.4.2 places few requirements on the output of
-	 *  this function:
+	 *  this function: the result is implementation dependent, must
+	 *  follow FunctionDeclaration syntax (in particular, must have a
+	 *  name even for anonymous functions or functions with empty name).
+	 *  The output does NOT need to compile into anything useful.
 	 *
-	 *    - The result is an implementation dependent representation
-	 *      of the function; in particular
+	 *  E6 Section 19.2.3.5 changes the requirements completely: the
+	 *  result must either eval() to a functionally equivalent object
+	 *  OR eval() to a SyntaxError.
 	 *
-	 *    - The result must follow the syntax of a FunctionDeclaration.
-	 *      In particular, the function must have a name (even in the
-	 *      case of an anonymous function or a function with an empty
-	 *      name).
+	 *  We opt for the SyntaxError approach for now, with a syntax that
+	 *  mimics V8's native function syntax:
 	 *
-	 *    - Note in particular that the output does NOT need to compile
-	 *      into anything useful.
+	 *      'function cos() { [native code] }'
+	 *
+	 *  but extended with [ecmascript code], [bound code], and
+	 *  [lightfunc code].
 	 */
 
-
-	/* XXX: faster internal way to get this */
 	duk_push_this(ctx);
-	tv = duk_get_tval(ctx, -1);
+	tv = DUK_GET_TVAL_NEGIDX(ctx, -1);
 	DUK_ASSERT(tv != NULL);
 
 	if (DUK_TVAL_IS_OBJECT(tv)) {
 		duk_hobject *obj = DUK_TVAL_GET_OBJECT(tv);
 		const char *func_name;
 
 		/* Function name: missing/undefined is mapped to empty string,
-		 * otherwise coerce to string.
-		 */
-		/* XXX: currently no handling for non-allowed identifier characters,
-		 * e.g. a '{' in the function name.
+		 * otherwise coerce to string.  No handling for invalid identifier
+		 * characters or e.g. '{' in the function name.  This doesn't
+		 * really matter as long as a SyntaxError results.  Technically
+		 * if the name contained a suitable prefix followed by '//' it
+		 * might cause the result to parse without error.
 		 */
 		duk_get_prop_stridx(ctx, -1, DUK_STRIDX_NAME);
 		if (duk_is_undefined(ctx, -1)) {
@@ -133,15 +136,12 @@ DUK_INTERNAL duk_ret_t duk_bi_function_prototype_to_string(duk_context *ctx) {
 			DUK_ASSERT(func_name != NULL);
 		}
 
-		/* Indicate function type in the function body using a dummy
-		 * directive.
-		 */
 		if (DUK_HOBJECT_IS_COMPFUNC(obj)) {
-			duk_push_sprintf(ctx, "function %s() {\"ecmascript\"}", (const char *) func_name);
+			duk_push_sprintf(ctx, "function %s() { [ecmascript code] }", (const char *) func_name);
 		} else if (DUK_HOBJECT_IS_NATFUNC(obj)) {
-			duk_push_sprintf(ctx, "function %s() {\"native\"}", (const char *) func_name);
+			duk_push_sprintf(ctx, "function %s() { [native code] }", (const char *) func_name);
 		} else if (DUK_HOBJECT_IS_BOUNDFUNC(obj)) {
-			duk_push_sprintf(ctx, "function %s() {\"bound\"}", (const char *) func_name);
+			duk_push_sprintf(ctx, "function %s() { [bound code] }", (const char *) func_name);
 		} else {
 			goto type_error;
 		}

tests/api/test-dev-api-verbose-error-messages-gh441.c

@@ -120,14 +120,14 @@ TypeError: buffer required, found [object Function] (stack index -3)
 TypeError: pointer required, found [object Function] (stack index -3)
 test__c_function ok
 top: 1
-TypeError: undefined required, found function LFUNC() {"light"} (stack index -3)
-TypeError: null required, found function LFUNC() {"light"} (stack index -3)
-TypeError: boolean required, found function LFUNC() {"light"} (stack index -3)
-TypeError: number required, found function LFUNC() {"light"} (stack index -3)
-TypeError: string required, found function LFUNC() {"light"} (stack index -3)
-TypeError: buffer required, found function LFUNC() {"light"} (stack index -3)
-TypeError: pointer required, found function LFUNC() {"light"} (stack index -3)
-TypeError: nativefunction required, found function LFUNC() {"light"} (stack index -3)
+TypeError: undefined required, found function LFUNC() { [lightfunc code] } (stack index -3)
+TypeError: null required, found function LFUNC() { [lightfunc code] } (stack index -3)
+TypeError: boolean required, found function LFUNC() { [lightfunc code] } (stack index -3)
+TypeError: number required, found function LFUNC() { [lightfunc code] } (stack index -3)
+TypeError: string required, found function LFUNC() { [lightfunc code] } (stack index -3)
+TypeError: buffer required, found function LFUNC() { [lightfunc code] } (stack index -3)
+TypeError: pointer required, found function LFUNC() { [lightfunc code] } (stack index -3)
+TypeError: nativefunction required, found function LFUNC() { [lightfunc code] } (stack index -3)
 top: 1
 TypeError: undefined required, found [object Function] (stack index -3)
 TypeError: null required, found [object Function] (stack index -3)

tests/api/test-dev-func-tostring.c

@@ -1,13 +1,13 @@
 /*
- *  Test the updated Function .toString() format in Duktape 1.5.0.
+ *  Test the current Function .toString() format.
  *
  *  Cover a few cases which cannot be exercised using Ecmascript code alone.
  */
 
 /*===
 *** test_1 (duk_safe_call)
-function light_PTR() {"light"}
-function dummy {() {"native"}
+function light_PTR() { [lightfunc code] }
+function dummy {() { [native code] }
 final top: 0
 ==> rc=0, result='undefined'
 ===*/

tests/api/test-dev-lightfunc.c

@@ -928,7 +928,7 @@ static duk_ret_t test_to_object(duk_context *ctx, void *udata) {
 
 /*===
 *** test_to_buffer (duk_safe_call)
-function light_PTR_4232() {"light"}
+function light_PTR_4232() { [lightfunc code] }
 final top: 1
 ==> rc=0, result='undefined'
 ===*/

tests/api/test-dev-plain-buffer.c

@@ -144,15 +144,15 @@ flag index: 2, top: 2
 - byteLength: 16
 - byteOffset: 0
 - BYTES_PER_ELEMENT: 1
-- constructor: function ArrayBuffer() {"native"}
-- slice: function slice() {"native"}
+- constructor: function ArrayBuffer() { [native code] }
+- slice: function slice() { [native code] }
 - __proto__: [object Object]
-- toString: function toString() {"native"}
-- toLocaleString: function toLocaleString() {"native"}
-- valueOf: function valueOf() {"native"}
-- hasOwnProperty: function hasOwnProperty() {"native"}
-- isPrototypeOf: function isPrototypeOf() {"native"}
-- propertyIsEnumerable: function propertyIsEnumerable() {"native"}
+- toString: function toString() { [native code] }
+- toLocaleString: function toLocaleString() { [native code] }
+- valueOf: function valueOf() { [native code] }
+- hasOwnProperty: function hasOwnProperty() { [native code] }
+- isPrototypeOf: function isPrototypeOf() { [native code] }
+- propertyIsEnumerable: function propertyIsEnumerable() { [native code] }
 flag index: 3, top: 2
 - 0: 100
 - 1: 101

tests/api/test-push-object.c

@@ -1,6 +1,6 @@
 /*===
 duk_is_object(1) = 1
-.toString rc=1 -> function toString() {"native"}
+.toString rc=1 -> function toString() { [native code] }
 json encoded: {"meaningOfLife":42}
 top=2
 ===*/

tests/ecmascript/test-bi-function-proto-tostring-custom.js

@@ -0,0 +1,36 @@
+/*
+ *  Duktape custom behavior for Function.prototype.toString().
+ */
+
+/*---
+{
+    "custom": true
+}
+---*/
+
+/*===
+function foo() { [ecmascript code] }
+function cos() { [native code] }
+function bound foo() { [bound code] }
+function bound cos() { [bound code] }
+===*/
+
+function test() {
+    var scriptFunc = function foo() {};
+    var nativeFunc = Math.cos;
+    var boundFunc1 = scriptFunc.bind(null, 1);
+    var boundFunc2 = nativeFunc.bind(null, 1);
+
+    [ scriptFunc, nativeFunc, boundFunc1, boundFunc2 ].forEach(function (v) {
+        print(String(v));
+    });
+
+    // Lightfunc and some other cases are covered by
+    // tests/api/test-dev-func-tostring.c.
+}
+
+try {
+    test();
+} catch (e) {
+    print(e.stack || e);
+}

tests/ecmascript/test-bi-function-proto-tostring.js

@@ -0,0 +1,39 @@
+/*
+ *  Function.prototype.toString() requirements were quite general in ES5:
+ *  output must parse as FunctionDeclaration but doesn't need to compile
+ *  into useful code.
+ *
+ *  ES6 requires that the source code eval()s to an equivalent object or,
+ *  if that's not possible, evals to a SyntaxError.
+ *
+ *  Duktape 2.x follows the ES6 requirements.
+ */
+
+/*===
+SyntaxError
+SyntaxError
+SyntaxError
+SyntaxError
+===*/
+
+function test() {
+    var scriptFunc = function foo() {};
+    var nativeFunc = Math.cos;
+    var boundFunc1 = scriptFunc.bind(null, 1);
+    var boundFunc2 = nativeFunc.bind(null, 1);
+
+    [ scriptFunc, nativeFunc, boundFunc1, boundFunc2 ].forEach(function (v) {
+        try {
+            var res = eval(String(v));
+            print('never here:', typeof res);
+        } catch (e) {
+            print(e.name);
+        }
+    });
+}
+
+try {
+    test();
+} catch (e) {
+    print(e.stack || e);
+}

tests/ecmascript/test-bi-reflect-arg-policy.js

@@ -45,7 +45,7 @@ true
 true
 812
 [object Object]
-function () {"native"}
+function () { [native code] }
 false
 true
 name,fileName,length,prototype,test

tests/ecmascript/test-conv-tostring.js

@@ -18,7 +18,7 @@
 3 string false
 4 string 123
 5 string foo
-6 string function myfunc() {"ecmascript"}
+6 string function myfunc() { [ecmascript code] }
 7 string [object Object]
 8 TypeError
 9 string foo