From 32c2cc33bbb05a70cd67106afb6913d4517d00de Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Wed, 7 Sep 2016 01:00:07 +0200 Subject: [PATCH] Improve binding patterns for arrow functions --- README.md | 3 ++ lib/ast.js | 49 +++++++++++++++------------- lib/output.js | 39 +++++++++++++++------- lib/parse.js | 15 +++++---- lib/scope.js | 8 +++-- test/compress/arrays.js | 2 ++ test/compress/arrow.js | 36 +++++++++++++++++++++ test/compress/destructuring.js | 18 +++++++++-- test/compress/object.js | 25 +++++++++++++- test/mocha/arrow.js | 39 ++++++++++++++++++++++ test/mocha/function.js | 59 +++++++++++++++++++++++++++++----- test/mocha/object.js | 4 +++ 12 files changed, 243 insertions(+), 54 deletions(-) diff --git a/README.md b/README.md index 4f5b21a0..df7eda44 100644 --- a/README.md +++ b/README.md @@ -499,6 +499,9 @@ can pass additional arguments that control the code output: - `3` -- always use the original quotes - `keep_quoted_props` (default `false`) -- when turned on, prevents stripping quotes from property names in object literals. +- `es` (default `5`) -- set output printing mode. This will only change the + output in direct control of the beautifier. Non-compatible features in the + abstract syntax tree will still be outputted as is. ### Keeping copyright notices or other comments diff --git a/lib/ast.js b/lib/ast.js index 2042332d..68ef352a 100644 --- a/lib/ast.js +++ b/lib/ast.js @@ -394,8 +394,6 @@ var AST_ArrowParametersOrSeq = DEFNODE("ArrowParametersOrSeq", "expressions", { var root = this; return this.expressions.map(function to_fun_args(ex, _, __, default_seen_above) { if (ex instanceof AST_Object) { - if (ex.properties.length == 0) - croak("Invalid destructuring function parameter", ex.start.line, ex.start.col); return new AST_Destructuring({ start: ex.start, end: ex.end, @@ -403,12 +401,16 @@ var AST_ArrowParametersOrSeq = DEFNODE("ArrowParametersOrSeq", "expressions", { default: default_seen_above, names: ex.properties.map(to_fun_args) }); - } else if (ex instanceof AST_ObjectKeyVal && ex.shorthand) { - return new AST_SymbolFunarg({ - name: ex.key, - start: ex.start, - end: ex.end - }); + } else if (ex instanceof AST_ObjectKeyVal || + ex instanceof AST_ObjectComputedKeyVal + ) { + if (ex.key instanceof AST_SymbolRef) { + ex.key = to_fun_args(ex.key, 0, [ex.key], ex.default); + } + ex.value = to_fun_args(ex.value, 0, [ex.key], ex.default); + return ex; + } else if (ex instanceof AST_Hole) { + return ex; } else if (ex instanceof AST_Destructuring) { if (ex.names.length == 0) croak("Invalid destructuring function parameter", ex.start.line, ex.start.col); @@ -424,8 +426,6 @@ var AST_ArrowParametersOrSeq = DEFNODE("ArrowParametersOrSeq", "expressions", { } else if (ex instanceof AST_Expansion) { return ex; } else if (ex instanceof AST_Array) { - if (ex.elements.length === 0) - croak("Invalid destructuring function parameter", ex.start.line, ex.start.col); return new AST_Destructuring({ start: ex.start, end: ex.end, @@ -494,6 +494,11 @@ var AST_Defun = DEFNODE("Defun", null, { /* -----[ DESTRUCTURING ]----- */ var AST_Destructuring = DEFNODE("Destructuring", "names is_array default", { $documentation: "A destructuring of several names. Used in destructuring assignment and with destructuring function argument names", + $propdoc: { + "names": "[AST_Destructuring|AST_Expansion|AST_Hole|AST_ObjectKeyVal|AST_Symbol] Array of properties or elements", + "is_array": "[Boolean] Whether the destructuring represents an object or array", + "default": "[AST_Node?] Default assign value" + }, _walk: function(visitor) { return visitor._visit(this, function(){ this.names.forEach(function(name){ @@ -1001,11 +1006,11 @@ var AST_ObjectProperty = DEFNODE("ObjectProperty", "key value", { } }); -var AST_ObjectKeyVal = DEFNODE("ObjectKeyVal", "quote shorthand", { +var AST_ObjectKeyVal = DEFNODE("ObjectKeyVal", "quote default", { $documentation: "A key: value object property", $propdoc: { quote: "[string] the original quote character", - shorthand: "[boolean] whether this is a shorthand key:value pair, expressed as just the key." + default: "[AST_Expression] The default parameter value, only used when nested inside a binding pattern" } }, AST_ObjectProperty); @@ -1068,13 +1073,19 @@ var AST_ClassExpression = DEFNODE("ClassExpression", null, { $documentation: "A class expression." }, AST_Class); -var AST_Symbol = DEFNODE("Symbol", "scope name thedef", { +var AST_Symbol = DEFNODE("Symbol", "scope name thedef default", { $propdoc: { name: "[string] name of this symbol", scope: "[AST_Scope/S] the current scope (not necessarily the definition scope)", - thedef: "[SymbolDef/S] the definition of this symbol" + thedef: "[SymbolDef/S] the definition of this symbol", + default: "[AST_Expression] The default parameter value, only used when nested inside a binding pattern" }, $documentation: "Base class for all symbols", + _walk: function (visitor) { + return visitor._visit(this, function() { + if (this.default) this.default._walk(visitor); + }); + } }); var AST_NewTarget = DEFNODE("NewTarget", null, { @@ -1085,17 +1096,11 @@ var AST_SymbolAccessor = DEFNODE("SymbolAccessor", null, { $documentation: "The name of a property accessor (setter/getter function)" }, AST_Symbol); -var AST_SymbolDeclaration = DEFNODE("SymbolDeclaration", "init default", { +var AST_SymbolDeclaration = DEFNODE("SymbolDeclaration", "init", { $documentation: "A declaration symbol (symbol in var/const, function name or argument, symbol in catch)", $propdoc: { - init: "[AST_Node*/S] array of initializers for this declaration.", - default: "[AST_Expression] The default for this parameter. For example, `= 6`" + init: "[AST_Node*/S] array of initializers for this declaration." }, - _walk: function (visitor) { - return visitor._visit(this, function() { - if (this.default) this.default._walk(visitor); - }); - } }, AST_Symbol); var AST_SymbolVar = DEFNODE("SymbolVar", null, { diff --git a/lib/output.js b/lib/output.js index cacd5f2f..d176d491 100644 --- a/lib/output.js +++ b/lib/output.js @@ -69,11 +69,16 @@ function OutputStream(options) { preamble : null, quote_style : 0, keep_quoted_props: false, + shorthand : undefined, ecma : 5, }, true); + if (typeof options.ascii_identifiers === 'undefined') options.ascii_identifiers = options.ascii_only; + if (options.shorthand === undefined) + options.shorthand = options.ecma > 5; + var indentation = 0; var current_col = 0; var current_line = 1; @@ -728,9 +733,15 @@ function OutputStream(options) { DEFPRINT(AST_Destructuring, function (self, output) { output.print(self.is_array ? "[" : "{"); var first = true; - self.names.forEach(function (name) { + var len = self.names.length; + self.names.forEach(function (name, i) { if (first) first = false; else { output.comma(); output.space(); } name.print(output); + // If the final element is a hole, we need to make sure it + // doesn't look like a trailing comma, by inserting an actual + // trailing comma. + if (i === len - 1 && name instanceof AST_Hole) + output.comma(); }) output.print(self.is_array ? "]" : "}"); if (self.default) { @@ -739,7 +750,7 @@ function OutputStream(options) { output.space(); self.default.print(output) } - }) + }); DEFPRINT(AST_Debugger, function(self, output){ output.print("debugger"); @@ -1444,17 +1455,21 @@ function OutputStream(options) { } }); DEFPRINT(AST_ObjectKeyVal, function(self, output){ - var print_as_shorthand = self.shorthand && - self.value instanceof AST_Symbol && - self.key == self.value.print_to_string(); - - if (print_as_shorthand) { - output.print_name(self.key); - return; + function get_name(self) { + var def = self.value.definition(); + return def ? def.mangled_name || def.name : self.value.name; + } + if (output.option("shorthand") && + self.value instanceof AST_Symbol && + is_identifier_string(self.key) && + get_name(self) === self.key + ) { + self.print_property_name(self.key, self.quote, output); + } else { + self.print_property_name(self.key, self.quote, output); + output.colon(); + self.value.print(output); } - self.print_property_name(self.key, self.quote, output); - output.colon(); - self.value.print(output); if (self.default) { output.space(); output.print('='); diff --git a/lib/parse.js b/lib/parse.js index 7c9b1cd5..aa1bda45 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -1232,6 +1232,10 @@ function parse($TEXT, options) { }; var arrow_function = function(args) { + if (S.token.nlb) { + croak("SyntaxError: Unexpected newline before arrow (=>)"); + } + expect_token("arrow", "=>"); var argnames; @@ -1425,12 +1429,13 @@ function parse($TEXT, options) { if (is("punc")) { switch (S.token.value) { case ",": - case "]": // Last element elements.push(new AST_Hole({ start: S.token, end: S.token })); continue; + case "]": // Trailing comma after last element + break; case "[": case "{": elements.push(binding_element(used_parameters, symbol_type)); @@ -1560,7 +1565,7 @@ function parse($TEXT, options) { next(); a.push(new AST_Expansion({ start: prev(), - expression: as_symbol(AST_SymbolFunarg), + expression: binding_element(undefined, AST_SymbolFunarg), end: S.token, })); if (!is("punc", ")")) { @@ -2011,15 +2016,13 @@ function parse($TEXT, options) { // It's one of those object destructurings, the value is its own name a.push(new AST_ObjectKeyVal({ start: start, - quote: start.quote, - end: start, key: name, value: new AST_SymbolRef({ start: start, - end: start, + end: prev(), name: name }), - shorthand: true, + end: start, })); } else { expect(":"); diff --git a/lib/scope.js b/lib/scope.js index d2712256..056131db 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -567,10 +567,12 @@ AST_Toplevel.DEFMETHOD("compute_char_frequency", function(options){ else if (node instanceof AST_With) base54.consider("with"); else if (node instanceof AST_ObjectSetter) - base54.consider("set" + node.key); + base54.consider("set" + (typeof node.key === "string" ? node.key : "")); else if (node instanceof AST_ObjectGetter) - base54.consider("get" + node.key); - else if (node instanceof AST_ObjectKeyVal) + base54.consider("get" + (typeof node.key === "string" ? node.key : "")); + else if (node instanceof AST_ObjectKeyVal && typeof node.key === "string") + base54.consider(node.key); + else if (node instanceof AST_ConciseMethod && typeof node.key === "string") base54.consider(node.key); else if (node instanceof AST_New) base54.consider("new"); diff --git a/test/compress/arrays.js b/test/compress/arrays.js index bc573083..fa98309f 100644 --- a/test/compress/arrays.js +++ b/test/compress/arrays.js @@ -1,3 +1,5 @@ +// NOTE trailing comma doesn't contribute to length of an array +// That also means the array changes length if previous element is a hole too and got cut off holes_and_undefined: { input: { w = [1,,]; diff --git a/test/compress/arrow.js b/test/compress/arrow.js index 1a8b17ed..fb9063a2 100644 --- a/test/compress/arrow.js +++ b/test/compress/arrow.js @@ -90,3 +90,39 @@ arrow_function_with_single_parameter_with_default: { } expect_exact: "var foo=(a=0)=>doSomething(a);" } + +arrow_binding_pattern: { + input: { + var foo = ([]) => "foo"; + var bar = ({}) => "bar"; + var with_default = (foo = "default") => foo; + var object_with_default = ({foo = "default", bar: baz = "default"}) => foo; + var array_after_spread = (...[foo]) => foo; + var array_after_spread = (...{foo}) => foo; + var computed = ({ [compute()]: x }) => {}; + var array_hole = ([, , ...x] = [1, 2]) => {}; + var object_trailing_elision = ({foo,}) => {}; + var spread_empty_array = (...[]) => "foo"; + var spread_empty_object = (...{}) => "foo"; + } + expect: { + var foo = ([]) => "foo"; + var bar = ({}) => "bar"; + var with_default = (foo = "default") => foo; + var object_with_default = ({foo = "default", bar: baz = "default"}) => foo; + var array_after_spread = (...[foo]) => foo; + var array_after_spread = (...{foo}) => foo; + var computed = ({ [compute()]: x }) => {}; + var array_hole = ([, , ...x] = [1, 2]) => {}; + var object_trailing_elision = ({foo,}) => {}; + var spread_empty_array = (...[]) => "foo"; + var spread_empty_object = (...{}) => "foo"; + } +} + +arrow_binding_pattern_strict: { + input: { + var foo = ([,]) => "foo"; + } + expect_exact: 'var foo=([,])=>"foo";' +} diff --git a/test/compress/destructuring.js b/test/compress/destructuring.js index 35be0a2c..5468bb81 100644 --- a/test/compress/destructuring.js +++ b/test/compress/destructuring.js @@ -6,7 +6,8 @@ destructuring_arrays: { {let [aa, [bb, cc]] = dd;} var [aa, bb] = cc; var [aa, [bb, cc]] = dd; - var [,[,,,,,],,,zz,] = xx; + var [,[,,,,,],,,zz,] = xx; // Trailing comma + var [,,zzz,,] = xxx; // Trailing comma after hole } expect: { {const [aa, bb] = cc;} @@ -15,10 +16,20 @@ destructuring_arrays: { {let [aa, [bb, cc]] = dd;} var [aa, bb] = cc; var [aa, [bb, cc]] = dd; - var [,[,,,,,],,,zz,] = xx; + var [,[,,,,,],,,zz] = xx; + var [,,zzz,,] = xxx; } } +destructuring_arrays_holes: { + input: { + var [,,,,] = a; + var [,,b,] = c; + var [d,,] = e; + } + expect_exact: "var[,,,,]=a;var[,,b]=c;var[d,,]=e;" +} + destructuring_objects: { input: { {const {aa, bb} = {aa:1, bb:2};} @@ -82,6 +93,9 @@ destructuring_vardef_in_loops: { } destructuring_expressions: { + beautify = { + ecma: 6 + } input: { ({a, b}); [{a}]; diff --git a/test/compress/object.js b/test/compress/object.js index b493204a..c3ad2989 100644 --- a/test/compress/object.js +++ b/test/compress/object.js @@ -89,6 +89,9 @@ getter_setter: { getter_setter_mangler: { mangle = {} + beautify = { + ecma: 6 + } input: { function f(get,set) { return { @@ -102,7 +105,18 @@ getter_setter_mangler: { }; } } - expect_exact: "function f(t,e){return{get:t,set:e,get g(){},set s(t){},c,a:1,m(){}}}" + expect_exact: "function f(n,t){return{get:n,set:t,get g(){},set s(n){},c,a:1,m(){}}}" +} + +use_shorthand_opportunity: { + beautify = { + ecma: 6 + } + input: { + var foo = 123; + var obj = {foo: foo}; + } + expect_exact: "var foo=123;var obj={foo};" } computed_property_names: { @@ -468,3 +482,12 @@ methods_and_getters_with_keep_quoted_props_enabled: { } expect_exact: 'var obj={a(){},"b"(){},get c(){return"c"},get"d"(){return"d"},set e(a){doSomething(a)},set f(a){doSomething(b)}};' } + +allow_assignments_to_property_values: { + input: { + var foo = {123: foo = 123} = {foo: "456"}; + } + expect: { + var foo = {123: foo = 123} = {foo: "456"}; + } +} diff --git a/test/mocha/arrow.js b/test/mocha/arrow.js index a02286c2..ffd037d0 100644 --- a/test/mocha/arrow.js +++ b/test/mocha/arrow.js @@ -18,6 +18,45 @@ describe("Arrow functions", function() { e.message === "SyntaxError: Unexpected token: expand (...)"; } + for (var i = 0; i < tests.length; i++) { + assert.throws(test(tests[i]), error); + } + }); + it("Should not accept holes in object binding patterns, while still allowing a trailing elision", function() { + var tests = [ + "f = ({, , ...x} = [1, 2]) => {};" + ]; + var test = function(code) { + return function() { + uglify.parse(code, {fromString: true}); + } + } + var error = function(e) { + return e instanceof uglify.JS_Parse_Error && + e.message === "SyntaxError: Unexpected token: punc (,)"; + } + + for (var i = 0; i < tests.length; i++) { + assert.throws(test(tests[i]), error); + } + }); + it("Should not accept newlines before arrow token", function() { + var tests = [ + "f = foo\n=> 'foo';", + "f = (foo, bar)\n=> 'foo';", + "f = ()\n=> 'foo';", + "foo((bar)\n=>'baz';);" + ]; + var test = function(code) { + return function() { + uglify.parse(code, {fromString: true}); + } + } + var error = function(e) { + return e instanceof uglify.JS_Parse_Error && + e.message === "SyntaxError: Unexpected newline before arrow (=>)"; + } + for (var i = 0; i < tests.length; i++) { assert.throws(test(tests[i]), error); } diff --git a/test/mocha/function.js b/test/mocha/function.js index 94c518ae..2ae8a202 100644 --- a/test/mocha/function.js +++ b/test/mocha/function.js @@ -13,30 +13,39 @@ describe("Function", function() { // Destructurings as arguments var destr_fun1 = uglify.parse('(function ({a, b}) {})').body[0].body; var destr_fun2 = uglify.parse('(function ([a, [b]]) {})').body[0].body; + var destr_fun3 = uglify.parse('({a, b}) => null').body[0].body; + var destr_fun4 = uglify.parse('([a, [b]]) => null').body[0].body; assert.equal(destr_fun1.argnames.length, 1); assert.equal(destr_fun2.argnames.length, 1); - - var destr_fun1 = uglify.parse('({a, b}) => null').body[0].body; - var destr_fun2 = uglify.parse('([a, [b]]) => null').body[0].body; - - assert.equal(destr_fun1.argnames.length, 1); - assert.equal(destr_fun2.argnames.length, 1); + assert.equal(destr_fun3.argnames.length, 1); + assert.equal(destr_fun4.argnames.length, 1); var destruct1 = destr_fun1.argnames[0]; var destruct2 = destr_fun2.argnames[0]; + var destruct3 = destr_fun3.argnames[0]; + var destruct4 = destr_fun4.argnames[0]; assert(destruct1 instanceof uglify.AST_Destructuring); assert(destruct2 instanceof uglify.AST_Destructuring); + assert(destruct3 instanceof uglify.AST_Destructuring); + assert(destruct4 instanceof uglify.AST_Destructuring); assert(destruct2.names[1] instanceof uglify.AST_Destructuring); + assert(destruct4.names[1] instanceof uglify.AST_Destructuring); assert.equal(destruct1.start.value, '{'); assert.equal(destruct1.end.value, '}'); assert.equal(destruct2.start.value, '['); assert.equal(destruct2.end.value, ']'); + assert.equal(destruct3.start.value, '{'); + assert.equal(destruct3.end.value, '}'); + assert.equal(destruct4.start.value, '['); + assert.equal(destruct4.end.value, ']'); assert.equal(destruct1.is_array, false); assert.equal(destruct2.is_array, true); + assert.equal(destruct3.is_array, false); + assert.equal(destruct4.is_array, true); var aAndB = [ ['SymbolFunarg', 'a'], @@ -46,15 +55,43 @@ describe("Function", function() { assert.deepEqual( [ destruct1.names[0].TYPE, - destruct1.names[0].name], + destruct1.names[0].name + ], + aAndB[0]); + assert.deepEqual( + [ + destruct1.names[1].TYPE, + destruct1.names[1].name + ], + aAndB[1]); + assert.deepEqual( + [ + destruct2.names[0].TYPE, + destruct2.names[0].name + ], aAndB[0]); - assert.deepEqual( [ destruct2.names[1].names[0].TYPE, destruct2.names[1].names[0].name ], aAndB[1]); + assert.strictEqual(typeof destruct3.names[0].key, "string"); + assert.strictEqual(destruct3.names[0].key, "a"); + assert.strictEqual(typeof destruct3.names[1].key, "string"); + assert.strictEqual(destruct3.names[1].key, "b"); + assert.deepEqual( + [ + destruct4.names[0].TYPE, + destruct4.names[0].name + ], + aAndB[0]); + assert.deepEqual( + [ + destruct4.names[1].names[0].TYPE, + destruct4.names[1].names[0].name + ], + aAndB[1]); assert.deepEqual( get_args(destr_fun1.args_as_names()), @@ -62,6 +99,12 @@ describe("Function", function() { assert.deepEqual( get_args(destr_fun2.args_as_names()), aAndB); + assert.deepEqual( + get_args(destr_fun3.args_as_names()), + aAndB); + assert.deepEqual( + get_args(destr_fun4.args_as_names()), + aAndB); // Making sure we don't accidentally accept things which // Aren't argument destructurings diff --git a/test/mocha/object.js b/test/mocha/object.js index f5cd1a62..cabe8a9a 100644 --- a/test/mocha/object.js +++ b/test/mocha/object.js @@ -124,4 +124,8 @@ describe("Object", function() { assert.throws(testCase(test), fail(test), errorMessage(test)); } }); + it("Should be able to use shorthand properties", function() { + var ast = Uglify.parse("var foo = 123\nvar obj = {foo: foo}"); + assert.strictEqual(ast.print_to_string({ecma: 6}), "var foo=123;var obj={foo};"); + }) });