From 39d4d7e20ae013b1c6071964ce048ee86cbff6e9 Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Fri, 26 May 2017 13:35:40 +0800 Subject: [PATCH] fix `export` related issues (#2005) - `mangle` non-exported names - `unused` on `export` of `function` - `hoist_funs` on `export` - `export default` - prohibit definition statements - parse `AST_Defun` properly - drop only unused class and function names fixes #2001 fixes #2004 --- lib/compress.js | 16 +-- lib/parse.js | 22 +++- lib/scope.js | 86 +++++--------- test/compress/issue-2001.js | 218 ++++++++++++++++++++++++++++++++++++ 4 files changed, 270 insertions(+), 72 deletions(-) diff --git a/lib/compress.js b/lib/compress.js index 0d6aae80..904a15e9 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -2157,9 +2157,11 @@ merge(Compressor.prototype, { // pass 3: we should drop declarations not in_use var tt = new TreeTransformer( function before(node, descend, in_list) { - if (node instanceof AST_Function - && node.name - && !compressor.option("keep_fnames")) { + var parent = tt.parent(); + if (!compressor.option("keep_fnames") + && ((node instanceof AST_Function || node instanceof AST_ClassExpression) && node.name + || (node instanceof AST_Defun || node instanceof AST_DefClass) + && parent instanceof AST_Export && parent.is_default)) { var def = node.name.definition(); // any declarations with same name will overshadow // name of this anonymous function and can therefore @@ -2194,7 +2196,7 @@ merge(Compressor.prototype, { } } } - if ((node instanceof AST_Defun || node instanceof AST_DefClass) && node !== self) { + if ((node instanceof AST_Defun || node instanceof AST_DefClass) && !(parent instanceof AST_Export) && node !== self) { var keep = (node.name.definition().id in in_use_ids) || !drop_funcs && node.name.definition().global; if (!keep) { compressor[node.name.unreferenced() ? "warn" : "info"]("Dropping unused function {name} [{file}:{line},{col}]", template(node.name)); @@ -2202,7 +2204,7 @@ merge(Compressor.prototype, { } return node; } - if (node instanceof AST_Definitions && !(tt.parent() instanceof AST_ForIn && tt.parent().init === node)) { + if (node instanceof AST_Definitions && !(parent instanceof AST_ForIn && parent.init === node)) { // place uninitialized names at the start var body = [], head = [], tail = []; // for unused names whose initialization has @@ -2298,7 +2300,7 @@ merge(Compressor.prototype, { if (!(def.id in in_use_ids) && (drop_vars || !def.global) && self.variables.get(def.name) === def) { - return maintain_this_binding(tt.parent(), node, node.right.transform(tt)); + return maintain_this_binding(parent, node, node.right.transform(tt)); } } // certain combination of unused name + side effect leads to: @@ -2384,7 +2386,7 @@ merge(Compressor.prototype, { dirs.push(node); return make_node(AST_EmptyStatement, node); } - if (node instanceof AST_Defun && hoist_funs) { + if (hoist_funs && node instanceof AST_Defun && !(tt.parent() instanceof AST_Export)) { hoisted.push(node); return make_node(AST_EmptyStatement, node); } diff --git a/lib/parse.js b/lib/parse.js index 469e14a6..d5012adc 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -2417,14 +2417,26 @@ function parse($TEXT, options) { } } - var is_definition = is("keyword", "var") - || is("keyword", "let") - || is("keyword", "const") - || is("keyword", "function") && !is_default; + var is_definition = is("keyword", "var") || is("keyword", "let") || is("keyword", "const"); if (is_definition) { + if (is_default) unexpected(); exported_definition = statement(); + } else if (is("keyword", "class")) { + var cls = expr_atom(false); + if (cls.name) { + cls.name = new AST_SymbolDefClass(cls.name); + exported_definition = new AST_DefClass(cls); + } else { + exported_value = cls; + } } else if (is("keyword", "function")) { - exported_value = expr_atom(false); + var func = expr_atom(false); + if (func.name) { + func.name = new AST_SymbolDefun(func.name); + exported_definition = new AST_Defun(func); + } else { + exported_value = func; + } } else { exported_value = expression(false); semicolon(); diff --git a/lib/scope.js b/lib/scope.js index ccb836e7..39a4a1a7 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -111,8 +111,6 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){ var labels = new Dictionary(); var defun = null; var in_destructuring = null; - var in_export = false; - var in_block = 0; var for_scopes = []; var tw = new TreeWalker(function(node, descend){ if (node.is_block_scope()) { @@ -152,22 +150,6 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){ labels = save_labels; return true; // don't descend again in TreeWalker } - if (node instanceof AST_Export) { - in_export = true; - descend(); - in_export = false; - return true; - } - if (node instanceof AST_BlockStatement - || node instanceof AST_Switch - || node instanceof AST_Try - || node instanceof AST_Catch - || node instanceof AST_Finally) { - in_block++; - descend(); - in_block--; - return true; - } if (node instanceof AST_LabeledStatement) { var l = node.label; if (labels.has(l.name)) { @@ -194,7 +176,7 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){ node.references = []; } if (node instanceof AST_SymbolLambda) { - defun.def_function(node, in_export, in_block); + defun.def_function(node); } else if (node instanceof AST_SymbolDefun) { // Careful here, the scope where this should be defined is @@ -206,23 +188,24 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){ while (parent_lambda.is_block_scope()) { parent_lambda = parent_lambda.parent_scope; } - (node.scope = parent_lambda).def_function(node, in_export, in_block); + mark_export((node.scope = parent_lambda).def_function(node), 1); } else if (node instanceof AST_SymbolClass) { - defun.def_variable(node, in_export, in_block); + mark_export(defun.def_variable(node), 1); } else if (node instanceof AST_SymbolImport) { - scope.def_variable(node, in_export, in_block); + scope.def_variable(node); } else if (node instanceof AST_SymbolDefClass) { // This deals with the name of the class being available // inside the class. - (node.scope = defun.parent_scope).def_function(node, in_export, in_block); + mark_export((node.scope = defun.parent_scope).def_function(node), 1); } else if (node instanceof AST_SymbolVar || node instanceof AST_SymbolLet || node instanceof AST_SymbolConst) { - var def = ((node instanceof AST_SymbolBlockDeclaration) ? scope : defun).def_variable(node, in_export, in_block); + var def = ((node instanceof AST_SymbolBlockDeclaration) ? scope : defun).def_variable(node); + if (!(node instanceof AST_SymbolFunarg)) mark_export(def, 2); def.destructuring = in_destructuring; if (defun !== scope) { node.mark_enclosed(options); @@ -234,7 +217,7 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){ } } else if (node instanceof AST_SymbolCatch) { - scope.def_variable(node, in_export, in_block).defun = defun; + scope.def_variable(node).defun = defun; } else if (node instanceof AST_LabelRef) { var sym = labels.get(node.name); @@ -245,12 +228,16 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){ })); node.thedef = sym; } + + function mark_export(def, level) { + var node = tw.parent(level); + def.export = node instanceof AST_Export && !node.is_default; + } }); self.walk(tw); // pass 2: find back references and eval var func = null; - var cls = null; var globals = self.globals = new Dictionary(); var tw = new TreeWalker(function(node, descend){ if (node instanceof AST_Lambda) { @@ -260,13 +247,6 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){ func = prev_func; return true; } - if (node instanceof AST_Class) { - var prev_cls = cls; - cls = node; - descend(); - cls = prev_cls; - return true; - } if (node instanceof AST_LoopControl && node.label) { node.label.thedef.references.push(node); return true; @@ -351,22 +331,13 @@ AST_Scope.DEFMETHOD("init_scope_vars", function(parent_scope){ this.cname = -1; // the current index for mangling functions/variables }); -AST_Node.DEFMETHOD("is_block_scope", function(){ - return false; // Behaviour will be overridden by AST_Block -}); - -AST_Block.DEFMETHOD("is_block_scope", function(){ - return ( - !(this instanceof AST_Lambda) && - !(this instanceof AST_Toplevel) && - !(this instanceof AST_Class) && - !(this instanceof AST_SwitchBranch) - ); -}); - -AST_IterationStatement.DEFMETHOD("is_block_scope", function(){ - return true; -}); +AST_Node.DEFMETHOD("is_block_scope", return_false); +AST_Class.DEFMETHOD("is_block_scope", return_false); +AST_Lambda.DEFMETHOD("is_block_scope", return_false); +AST_Toplevel.DEFMETHOD("is_block_scope", return_false); +AST_SwitchBranch.DEFMETHOD("is_block_scope", return_false); +AST_Block.DEFMETHOD("is_block_scope", return_true); +AST_IterationStatement.DEFMETHOD("is_block_scope", return_true); AST_Lambda.DEFMETHOD("init_scope_vars", function(){ AST_Scope.prototype.init_scope_vars.apply(this, arguments); @@ -404,24 +375,19 @@ AST_Scope.DEFMETHOD("find_variable", function(name){ || (this.parent_scope && this.parent_scope.find_variable(name)); }); -AST_Scope.DEFMETHOD("def_function", function(symbol, in_export, in_block){ - this.functions.set(symbol.name, this.def_variable(symbol, in_export, in_block)); +AST_Scope.DEFMETHOD("def_function", function(symbol){ + var def = this.def_variable(symbol); + this.functions.set(symbol.name, def); + return def; }); -AST_Scope.DEFMETHOD("def_variable", function(symbol, in_export, in_block){ +AST_Scope.DEFMETHOD("def_variable", function(symbol){ var def; if (!this.variables.has(symbol.name)) { def = new SymbolDef(this, this.variables.size(), symbol); this.variables.set(symbol.name, def); def.object_destructuring_arg = symbol.object_destructuring_arg; - if (in_export) { - def.export = true; - } - if (in_block && symbol instanceof AST_SymbolBlockDeclaration) { - def.global = false; - } else { - def.global = !this.parent_scope; - } + def.global = !this.parent_scope; } else { def = this.variables.get(symbol.name); def.orig.push(symbol); diff --git a/test/compress/issue-2001.js b/test/compress/issue-2001.js index 4efa8261..0109641b 100644 --- a/test/compress/issue-2001.js +++ b/test/compress/issue-2001.js @@ -1,5 +1,7 @@ export_func_1: { options = { + hoist_funs: true, + toplevel: true, unused: true, } input: { @@ -10,7 +12,9 @@ export_func_1: { export_func_2: { options = { + hoist_funs: true, side_effects: false, + toplevel: true, unused: true, } input: { @@ -21,7 +25,9 @@ export_func_2: { export_func_3: { options = { + hoist_funs: true, side_effects: true, + toplevel: true, unused: true, } input: { @@ -32,6 +38,8 @@ export_func_3: { export_default_func_1: { options = { + hoist_funs: true, + toplevel: true, unused: true, } input: { @@ -42,7 +50,9 @@ export_default_func_1: { export_default_func_2: { options = { + hoist_funs: true, side_effects: false, + toplevel: true, unused: true, } input: { @@ -53,7 +63,9 @@ export_default_func_2: { export_default_func_3: { options = { + hoist_funs: true, side_effects: true, + toplevel: true, unused: true, } input: { @@ -61,3 +73,209 @@ export_default_func_3: { } expect_exact: "export default function(){};" } + +export_class_1: { + options = { + hoist_funs: true, + toplevel: true, + unused: true, + } + input: { + export class C {}; + } + expect_exact: "export class C{};" +} + +export_class_2: { + options = { + hoist_funs: true, + side_effects: false, + toplevel: true, + unused: true, + } + input: { + export class C {}(1); + } + expect_exact: "export class C{};1;" +} + +export_class_3: { + options = { + hoist_funs: true, + side_effects: true, + toplevel: true, + unused: true, + } + input: { + export class C {}(1); + } + expect_exact: "export class C{};" +} + +export_default_class_1: { + options = { + hoist_funs: true, + toplevel: true, + unused: true, + } + input: { + export default class C {}; + } + expect_exact: "export default class{};" +} + +export_default_class_2: { + options = { + hoist_funs: true, + side_effects: false, + toplevel: true, + unused: true, + } + input: { + export default class C {}(1); + } + expect_exact: "export default class{};1;" +} + +export_default_class_3: { + options = { + hoist_funs: true, + side_effects: true, + toplevel: true, + unused: true, + } + input: { + export default class C {}(1); + } + expect_exact: "export default class{};" +} + +export_mangle_1: { + mangle = { + toplevel: true, + } + input: { + export function foo(one, two) { + return one - two; + }; + } + expect_exact: "export function foo(n,o){return n-o};" +} + +export_mangle_2: { + mangle = { + toplevel: true, + } + input: { + export default function foo(one, two) { + return one - two; + }; + } + expect_exact: "export default function n(n,r){return n-r};" +} + +export_mangle_3: { + options = { + collapse_vars: true, + } + mangle = { + toplevel: true, + } + input: { + export class C { + go(one, two) { + var z = one; + return one - two + z; + } + }; + } + expect_exact: "export class C{go(n,r){return n-r+n}};" +} + +export_mangle_4: { + options = { + collapse_vars: true, + } + mangle = { + toplevel: true, + } + input: { + export default class C { + go(one, two) { + var z = one; + return one - two + z; + } + }; + } + expect_exact: "export default class n{go(n,r){return n-r+n}};" +} + +export_mangle_5: { + mangle = { + toplevel: true, + } + input: { + export default { + prop: function(one, two) { + return one - two; + } + }; + } + expect_exact: "export default{prop:function(n,r){return n-r}};" +} + +export_mangle_6: { + mangle = { + toplevel: true, + } + input: { + var baz = 2; + export let foo = 1, bar = baz; + } + expect_exact: "var a=2;export let foo=1,bar=a;" +} + +export_toplevel_1: { + options = { + toplevel: true, + unused: true, + } + input: { + function f(){} + export function g(){}; + export default function h(){}; + } + expect: { + export function g(){}; + export default function(){}; + } +} + +export_toplevel_2: { + options = { + toplevel: true, + unused: true, + } + input: { + class A {} + export class B {}; + export default class C {}; + } + expect: { + export class B {}; + export default class {}; + } +} + +export_default_func_ref: { + options = { + hoist_funs: true, + toplevel: true, + unused: true, + } + input: { + export default function f(){}; + f(); + } + expect_exact: "export default function f(){};f();" +}