Add support for ES6 function notations

[rhendric]

This PR contains two improvements for stringifying functions. The first is support for ensuring that function values produced with the following ES6 function notations are stringified into valid code:

• Arrow functions (a, b) => a + b
• Generators function* (x) { yield x; }
• Method notation { add(a, b) { return a + b; } }

The second, which applies to all functions, improves function body indentation by removing extra indentation from function bodies before stringifying them, where extra indentation is determined by finding the line in the function body (after the first line) with the fewest initial space characters.

[coveralls]

Coverage remained the same at 100.0% when pulling 6be4cd4 on rhendric:es6-function-notations into 9d2b937 on blakeembrey:master.

[blakeembrey]

Just a heads up but startsWith is ES2015. Probably not an issue and will look at updating to ES2015 soon.

[rhendric]

Ah, good catch. (So is repeat, which I also used.)

[rhendric]

PR updated to not use ES6 String methods.

[rhendric]

Hey @blakeembrey, are there any other concerns about this patch?

[blakeembrey]

Sorry for the delay in merging this, I disappeared from OSS for a while. I just tried landing this but it looks like things have changed in serializing function names in the latest node.js release. I'll try tweaking the code a little to rely less on the generated output of toString if I can, let me know if you have any ideas to avoid testing two different code paths on node.js releases.

[rhendric]

Here are some unfortunate conclusions I've arrived at regarding correctness in different node.js versions. Just read the bolded statements if you hate that I wrote a whole essay.

Conclusion 1: For node.js versions between 6 and 8, it is impossible for javascript-stringify to correctly stringify 100% of functions.

Consider the following pair of functions.

f = ({'()=>function'(){}})['()=>function']
g = ({'()=>function':()=>function(){}})['()=>function']

These functions are clearly fundamentally different; f() returns undefined, and g() returns another function. Any attempt to stringify these functions has only two pieces of data to use: the functions' names, and their toStrings. In node.js 4, these functions have different names; f's name is '()=>function' whereas g's name is ''. In node.js 10+, these functions have different toStrings, because the source code used to create them is different. But in node.js 6 and 8, f and g are given identical names and toStrings, and therefore no implementation of javascript-stringify could correctly stringify f to something that, when eval'd, would behave like f, and also do the same for g.

Conclusion 2: Any implementation of javascript-stringify that correctly stringifies 100% of functions on both node.js 4 and node.js 10 must somehow detect which engine is running it to select a code path.

In other words, there exist two different function-valued expressions f and g such that, when f is evaluated on node.js 10 and g is evaluated on node.js 4, the names and toStrings of the two functions are identical. Therefore, as in Conclusion 1, if javascript-stringify only uses the names and toStrings to stringify functions, and not any other information about its environment, it would have to stringify both f and g to the same string, which can't represent both f and g accurately.

Actually constructing f and g is complicated. There may exist other ways to do it, but I used computed properties. First, consider that node.js 4's toString concatenates the name and body of the function, whereas node.js 10 concatenates the definition of the name with the body. In general, the definition will not be equal to the name; but one can construct a tricky expression that evaluates to itself, plus an extra suffix. If that suffix, when prepended to the body of the g function, produces the body of the f function, then the toStrings of f and g will be equal:

f.toString() = nameDefinition + fBody
             = nameDefinition + suffix + gBody
             = eval(nameDefinition) + gBody
             = name + gBody
             = g.toString()

Working out a suffix is a relatively simple trick with block comments. Working out a computed property that evaluates to itself plus the suffix is basically constructing a quine, which can be done pretty mechanically although the results aren't pretty. See below:

In node.js 10:

o = {
  [((s,a,b)=>a+s(a)+","+s(b)+b)(JSON.stringify,"[((s,a,b)=>a+s(a)+\",\"+s(b)+b)(JSON.stringify,",")]() { return 0; /*")]() { return 0; /*() {/* */ return 1;}
};
f = o['[((s,a,b)=>a+s(a)+","+s(b)+b)(JSON.stringify,"[((s,a,b)=>a+s(a)+\\",\\"+s(b)+b)(JSON.stringify,",")]() { return 0; /*")]() { return 0; /*'];

And in node.js 4:

o = {
  '[((s,a,b)=>a+s(a)+","+s(b)+b)(JSON.stringify,"[((s,a,b)=>a+s(a)+\\",\\"+s(b)+b)(JSON.stringify,",")]() { return 0; /*")]() { return 0; /*'() {/* */ return 1;}
};
g = o['[((s,a,b)=>a+s(a)+","+s(b)+b)(JSON.stringify,"[((s,a,b)=>a+s(a)+\\",\\"+s(b)+b)(JSON.stringify,",")]() { return 0; /*")]() { return 0; /*'];

If you run these snippets in their respective engines, you'll see that both f and g have a name of '[((s,a,b)=>a+s(a)+","+s(b)+b)(JSON.stringify,"[((s,a,b)=>a+s(a)+\\",\\"+s(b)+b)(JSON.stringify,",")]() { return 0; /*")]() { return 0; /*' and a toString of '[((s,a,b)=>a+s(a)+","+s(b)+b)(JSON.stringify,"[((s,a,b)=>a+s(a)+\\",\\"+s(b)+b)(JSON.stringify,",")]() { return 0; /*")]() { return 0; /*() {/* */ return 1;}'. But f() is 0 and g() is 1.

This is especially sad because the only options I can think of for detecting whether methods are toString'd with their names or the definitions of their names are:

• define a method, which isn't valid in pre-ES6 environments 😦
• use eval to define a method, which can cause problems in environments like Atom packages and websites with a restrictive CSP 😟
• check the node.js version directly 😞

Do you have thoughts on any of the above?

[blakeembrey]

It sounds like the best approach is to substring the prefix matching against fn.name, "${fn.name}" and '${fn.name}' (similar to before but with two quoted additions). We'll always need to substring and change the name to the computed key in object definition or regular function definition elsewhere - there doesn't seem to be any way we use fn.name for method definition syntax in javascript-stringify. This will just be a caveat we mention in the README. Does this sound like a reasonable approach for, at least, the latest node.js version?

I haven't quite grokked the difference between the two o results and why they have the same fn.name yet. Seems like it might be a bug in the updated toString() output.

[rhendric]

To be continued in #22...