From 0e5129de9b3485d80162a2fecb510e1c8b0601df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Santos?= Date: Thu, 13 Aug 2015 02:56:57 +0100 Subject: [PATCH 1/7] prepare AST_Destructuring for the Ents --- lib/transform.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/transform.js b/lib/transform.js index 41c82c99..7858759a 100644 --- a/lib/transform.js +++ b/lib/transform.js @@ -163,6 +163,10 @@ TreeTransformer.prototype = new TreeWalker; if (self.value) self.value = self.value.transform(tw); }); + _(AST_Destructuring, function(self, tw) { + self.names = do_list(self.names, tw); + }); + _(AST_Lambda, function(self, tw){ if (self.name) self.name = self.name.transform(tw); self.argnames = do_list(self.argnames, tw); From 0e1c4a02c2ce4df6ac6d35bd87f8d92ad1d020ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Santos?= Date: Thu, 13 Aug 2015 02:58:08 +0100 Subject: [PATCH 2/7] A little refactoring. Add a new function to get all symbols in a destructuring. --- lib/ast.js | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/lib/ast.js b/lib/ast.js index 2622cf18..4a9cc9fb 100644 --- a/lib/ast.js +++ b/lib/ast.js @@ -445,15 +445,13 @@ var AST_Lambda = DEFNODE("Lambda", "name argnames uses_arguments", { }, args_as_names: function () { var out = []; - this.walk(new TreeWalker(function (parm) { - var that = this; - if (parm instanceof AST_SymbolFunarg) { - out.push(parm); + for (var i = 0; i < this.argnames.length; i++) { + if (this.argnames[i] instanceof AST_Destructuring) { + out = out.concat(this.argnames[i].all_symbols()); + } else { + out.push(this.argnames[i]); } - if (parm instanceof AST_Expansion) { - out.push(parm.symbol); - } - })); + } return out; }, _walk: function(visitor) { @@ -492,6 +490,18 @@ var AST_Destructuring = DEFNODE("Destructuring", "names is_array", { name._walk(visitor); }); }); + }, + all_symbols: function() { + var out = []; + this.walk(new TreeWalker(function (node) { + if (node instanceof AST_Symbol) { + out.push(node); + } + if (node instanceof AST_Expansion) { + out.push(node.symbol); + } + })); + return out; } }); From 5c45e6df90015c8473fd5ff9f026efc2ce68b437 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Santos?= Date: Fri, 14 Aug 2015 00:20:21 +0100 Subject: [PATCH 3/7] Parse and compress destructuring VarDefs --- lib/ast.js | 5 ++++- lib/compress.js | 15 ++++++++++++++- lib/parse.js | 33 +++++++++++++++++++++++++++------ test/compress/destructuring.js | 19 +++++++++++++++++++ test/compress/drop-unused.js | 15 +++++++++++++++ test/compress/hoist.js | 18 ++++++++++++++++++ test/parser.js | 8 ++++++++ 7 files changed, 105 insertions(+), 8 deletions(-) create mode 100644 test/compress/destructuring.js diff --git a/lib/ast.js b/lib/ast.js index 4a9cc9fb..7a080b3e 100644 --- a/lib/ast.js +++ b/lib/ast.js @@ -665,9 +665,12 @@ var AST_Const = DEFNODE("Const", null, { var AST_VarDef = DEFNODE("VarDef", "name value", { $documentation: "A variable declaration; only appears in a AST_Definitions node", $propdoc: { - name: "[AST_SymbolVar|AST_SymbolConst] name of the variable", + name: "[AST_SymbolVar|AST_SymbolConst|AST_Destructuring] name of the variable", value: "[AST_Node?] initializer, or null of there's no initializer" }, + is_destructuring: function() { + return this.name instanceof AST_Destructuring; + }, _walk: function(visitor) { return visitor._visit(this, function(){ this.name._walk(visitor); diff --git a/lib/compress.js b/lib/compress.js index 6b480390..5a76849f 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -1052,6 +1052,7 @@ merge(Compressor.prototype, { } if (node instanceof AST_Definitions && scope === self) { node.definitions.forEach(function(def){ + if (def.is_destructuring()) return; /* Destructurings are type assertions! */ if (def.value) { initializations.add(def.name.name, def.value); if (def.value.has_side_effects(compressor)) { @@ -1138,6 +1139,7 @@ merge(Compressor.prototype, { } if (node instanceof AST_Definitions && !(tt.parent() instanceof AST_ForIn)) { var def = node.definitions.filter(function(def){ + if (def.is_destructuring()) return true; if (member(def.name.definition(), in_use)) return true; var w = { name : def.name.name, @@ -1258,6 +1260,7 @@ merge(Compressor.prototype, { } if (node instanceof AST_Var && hoist_vars) { node.definitions.forEach(function(def){ + if (def.is_destructuring()) { return; } vars.set(def.name.name, def); ++vars_found; }); @@ -1698,13 +1701,23 @@ merge(Compressor.prototype, { AST_Definitions.DEFMETHOD("to_assignments", function(){ var assignments = this.definitions.reduce(function(a, def){ - if (def.value) { + if (def.value && !def.is_destructuring()) { var name = make_node(AST_SymbolRef, def.name, def.name); a.push(make_node(AST_Assign, def, { operator : "=", left : name, right : def.value })); + } else if (def.value) { + // Because it's a destructuring, do not turn into an assignment. + var varDef = make_node(AST_VarDef, def, { + name: def.name, + value: def.value + }); + var var_ = make_node(AST_Var, def, { + definitions: [ varDef ] + }); + a.push(var_); } return a; }, []); diff --git a/lib/parse.js b/lib/parse.js index ba87acf7..96e158a1 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -1174,13 +1174,24 @@ function parse($TEXT, options) { function vardefs(no_in, in_const) { var a = []; + var def; for (;;) { - a.push(new AST_VarDef({ - start : S.token, - name : as_symbol(in_const ? AST_SymbolConst : AST_SymbolVar), - value : is("operator", "=") ? (next(), expression(false, no_in)) : null, - end : prev() - })); + if (is("punc", "{") || is("punc", "[")) { + def = new AST_VarDef({ + start: S.token, + name: destructuring_(), + value: (expect_token("operator", "="), expression(false, no_in)), + end: prev() + }); + } else { + def = new AST_VarDef({ + start : S.token, + name : as_symbol(in_const ? AST_SymbolConst : AST_SymbolVar), + value : is("operator", "=") ? (next(), expression(false, no_in)) : null, + end : prev() + }) + } + a.push(def); if (!is("punc", ",")) break; next(); @@ -1188,6 +1199,16 @@ function parse($TEXT, options) { return a; }; + var destructuring_ = embed_tokens(function () { + var is_array = is("punc", "["); + var closing = is_array ? ']' : '}'; + next() + return new AST_Destructuring({ + names: expr_list(closing), + is_array: is_array + }) + }); + var var_ = function(no_in) { return new AST_Var({ start : prev(), diff --git a/test/compress/destructuring.js b/test/compress/destructuring.js new file mode 100644 index 00000000..58a77cd4 --- /dev/null +++ b/test/compress/destructuring.js @@ -0,0 +1,19 @@ + +destructuring_arrays: { + input: { + var [aa, bb] = cc; + } + expect: { + var[aa,bb]=cc; + } +} + +destructuring_objects: { + input: { + var {aa, bb} = {aa:1, bb:2}; + } + expect: { + var{aa,bb}={aa:1,bb:2}; + } +} + diff --git a/test/compress/drop-unused.js b/test/compress/drop-unused.js index c1cf5c3c..9bee9a2b 100644 --- a/test/compress/drop-unused.js +++ b/test/compress/drop-unused.js @@ -164,6 +164,21 @@ used_var_in_catch: { } } +unused_keep_harmony_destructuring: { + options = { unused: true }; + input: { + function foo() { + var {x, y} = foo; + var a = foo; + } + } + expect: { + function foo() { + var {x, y} = foo; + } + } +} + keep_fnames: { options = { unused: true, keep_fnames: true, unsafe: true }; input: { diff --git a/test/compress/hoist.js b/test/compress/hoist.js index 2b359fc5..68f5a8c3 100644 --- a/test/compress/hoist.js +++ b/test/compress/hoist.js @@ -65,3 +65,21 @@ hoist_no_destructurings: { } } } + +dont_hoist_var_destructurings: { + options = { + hoist_vars: true, + hoist_funs: true + } + input: { + function x() { + // If foo is null or undefined, this should be an exception + var {x,y} = foo; + } + } + expect: { + function x() { + var {x,y} = foo; + } + } +} diff --git a/test/parser.js b/test/parser.js index e661907a..85c2e23e 100644 --- a/test/parser.js +++ b/test/parser.js @@ -94,6 +94,14 @@ module.exports = function () { UglifyJS.parse('(function [a] { })'); }); + // Destructuring variable declaration + + var decls = UglifyJS.parse('var {a,b} = foo, { c, d } = bar'); + + ok.equal(decls.body[0].TYPE, 'Var'); + ok.equal(decls.body[0].definitions.length, 2); + ok.equal(decls.body[0].definitions[0].name.TYPE, 'Destructuring'); + ok.throws(function () { // Note: this *is* a valid destructuring, but before we implement // destructuring (right now it's only destructuring *arguments*), From d77e9aa2ba7d550d8c6a415a83152442c445f972 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Santos?= Date: Fri, 14 Aug 2015 01:16:54 +0100 Subject: [PATCH 4/7] Add holes in destructuring defs, also make them nestable --- lib/parse.js | 30 +++++++++++++++++++++++++----- test/compress/destructuring.js | 6 ++++++ test/parser.js | 11 +++++++++++ 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/lib/parse.js b/lib/parse.js index 96e158a1..00eec02b 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -747,7 +747,7 @@ function parse($TEXT, options) { function embed_tokens(parser) { return function() { var start = S.token; - var expr = parser(); + var expr = parser.apply(null, arguments); var end = prev(); expr.start = start; expr.end = end; @@ -1176,17 +1176,18 @@ function parse($TEXT, options) { var a = []; var def; for (;;) { + var sym_type = in_const ? AST_SymbolConst : AST_SymbolVar; if (is("punc", "{") || is("punc", "[")) { def = new AST_VarDef({ start: S.token, - name: destructuring_(), + name: destructuring_(sym_type), value: (expect_token("operator", "="), expression(false, no_in)), end: prev() }); } else { def = new AST_VarDef({ start : S.token, - name : as_symbol(in_const ? AST_SymbolConst : AST_SymbolVar), + name : as_symbol(sym_type), value : is("operator", "=") ? (next(), expression(false, no_in)) : null, end : prev() }) @@ -1199,12 +1200,31 @@ function parse($TEXT, options) { return a; }; - var destructuring_ = embed_tokens(function () { + var destructuring_ = embed_tokens(function (sym_type) { var is_array = is("punc", "["); var closing = is_array ? ']' : '}'; + var sym_type = sym_type || AST_SymbolRef; + + next(); + + var first = true, children = []; + while (!is("punc", closing)) { + if (first) first = false; else expect(","); + if (is("punc", closing)) break; + if (is("punc", ",")) { + children.push(new AST_Hole({ start: S.token, end: S.token })); + } else if (is("punc", "[") || is("punc", "{")) { + children.push(destructuring_(sym_type)); + } else if (is("name")) { + children.push(_make_symbol(sym_type)); + next(); + } else { + children.push(expression()); + } + } next() return new AST_Destructuring({ - names: expr_list(closing), + names: children, is_array: is_array }) }); diff --git a/test/compress/destructuring.js b/test/compress/destructuring.js index 58a77cd4..b667c9ea 100644 --- a/test/compress/destructuring.js +++ b/test/compress/destructuring.js @@ -17,3 +17,9 @@ destructuring_objects: { } } +nested_destructuring_objects: { + input: { + var [{a},b] = c; + } + expect_exact: 'var[{a},b]=c;'; +} diff --git a/test/parser.js b/test/parser.js index 85c2e23e..125f76c2 100644 --- a/test/parser.js +++ b/test/parser.js @@ -101,6 +101,17 @@ module.exports = function () { ok.equal(decls.body[0].TYPE, 'Var'); ok.equal(decls.body[0].definitions.length, 2); ok.equal(decls.body[0].definitions[0].name.TYPE, 'Destructuring'); + ok.equal(decls.body[0].definitions[0].value.TYPE, 'SymbolRef'); + + var nested_def = UglifyJS.parse('var [{x}] = foo').body[0].definitions[0]; + + ok.equal(nested_def.name.names[0].names[0].TYPE, 'SymbolVar') + ok.equal(nested_def.name.names[0].names[0].name, 'x') + + var holey_def = UglifyJS.parse('const [,,third] = [1,2,3]').body[0].definitions[0]; + + ok.equal(holey_def.name.names[0].TYPE, 'Hole') + ok.equal(holey_def.name.names[2].TYPE, 'SymbolConst') ok.throws(function () { // Note: this *is* a valid destructuring, but before we implement From 183cc0dcd88ca68afcdb006fca9c7fba4cb94bc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Santos?= Date: Fri, 14 Aug 2015 02:00:31 +0100 Subject: [PATCH 5/7] Do not mangle a name if it is in a destructuring vardef. --- lib/scope.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/scope.js b/lib/scope.js index a251f55b..7f20844e 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -96,7 +96,7 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){ var scope = self.parent_scope = null; var defun = null; var nesting = 0; - var object_destructuring_arg = false; + var in_destructuring = null; var tw = new TreeWalker(function(node, descend){ if (options.screw_ie8 && node instanceof AST_Catch) { var save_scope = scope; @@ -108,9 +108,9 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){ return true; } if (node instanceof AST_Destructuring && node.is_array === false) { - object_destructuring_arg = true; // These don't nest + in_destructuring = node; // These don't nest descend(); - object_destructuring_arg = false; + in_destructuring = null; return true; } if (node instanceof AST_Scope) { @@ -137,7 +137,7 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){ node.scope = scope; } if (node instanceof AST_SymbolFunarg) { - node.object_destructuring_arg = object_destructuring_arg; + node.object_destructuring_arg = !!in_destructuring; defun.def_variable(node); } if (node instanceof AST_SymbolLambda) { @@ -155,6 +155,7 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){ || node instanceof AST_SymbolConst) { var def = defun.def_variable(node); def.constant = node instanceof AST_SymbolConst; + def.destructuring = in_destructuring; def.init = tw.parent().value; } else if (node instanceof AST_SymbolCatch) { @@ -412,7 +413,10 @@ AST_Toplevel.DEFMETHOD("mangle_names", function(options){ } }); this.walk(tw); - to_mangle.forEach(function(def){ def.mangle(options) }); + to_mangle.forEach(function(def){ + if (def.destructuring && !def.destructuring.is_array) return; + def.mangle(options); + }); if (options.cache) { options.cache.cname = this.cname; From 0c4408351b4934f2ef890baf77ef0007043b4448 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Santos?= Date: Fri, 14 Aug 2015 02:11:38 +0100 Subject: [PATCH 6/7] Destructuring vardef in for..of and for..in --- lib/parse.js | 2 +- test/compress/destructuring.js | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/parse.js b/lib/parse.js index 00eec02b..0458e6bd 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -1181,7 +1181,7 @@ function parse($TEXT, options) { def = new AST_VarDef({ start: S.token, name: destructuring_(sym_type), - value: (expect_token("operator", "="), expression(false, no_in)), + value: is("operator", "=") ? (expect_token("operator", "="), expression(false, no_in)) : null, end: prev() }); } else { diff --git a/test/compress/destructuring.js b/test/compress/destructuring.js index b667c9ea..30fbbee8 100644 --- a/test/compress/destructuring.js +++ b/test/compress/destructuring.js @@ -23,3 +23,12 @@ nested_destructuring_objects: { } expect_exact: 'var[{a},b]=c;'; } + +destructuring_vardef_in_loops: { + input: { + for (var [x,y] in pairs); + for (var [a] = 0;;); + for (var {c} of cees); + } + expect_exact: "for(var[x,y]in pairs);for(var[a]=0;;);for(var{c}of cees);" +} From 16983df7682919304a298a6c367658fc04ec8263 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Santos?= Date: Fri, 14 Aug 2015 02:19:53 +0100 Subject: [PATCH 7/7] Tolerate expansions in vardefs, too! --- lib/ast.js | 10 +++------- lib/parse.js | 9 +++++++++ test/parser.js | 14 ++++++++++---- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/lib/ast.js b/lib/ast.js index 7a080b3e..52077cbb 100644 --- a/lib/ast.js +++ b/lib/ast.js @@ -363,14 +363,10 @@ var AST_Toplevel = DEFNODE("Toplevel", "globals", { } }, AST_Scope); -// TODO besides parameters and function calls, expansions can go in -// arrays, array destructuring parameters, and array destructuring -// assignment. But I'm not adding this right now because I'm trying -// to do the most minimal and independent changesets. -var AST_Expansion = DEFNODE("AST_Expansion", "symbol", { - $documentation: "An expandible argument, such as ...rest", +var AST_Expansion = DEFNODE("Expansion", "symbol", { + $documentation: "An expandible argument, such as ...rest, a splat, such as [1,2,...all], or an expansion in a variable declaration, such as var [first, ...rest] = list", $propdoc: { - symbol: "AST_SymbolFunarg the name of the argument as a SymbolFunarg" + symbol: "AST_Symbol the thing to be expanded" }, _walk: function(visitor) { var self = this; diff --git a/lib/parse.js b/lib/parse.js index 0458e6bd..50c833f7 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -1215,6 +1215,15 @@ function parse($TEXT, options) { children.push(new AST_Hole({ start: S.token, end: S.token })); } else if (is("punc", "[") || is("punc", "{")) { children.push(destructuring_(sym_type)); + } else if (is("expand", "...")) { + next(); + var symbol = _make_symbol(sym_type); + children.push(new AST_Expansion({ + start: prev(), + symbol: symbol, + end: S.token + })); + next(); } else if (is("name")) { children.push(_make_symbol(sym_type)); next(); diff --git a/test/parser.js b/test/parser.js index 125f76c2..66676b9d 100644 --- a/test/parser.js +++ b/test/parser.js @@ -105,13 +105,19 @@ module.exports = function () { var nested_def = UglifyJS.parse('var [{x}] = foo').body[0].definitions[0]; - ok.equal(nested_def.name.names[0].names[0].TYPE, 'SymbolVar') - ok.equal(nested_def.name.names[0].names[0].name, 'x') + ok.equal(nested_def.name.names[0].names[0].TYPE, 'SymbolVar'); + ok.equal(nested_def.name.names[0].names[0].name, 'x'); var holey_def = UglifyJS.parse('const [,,third] = [1,2,3]').body[0].definitions[0]; - ok.equal(holey_def.name.names[0].TYPE, 'Hole') - ok.equal(holey_def.name.names[2].TYPE, 'SymbolConst') + ok.equal(holey_def.name.names[0].TYPE, 'Hole'); + ok.equal(holey_def.name.names[2].TYPE, 'SymbolConst'); + + var expanding_def = UglifyJS.parse('var [first, ...rest] = [1,2,3]').body[0].definitions[0]; + + ok.equal(expanding_def.name.names[0].TYPE, 'SymbolVar'); + ok.equal(expanding_def.name.names[1].TYPE, 'Expansion'); + ok.equal(expanding_def.name.names[1].symbol.TYPE, 'SymbolVar'); ok.throws(function () { // Note: this *is* a valid destructuring, but before we implement