From: Alex Lam S.L Date: Sat, 13 May 2017 18:10:34 +0000 (+0800) Subject: fix bugs with getter/setter (#1926) X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=3ca902258c24209699f0b5bd5b9654252e492272;p=UglifyJS.git fix bugs with getter/setter (#1926) - `reduce_vars` - `side_effects` - property access for object - `AST_SymbolAccessor` as key names enhance `test/ufuzz.js` - add object getter & setter - property assignment to setter - avoid infinite recursion in setter - fix & adjust assignment operators - 50% `=` - 25% `+=` - 2.5% each for the rest - avoid "Invalid array length" - fix `console.log()` - bypass getter - curb recursive reference - deprecate `-E`, always report runtime errors --- diff --git a/lib/ast.js b/lib/ast.js index 57956233..2972b7aa 100644 --- a/lib/ast.js +++ b/lib/ast.js @@ -685,8 +685,8 @@ var AST_Object = DEFNODE("Object", "properties", { var AST_ObjectProperty = DEFNODE("ObjectProperty", "key value", { $documentation: "Base class for literal object properties", $propdoc: { - key: "[string] the property name converted to a string for ObjectKeyVal. For setters and getters this is an arbitrary AST_Node.", - value: "[AST_Node] property value. For setters and getters this is an AST_Function." + key: "[string] the property name converted to a string for ObjectKeyVal. For setters and getters this is an AST_SymbolAccessor.", + value: "[AST_Node] property value. For setters and getters this is an AST_Accessor." }, _walk: function(visitor) { return visitor._visit(this, function(){ diff --git a/lib/compress.js b/lib/compress.js index 892bceef..f168b942 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -368,6 +368,13 @@ merge(Compressor.prototype, { pop(); return true; } + if (node instanceof AST_Accessor) { + var save_ids = safe_ids; + safe_ids = Object.create(null); + descend(); + safe_ids = save_ids; + return true; + } if (node instanceof AST_Binary && (node.operator == "&&" || node.operator == "||")) { node.left.walk(tw); @@ -1220,12 +1227,12 @@ merge(Compressor.prototype, { && !node.expression.has_side_effects(compressor); } - // may_eq_null() - // returns true if this node may evaluate to null or undefined + // may_throw_on_access() + // returns true if this node may be null, undefined or contain `AST_Accessor` (function(def) { - AST_Node.DEFMETHOD("may_eq_null", function(compressor) { + AST_Node.DEFMETHOD("may_throw_on_access", function(compressor) { var pure_getters = compressor.option("pure_getters"); - return !pure_getters || this._eq_null(pure_getters); + return !pure_getters || this._throw_on_access(pure_getters); }); function is_strict(pure_getters) { @@ -1237,7 +1244,12 @@ merge(Compressor.prototype, { def(AST_Undefined, return_true); def(AST_Constant, return_false); def(AST_Array, return_false); - def(AST_Object, return_false); + def(AST_Object, function(pure_getters) { + if (!is_strict(pure_getters)) return false; + for (var i = this.properties.length; --i >=0;) + if (this.properties[i].value instanceof AST_Accessor) return true; + return false; + }); def(AST_Function, return_false); def(AST_UnaryPostfix, return_false); def(AST_UnaryPrefix, function() { @@ -1246,33 +1258,33 @@ merge(Compressor.prototype, { def(AST_Binary, function(pure_getters) { switch (this.operator) { case "&&": - return this.left._eq_null(pure_getters); + return this.left._throw_on_access(pure_getters); case "||": - return this.left._eq_null(pure_getters) - && this.right._eq_null(pure_getters); + return this.left._throw_on_access(pure_getters) + && this.right._throw_on_access(pure_getters); default: return false; } }) def(AST_Assign, function(pure_getters) { return this.operator == "=" - && this.right._eq_null(pure_getters); + && this.right._throw_on_access(pure_getters); }) def(AST_Conditional, function(pure_getters) { - return this.consequent._eq_null(pure_getters) - || this.alternative._eq_null(pure_getters); + return this.consequent._throw_on_access(pure_getters) + || this.alternative._throw_on_access(pure_getters); }) def(AST_Sequence, function(pure_getters) { - return this.expressions[this.expressions.length - 1]._eq_null(pure_getters); + return this.expressions[this.expressions.length - 1]._throw_on_access(pure_getters); }); def(AST_SymbolRef, function(pure_getters) { if (this.is_undefined) return true; if (!is_strict(pure_getters)) return false; var fixed = this.fixed_value(); - return !fixed || fixed._eq_null(pure_getters); + return !fixed || fixed._throw_on_access(pure_getters); }); })(function(node, func) { - node.DEFMETHOD("_eq_null", func); + node.DEFMETHOD("_throw_on_access", func); }); /* -----[ boolean/negation helpers ]----- */ @@ -1813,11 +1825,11 @@ merge(Compressor.prototype, { return any(this.elements, compressor); }); def(AST_Dot, function(compressor){ - return this.expression.may_eq_null(compressor) + return this.expression.may_throw_on_access(compressor) || this.expression.has_side_effects(compressor); }); def(AST_Sub, function(compressor){ - return this.expression.may_eq_null(compressor) + return this.expression.may_throw_on_access(compressor) || this.expression.has_side_effects(compressor) || this.property.has_side_effects(compressor); }); @@ -2370,6 +2382,7 @@ merge(Compressor.prototype, { var args = trim(this.args, compressor, first_in_statement); return args && make_sequence(this, args); }); + def(AST_Accessor, return_null); def(AST_Function, return_null); def(AST_Binary, function(compressor, first_in_statement){ var right = this.right.drop_side_effect_free(compressor); @@ -2437,11 +2450,11 @@ merge(Compressor.prototype, { return values && make_sequence(this, values); }); def(AST_Dot, function(compressor, first_in_statement){ - if (this.expression.may_eq_null(compressor)) return this; + if (this.expression.may_throw_on_access(compressor)) return this; return this.expression.drop_side_effect_free(compressor, first_in_statement); }); def(AST_Sub, function(compressor, first_in_statement){ - if (this.expression.may_eq_null(compressor)) return this; + if (this.expression.may_throw_on_access(compressor)) return this; var expression = this.expression.drop_side_effect_free(compressor, first_in_statement); if (!expression) return this.property.drop_side_effect_free(compressor, first_in_statement); var property = this.property.drop_side_effect_free(compressor); diff --git a/lib/parse.js b/lib/parse.js index c432ad7a..f1089501 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -1310,10 +1310,15 @@ function parse($TEXT, options) { var type = start.type; var name = as_property_name(); if (type == "name" && !is("punc", ":")) { + var key = new AST_SymbolAccessor({ + start: S.token, + name: as_property_name(), + end: prev() + }); if (name == "get") { a.push(new AST_ObjectGetter({ start : start, - key : as_atom_node(), + key : key, value : create_accessor(), end : prev() })); @@ -1322,7 +1327,7 @@ function parse($TEXT, options) { if (name == "set") { a.push(new AST_ObjectSetter({ start : start, - key : as_atom_node(), + key : key, value : create_accessor(), end : prev() })); diff --git a/lib/scope.js b/lib/scope.js index ea08d775..14ffb46f 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -360,11 +360,6 @@ AST_Symbol.DEFMETHOD("unmangleable", function(options){ return this.definition().unmangleable(options); }); -// property accessors are not mangleable -AST_SymbolAccessor.DEFMETHOD("unmangleable", function(){ - return true; -}); - // labels are always mangleable AST_Label.DEFMETHOD("unmangleable", function(){ return false; diff --git a/test/compress/dead-code.js b/test/compress/dead-code.js index 31d0664a..00dac069 100644 --- a/test/compress/dead-code.js +++ b/test/compress/dead-code.js @@ -131,3 +131,19 @@ try_catch_finally: { "1", ] } + +accessor: { + options = { + side_effects: true, + } + input: { + ({ + get a() {}, + set a(v){ + this.b = 2; + }, + b: 1 + }); + } + expect: {} +} diff --git a/test/compress/pure_getters.js b/test/compress/pure_getters.js index 846b53c3..0ca6a804 100644 --- a/test/compress/pure_getters.js +++ b/test/compress/pure_getters.js @@ -119,3 +119,62 @@ chained: { a.b.c; } } + +impure_getter_1: { + options = { + pure_getters: "strict", + side_effects: true, + } + input: { + ({ + get a() { + console.log(1); + }, + b: 1 + }).a; + ({ + get a() { + console.log(1); + }, + b: 1 + }).b; + } + expect: { + ({ + get a() { + console.log(1); + }, + b: 1 + }).a; + ({ + get a() { + console.log(1); + }, + b: 1 + }).b; + } + expect_stdout: "1" +} + +impure_getter_2: { + options = { + pure_getters: true, + side_effects: true, + } + input: { + // will produce incorrect output because getter is not pure + ({ + get a() { + console.log(1); + }, + b: 1 + }).a; + ({ + get a() { + console.log(1); + }, + b: 1 + }).b; + } + expect: {} +} diff --git a/test/compress/reduce_vars.js b/test/compress/reduce_vars.js index a0433b5d..2a44492a 100644 --- a/test/compress/reduce_vars.js +++ b/test/compress/reduce_vars.js @@ -2508,3 +2508,32 @@ issue_1922_2: { } expect_stdout: "1" } + +accessor: { + options = { + evaluate: true, + reduce_vars: true, + toplevel: true, + } + input: { + var a = 1; + console.log({ + get a() { + a = 2; + return a; + }, + b: 1 + }.b, a); + } + expect: { + var a = 1; + console.log({ + get a() { + a = 2; + return a; + }, + b: 1 + }.b, a); + } + expect_stdout: "1 1" +} diff --git a/test/mocha/accessorTokens-1492.js b/test/mocha/accessorTokens-1492.js index 2b5bbeaa..250d9b97 100644 --- a/test/mocha/accessorTokens-1492.js +++ b/test/mocha/accessorTokens-1492.js @@ -16,7 +16,7 @@ describe("Accessor tokens", function() { assert.equal(node.start.pos, 12); assert.equal(node.end.endpos, 46); - assert(node.key instanceof UglifyJS.AST_SymbolRef); + assert(node.key instanceof UglifyJS.AST_SymbolAccessor); assert.equal(node.key.start.pos, 16); assert.equal(node.key.end.endpos, 22); diff --git a/test/mocha/getter-setter.js b/test/mocha/getter-setter.js index 83bf5792..fe0481b3 100644 --- a/test/mocha/getter-setter.js +++ b/test/mocha/getter-setter.js @@ -71,7 +71,7 @@ describe("Getters and setters", function() { var fail = function(data) { return function (e) { return e instanceof UglifyJS.JS_Parse_Error && - e.message === "Invalid getter/setter name: " + data.operator; + e.message === "Unexpected token: operator (" + data.operator + ")"; }; }; diff --git a/test/sandbox.js b/test/sandbox.js index eb9f1f0f..c155f91c 100644 --- a/test/sandbox.js +++ b/test/sandbox.js @@ -1,14 +1,16 @@ var vm = require("vm"); -function safe_log(arg) { +function safe_log(arg, level) { if (arg) switch (typeof arg) { case "function": return arg.toString(); case "object": if (/Error$/.test(arg.name)) return arg.toString(); arg.constructor.toString(); - for (var key in arg) { - arg[key] = safe_log(arg[key]); + if (level--) for (var key in arg) { + if (!Object.getOwnPropertyDescriptor(arg, key).get) { + arg[key] = safe_log(arg[key], level); + } } } return arg; @@ -48,7 +50,9 @@ exports.run_code = function(code) { ].join("\n"), { console: { log: function() { - return console.log.apply(console, [].map.call(arguments, safe_log)); + return console.log.apply(console, [].map.call(arguments, function(arg) { + return safe_log(arg, 3); + })); } } }, { timeout: 5000 }); diff --git a/test/ufuzz.js b/test/ufuzz.js index 0a043c2f..6e716c5b 100644 --- a/test/ufuzz.js +++ b/test/ufuzz.js @@ -48,7 +48,6 @@ var STMT_COUNT_FROM_GLOBAL = true; // count statement depth from nearest functio var num_iterations = +process.argv[2] || 1/0; var verbose = false; // log every generated test var verbose_interval = false; // log every 100 generated tests -var verbose_error = false; var use_strict = false; for (var i = 2; i < process.argv.length; ++i) { switch (process.argv[i]) { @@ -58,9 +57,6 @@ for (var i = 2; i < process.argv.length; ++i) { case '-V': verbose_interval = true; break; - case '-E': - verbose_error = true; - break; case '-t': MAX_GENERATED_TOPLEVELS_PER_RUN = +process.argv[++i]; if (!MAX_GENERATED_TOPLEVELS_PER_RUN) throw new Error('Must generate at least one toplevel per run'); @@ -103,7 +99,6 @@ for (var i = 2; i < process.argv.length; ++i) { console.log(': generate this many cases (if used must be first arg)'); console.log('-v: print every generated test case'); console.log('-V: print every 100th generated test case'); - console.log('-E: print generated test case with runtime error'); console.log('-t : generate this many toplevels per run (more take longer)'); console.log('-r : maximum recursion depth for generator (higher takes longer)'); console.log('-s1 : force the first level statement to be this one (see list below)'); @@ -192,12 +187,33 @@ var ASSIGNMENTS = [ '=', '=', '=', + '=', + '=', + '=', + '=', - '==', - '!=', - '===', - '!==', + '=', + '=', + '=', + '=', + '=', + '=', + '=', + '=', + '=', + '=', + + '+=', + '+=', + '+=', + '+=', '+=', + '+=', + '+=', + '+=', + '+=', + '+=', + '-=', '*=', '/=', @@ -207,7 +223,8 @@ var ASSIGNMENTS = [ '<<=', '>>=', '>>>=', - '%=' ]; + '%=', +]; var UNARY_SAFE = [ '+', @@ -359,7 +376,6 @@ function createFunction(recurmax, inGlobal, noDecl, canThrow, stmtDepth) { // avoid "function statements" (decl inside statements) else if (inGlobal || rng(10) > 0) s += 'var ' + createVarName(MANDATORY) + ' = ' + name + '(' + createArgs() + ');'; - return s; } @@ -406,7 +422,7 @@ function getLabel(label) { return label && " L" + label; } -function createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn, stmtDepth) { +function createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn, stmtDepth, target) { ++stmtDepth; var loop = ++loops; if (--recurmax < 0) { @@ -414,10 +430,11 @@ function createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn } // allow to forcefully generate certain structures at first or second recursion level - var target = 0; - if (stmtDepth === 1 && STMT_FIRST_LEVEL_OVERRIDE >= 0) target = STMT_FIRST_LEVEL_OVERRIDE; - else if (stmtDepth === 2 && STMT_SECOND_LEVEL_OVERRIDE >= 0) target = STMT_SECOND_LEVEL_OVERRIDE; - else target = STMTS_TO_USE[rng(STMTS_TO_USE.length)]; + if (target === undefined) { + if (stmtDepth === 1 && STMT_FIRST_LEVEL_OVERRIDE >= 0) target = STMT_FIRST_LEVEL_OVERRIDE; + else if (stmtDepth === 2 && STMT_SECOND_LEVEL_OVERRIDE >= 0) target = STMT_SECOND_LEVEL_OVERRIDE; + else target = STMTS_TO_USE[rng(STMTS_TO_USE.length)]; + } switch (target) { case STMT_BLOCK: @@ -636,7 +653,7 @@ function _createExpression(recurmax, noComma, stmtDepth, canThrow) { strictMode() ); if (instantiate) for (var i = rng(4); --i >= 0;) { - if (rng(2)) s.push('this.' + getDotKey() + createAssignment() + _createBinaryExpr(recurmax, noComma, stmtDepth, canThrow) + ';'); + if (rng(2)) s.push('this.' + getDotKey(true) + createAssignment() + _createBinaryExpr(recurmax, noComma, stmtDepth, canThrow) + ';'); else s.push('this[' + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + ']' + createAssignment() + _createBinaryExpr(recurmax, noComma, stmtDepth, canThrow) + ';'); } s.push( @@ -689,19 +706,19 @@ function _createExpression(recurmax, noComma, stmtDepth, canThrow) { ") || " + rng(10) + ").toString()[" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + "] "; case p++: - return createArrayLiteral(recurmax, COMMA_OK, stmtDepth, canThrow); + return createArrayLiteral(recurmax, stmtDepth, canThrow); case p++: - return createObjectLiteral(recurmax, COMMA_OK, stmtDepth, canThrow); + return createObjectLiteral(recurmax, stmtDepth, canThrow); case p++: - return createArrayLiteral(recurmax, COMMA_OK, stmtDepth, canThrow) + '[' + + return createArrayLiteral(recurmax, stmtDepth, canThrow) + '[' + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + ']'; case p++: - return createObjectLiteral(recurmax, COMMA_OK, stmtDepth, canThrow) + '[' + + return createObjectLiteral(recurmax, stmtDepth, canThrow) + '[' + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + ']'; case p++: - return createArrayLiteral(recurmax, COMMA_OK, stmtDepth, canThrow) + '.' + getDotKey(); + return createArrayLiteral(recurmax, stmtDepth, canThrow) + '.' + getDotKey(); case p++: - return createObjectLiteral(recurmax, COMMA_OK, stmtDepth, canThrow) + '.' + getDotKey(); + return createObjectLiteral(recurmax, stmtDepth, canThrow) + '.' + getDotKey(); case p++: var name = getVarName(); return name + ' && ' + name + '[' + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + ']'; @@ -713,7 +730,7 @@ function _createExpression(recurmax, noComma, stmtDepth, canThrow) { return _createExpression(recurmax, noComma, stmtDepth, canThrow); } -function createArrayLiteral(recurmax, noComma, stmtDepth, canThrow) { +function createArrayLiteral(recurmax, stmtDepth, canThrow) { recurmax--; var arr = "["; for (var i = rng(6); --i >= 0;) { @@ -746,18 +763,56 @@ var KEYS = [ "3", ].concat(SAFE_KEYS); -function getDotKey() { - return SAFE_KEYS[rng(SAFE_KEYS.length)]; +function getDotKey(assign) { + var key; + do { + key = SAFE_KEYS[rng(SAFE_KEYS.length)]; + } while (assign && key == "length"); + return key; +} + +function createAccessor(recurmax, stmtDepth, canThrow) { + var namesLenBefore = VAR_NAMES.length; + var s; + var prop1 = getDotKey(); + if (rng(2) == 0) { + s = [ + 'get ' + prop1 + '(){', + strictMode(), + createStatements(2, recurmax, canThrow, CANNOT_BREAK, CANNOT_CONTINUE, CAN_RETURN, stmtDepth), + createStatement(recurmax, canThrow, CANNOT_BREAK, CANNOT_CONTINUE, CAN_RETURN, stmtDepth, STMT_RETURN_ETC), + '},' + ].join('\n'); + } else { + var prop2; + do { + prop2 = getDotKey(); + } while (prop1 == prop2); + s = [ + 'set ' + prop1 + '(' + createVarName(MANDATORY) + '){', + strictMode(), + createStatements(2, recurmax, canThrow, CANNOT_BREAK, CANNOT_CONTINUE, CAN_RETURN, stmtDepth), + 'this.' + prop2 + createAssignment() + _createBinaryExpr(recurmax, COMMA_OK, stmtDepth, canThrow) + ';', + '},' + ].join('\n'); + } + VAR_NAMES.length = namesLenBefore; + return s; } -function createObjectLiteral(recurmax, noComma, stmtDepth, canThrow) { +function createObjectLiteral(recurmax, stmtDepth, canThrow) { recurmax--; - var obj = "({"; + var obj = ['({']; for (var i = rng(6); --i >= 0;) { - var key = KEYS[rng(KEYS.length)]; - obj += key + ":(" + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + "), "; + if (rng(20) == 0) { + obj.push(createAccessor(recurmax, stmtDepth, canThrow)); + } else { + var key = KEYS[rng(KEYS.length)]; + obj.push(key + ':(' + createExpression(recurmax, COMMA_OK, stmtDepth, canThrow) + '),'); + } } - return obj + "})"; + obj.push('})'); + return obj.join('\n'); } function createNestedBinaryExpr(recurmax, noComma, stmtDepth, canThrow) { @@ -787,7 +842,7 @@ function _createSimpleBinaryExpr(recurmax, noComma, stmtDepth, canThrow) { return canThrow && rng(10) == 0 ? expr : '(' + assignee + ' && ' + expr + ')'; case 4: assignee = getVarName(); - expr = '(' + assignee + '.' + getDotKey() + createAssignment() + expr = '(' + assignee + '.' + getDotKey(true) + createAssignment() + _createBinaryExpr(recurmax, noComma, stmtDepth, canThrow) + ')'; return canThrow && rng(10) == 0 ? expr : '(' + assignee + ' && ' + expr + ')'; default: @@ -992,7 +1047,7 @@ for (var round = 1; round <= num_iterations; round++) { } } if (verbose || (verbose_interval && !(round % INTERVAL_COUNT)) || !ok) log(options); - else if (verbose_error && typeof original_result != "string") { + else if (typeof original_result != "string") { console.log("//============================================================="); console.log("// original code"); try_beautify(original_code, original_result);