From: Alex Lam S.L Date: Sun, 14 May 2017 18:37:53 +0000 (+0800) Subject: fix & improve coverage of `estree` (#1935) X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=e005099fb1b9a1b87ac50ba8223255e52cec452d;p=UglifyJS.git fix & improve coverage of `estree` (#1935) - fix `estree` conversion of getter/setter - fix non-directive literal in `to_mozilla_ast()` - revamp `test/mozilla-ast.js` - reuse `test/ufuzz.js` for code generation - use `acorn.parse()` for creating `estree` - extend `test/ufuzz.js` for `acorn` workaround - catch variable redefinition - non-trivial literal as directive - adjust options for tolerance Miscellaneous - optional semi-colon when parsing directives fixes #1914 closes #1915 --- diff --git a/lib/mozilla-ast.js b/lib/mozilla-ast.js index 7d246758..8d7ee4b8 100644 --- a/lib/mozilla-ast.js +++ b/lib/mozilla-ast.js @@ -111,23 +111,19 @@ }, Property: function(M) { var key = M.key; - var name = key.type == "Identifier" ? key.name : key.value; var args = { start : my_start_token(key), end : my_end_token(M.value), - key : name, + key : key.type == "Identifier" ? key.name : key.value, value : from_moz(M.value) }; - switch (M.kind) { - case "init": - return new AST_ObjectKeyVal(args); - case "set": - args.value.name = from_moz(key); - return new AST_ObjectSetter(args); - case "get": - args.value.name = from_moz(key); - return new AST_ObjectGetter(args); - } + if (M.kind == "init") return new AST_ObjectKeyVal(args); + args.key = new AST_SymbolAccessor({ + name: args.key + }); + args.value = new AST_Accessor(args.value); + if (M.kind == "get") return new AST_ObjectGetter(args); + if (M.kind == "set") return new AST_ObjectSetter(args); }, ArrayExpression: function(M) { return new AST_Array({ @@ -260,10 +256,7 @@ map("CallExpression", AST_Call, "callee>expression, arguments@args"); def_to_moz(AST_Toplevel, function To_Moz_Program(M) { - return { - type: "Program", - body: M.body.map(to_moz) - }; + return to_moz_scope("Program", M); }); def_to_moz(AST_Defun, function To_Moz_FunctionDeclaration(M) { @@ -271,7 +264,7 @@ type: "FunctionDeclaration", id: to_moz(M.name), params: M.argnames.map(to_moz), - body: to_moz_block(M) + body: to_moz_scope("BlockStatement", M) } }); @@ -280,7 +273,7 @@ type: "FunctionExpression", id: to_moz(M.name), params: M.argnames.map(to_moz), - body: to_moz_block(M) + body: to_moz_scope("BlockStatement", M) } }); @@ -386,11 +379,10 @@ }); def_to_moz(AST_ObjectProperty, function To_Moz_Property(M) { - var key = ( - is_identifier(M.key) - ? {type: "Identifier", name: M.key} - : {type: "Literal", value: M.key} - ); + var key = { + type: "Literal", + value: M.key instanceof AST_SymbolAccessor ? M.key.name : M.key + }; var kind; if (M instanceof AST_ObjectKeyVal) { kind = "init"; @@ -551,8 +543,8 @@ moz_to_me = new Function("U2", "my_start_token", "my_end_token", "from_moz", "return(" + moz_to_me + ")")( exports, my_start_token, my_end_token, from_moz ); - me_to_moz = new Function("to_moz", "to_moz_block", "return(" + me_to_moz + ")")( - to_moz, to_moz_block + me_to_moz = new Function("to_moz", "to_moz_block", "to_moz_scope", "return(" + me_to_moz + ")")( + to_moz, to_moz_block, to_moz_scope ); MOZ_TO_ME[moztype] = moz_to_me; def_to_moz(mytype, me_to_moz); @@ -610,4 +602,14 @@ }; }; + function to_moz_scope(type, node) { + var body = node.body.map(to_moz); + if (node.body[0] instanceof AST_SimpleStatement && node.body[0].body instanceof AST_String) { + body.unshift(to_moz(new AST_EmptyStatement(node.body[0]))); + } + return { + type: type, + body: body + }; + }; })(); diff --git a/lib/parse.js b/lib/parse.js index f1089501..74c00b74 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -805,24 +805,20 @@ function parse($TEXT, options) { handle_regexp(); switch (S.token.type) { case "string": - var dir = false; - if (S.in_directives === true) { - if ((is_token(peek(), "punc", ";") || peek().nlb) && S.token.raw.indexOf("\\") === -1) { + if (S.in_directives) { + tmp = peek(); + if (S.token.raw.indexOf("\\") == -1 + && (tmp.nlb + || is_token(tmp, "eof") + || is_token(tmp, "punc", ";") + || is_token(tmp, "punc", "}"))) { S.input.add_directive(S.token.value); } else { S.in_directives = false; } } var dir = S.in_directives, stat = simple_statement(); - if (dir) { - return new AST_Directive({ - start : stat.body.start, - end : stat.body.end, - quote : stat.body.quote, - value : stat.body.value, - }); - } - return stat; + return dir ? new AST_Directive(stat.body) : stat; case "num": case "regexp": case "operator": diff --git a/package.json b/package.json index 6de380eb..04b49f80 100644 --- a/package.json +++ b/package.json @@ -33,10 +33,7 @@ "source-map": "~0.5.1" }, "devDependencies": { - "acorn": "~0.6.0", - "escodegen": "~1.3.3", - "esfuzz": "~0.3.1", - "estraverse": "~1.5.1", + "acorn": "~5.0.3", "mocha": "~2.3.4" }, "optionalDependencies": { diff --git a/test/mocha/directives.js b/test/mocha/directives.js index ab8ad57f..256bb870 100644 --- a/test/mocha/directives.js +++ b/test/mocha/directives.js @@ -351,18 +351,28 @@ describe("Directives", function() { var tests = [ [ '"use strict";"use strict";"use strict";"use foo";"use strict";;"use sloppy";doSomething("foo");', - '"use strict";"use foo";doSomething("foo");' + '"use strict";"use foo";doSomething("foo");', + 'function f(){ "use strict" }', + 'function f(){ "use asm" }', + 'function f(){ "use nondirective" }', + 'function f(){ ;"use strict" }', + 'function f(){ "use \n"; }', ], [ // Nothing gets optimised in the compressor because "use asm" is the first statement '"use asm";"use\\x20strict";1+1;', - '"use asm";;"use strict";1+1;' // Yet, the parser noticed that "use strict" wasn't a directive + '"use asm";;"use strict";1+1;', // Yet, the parser noticed that "use strict" wasn't a directive + 'function f(){"use strict"}', + 'function f(){"use asm"}', + 'function f(){"use nondirective"}', + 'function f(){}', + 'function f(){}', ] ]; for (var i = 0; i < tests.length; i++) { assert.strictEqual( - uglify.minify(tests[i][0], {compress: {collapse_vars: true, side_effects: true}}).code, + uglify.minify(tests[i][0]).code, tests[i][1], tests[i][0] ); diff --git a/test/mozilla-ast.js b/test/mozilla-ast.js index 544ce8bc..b8026de5 100644 --- a/test/mozilla-ast.js +++ b/test/mozilla-ast.js @@ -1,103 +1,73 @@ // Testing UglifyJS <-> SpiderMonkey AST conversion -// through generative testing. - -var UglifyJS = require("./node"), - escodegen = require("escodegen"), - esfuzz = require("esfuzz"), - estraverse = require("estraverse"), - prefix = "\r "; - -// Normalizes input AST for UglifyJS in order to get correct comparison. - -function normalizeInput(ast) { - return estraverse.replace(ast, { - enter: function(node, parent) { - switch (node.type) { - // Internally mark all the properties with semi-standard type "Property". - case "ObjectExpression": - node.properties.forEach(function (property) { - property.type = "Property"; - }); - break; - - // Since UglifyJS doesn"t recognize different types of property keys, - // decision on SpiderMonkey node type is based on check whether key - // can be valid identifier or not - so we do in input AST. - case "Property": - var key = node.key; - if (key.type === "Literal" && typeof key.value === "string" && UglifyJS.is_identifier(key.value)) { - node.key = { - type: "Identifier", - name: key.value - }; - } else if (key.type === "Identifier" && !UglifyJS.is_identifier(key.name)) { - node.key = { - type: "Literal", - value: key.name - }; - } - break; - - // UglifyJS internally flattens all the expression sequences - either - // to one element (if sequence contains only one element) or flat list. - case "SequenceExpression": - node.expressions = node.expressions.reduce(function flatten(list, expr) { - return list.concat(expr.type === "SequenceExpression" ? expr.expressions.reduce(flatten, []) : [expr]); - }, []); - if (node.expressions.length === 1) { - return node.expressions[0]; - } - break; - } +"use strict"; + +var acorn = require("acorn"); +var ufuzz = require("./ufuzz"); +var UglifyJS = require(".."); + +function try_beautify(code) { + var beautified = UglifyJS.minify(code, { + compress: false, + mangle: false, + output: { + beautify: true, + bracketize: true } }); + if (beautified.error) { + console.log("// !!! beautify failed !!!"); + console.log(beautified.error.stack); + console.log(code); + } else { + console.log("// (beautified)"); + console.log(beautified.code); + } } -module.exports = function(options) { - console.log("--- UglifyJS <-> Mozilla AST conversion"); - - for (var counter = 0; counter < options.iterations; counter++) { - process.stdout.write(prefix + counter + "/" + options.iterations); - - var ast1 = normalizeInput(esfuzz.generate({ - maxDepth: options.maxDepth - })); - - var ast2 = - UglifyJS - .AST_Node - .from_mozilla_ast(ast1) - .to_mozilla_ast(); - - var astPair = [ - {name: 'expected', value: ast1}, - {name: 'actual', value: ast2} - ]; - - var jsPair = astPair.map(function(item) { - return { - name: item.name, - value: escodegen.generate(item.value) - } - }); - - if (jsPair[0].value !== jsPair[1].value) { - var fs = require("fs"); - var acorn = require("acorn"); - - fs.existsSync("tmp") || fs.mkdirSync("tmp"); - - jsPair.forEach(function (item) { - var fileName = "tmp/dump_" + item.name; - var ast = acorn.parse(item.value); - fs.writeFileSync(fileName + ".js", item.value); - fs.writeFileSync(fileName + ".json", JSON.stringify(ast, null, 2)); - }); - - process.stdout.write("\n"); - throw new Error("Got different outputs, check out tmp/dump_*.{js,json} for codes and ASTs."); +function test(original, estree, description) { + var transformed = UglifyJS.minify(UglifyJS.AST_Node.from_mozilla_ast(estree), { + compress: false, + mangle: false + }); + if (transformed.error || original !== transformed.code) { + console.log("//============================================================="); + console.log("// !!!!!! Failed... round", round); + console.log("// original code"); + try_beautify(original); + console.log(); + console.log(); + console.log("//-------------------------------------------------------------"); + console.log("//", description); + if (transformed.error) { + console.log(transformed.error.stack); + } else { + try_beautify(transformed.code); } + console.log("!!!!!! Failed... round", round); + process.exit(1); } +} - process.stdout.write(prefix + "Probability of error is less than " + (100 / options.iterations) + "%, stopping.\n"); -}; +var num_iterations = ufuzz.num_iterations; +for (var round = 1; round <= num_iterations; round++) { + process.stdout.write(round + " of " + num_iterations + "\r"); + var code = ufuzz.createTopLevelCode(); + var uglified = UglifyJS.minify(code, { + compress: false, + mangle: false, + output: { + ast: true + } + }); + test(uglified.code, uglified.ast.to_mozilla_ast(), "AST_Node.to_mozilla_ast()"); + try { + test(uglified.code, acorn.parse(code), "acorn.parse()"); + } catch (e) { + console.log("//============================================================="); + console.log("// acorn parser failed... round", round); + console.log(e); + console.log("// original code"); + console.log(code); + } +} +console.log(); diff --git a/test/run-tests.js b/test/run-tests.js index 05afc1b5..edcd7cc9 100755 --- a/test/run-tests.js +++ b/test/run-tests.js @@ -23,12 +23,6 @@ mocha_tests(); var run_sourcemaps_tests = require('./sourcemaps'); run_sourcemaps_tests(); -var run_ast_conversion_tests = require("./mozilla-ast"); - -run_ast_conversion_tests({ - iterations: 1000 -}); - /* -----[ utils ]----- */ function tmpl() { diff --git a/test/ufuzz.js b/test/ufuzz.js index 6e716c5b..ff403c60 100644 --- a/test/ufuzz.js +++ b/test/ufuzz.js @@ -49,6 +49,8 @@ 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 use_strict = false; +var catch_redef = require.main === module; +var generate_directive = require.main === module; for (var i = 2; i < process.argv.length; ++i) { switch (process.argv[i]) { case '-v': @@ -75,6 +77,12 @@ for (var i = 2; i < process.argv.length; ++i) { STMT_SECOND_LEVEL_OVERRIDE = STMT_ARG_TO_ID[name]; if (!(STMT_SECOND_LEVEL_OVERRIDE >= 0)) throw new Error('Unknown statement name; use -? to get a list'); break; + case '--no-catch-redef': + catch_redef = false; + break; + case '--no-directive': + generate_directive = false; + break; case '--use-strict': use_strict = true; break; @@ -103,6 +111,8 @@ for (var i = 2; i < process.argv.length; ++i) { 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)'); console.log('-s2 : force the second level statement to be this one (see list below)'); + console.log('--no-catch-redef: do not redefine catch variables'); + console.log('--no-directive: do not generate directives'); console.log('--use-strict: generate "use strict"'); console.log('--stmt-depth-from-func: reset statement depth counter at each function, counts from global otherwise'); console.log('--only-stmt : a comma delimited white list of statements that may be generated'); @@ -293,6 +303,7 @@ var TYPEOF_OUTCOMES = [ 'symbol', 'crap' ]; +var unique_vars = []; var loops = 0; var funcs = 0; var labels = 10000; @@ -307,6 +318,10 @@ function strictMode() { } function createTopLevelCode() { + VAR_NAMES.length = INITIAL_NAMES_LEN; // prune any previous names still in the list + unique_vars.length = 0; + loops = 0; + funcs = 0; return [ strictMode(), 'var a = 100, b = 10, c = 0;', @@ -342,33 +357,36 @@ function createArgs() { return args.join(', '); } +function filterDirective(s) { + if (!generate_directive && !s[1] && /\("/.test(s[2])) s[2] = ';' + s[2]; + return s; +} + function createFunction(recurmax, inGlobal, noDecl, canThrow, stmtDepth) { if (--recurmax < 0) { return ';'; } if (!STMT_COUNT_FROM_GLOBAL) stmtDepth = 0; var func = funcs++; var namesLenBefore = VAR_NAMES.length; - var name = (inGlobal || rng(5) > 0) ? 'f' + func : createVarName(MANDATORY, noDecl); - if (name === 'a' || name === 'b' || name === 'c') name = 'f' + func; // quick hack to prevent assignment to func names of being called - var s = ''; + var name; + if (inGlobal || rng(5) > 0) name = 'f' + func; + else { + unique_vars.push('a', 'b', 'c'); + name = createVarName(MANDATORY, noDecl); + unique_vars.length -= 3; + } + var s = [ + 'function ' + name + '(' + createParams() + '){', + strictMode() + ]; if (rng(5) === 0) { // functions with functions. lower the recursion to prevent a mess. - s = [ - 'function ' + name + '(' + createParams() + '){', - strictMode(), - createFunctions(rng(5) + 1, Math.ceil(recurmax * 0.7), NOT_GLOBAL, ANY_TYPE, canThrow, stmtDepth), - '}', - '' - ].join('\n'); + s.push(createFunctions(rng(5) + 1, Math.ceil(recurmax * 0.7), NOT_GLOBAL, ANY_TYPE, canThrow, stmtDepth)); } else { // functions with statements - s = [ - 'function ' + name + '(' + createParams() + '){', - strictMode(), - createStatements(3, recurmax, canThrow, CANNOT_BREAK, CANNOT_CONTINUE, CAN_RETURN, stmtDepth), - '}', - '' - ].join('\n'); + s.push(createStatements(3, recurmax, canThrow, CANNOT_BREAK, CANNOT_CONTINUE, CAN_RETURN, stmtDepth)); } + s.push('}', ''); + s = filterDirective(s).join('\n'); VAR_NAMES.length = namesLenBefore; @@ -477,20 +495,22 @@ function createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn case STMT_VAR: switch (rng(3)) { case 0: + unique_vars.push('c'); var name = createVarName(MANDATORY); - if (name === 'c') name = 'a'; + unique_vars.pop(); return 'var ' + name + ';'; case 1: // initializer can only have one expression + unique_vars.push('c'); var name = createVarName(MANDATORY); - if (name === 'c') name = 'b'; + unique_vars.pop(); return 'var ' + name + ' = ' + createExpression(recurmax, NO_COMMA, stmtDepth, canThrow) + ';'; default: // initializer can only have one expression + unique_vars.push('c'); var n1 = createVarName(MANDATORY); - if (n1 === 'c') n1 = 'b'; var n2 = createVarName(MANDATORY); - if (n2 === 'c') n2 = 'b'; + unique_vars.pop(); return 'var ' + n1 + ' = ' + createExpression(recurmax, NO_COMMA, stmtDepth, canThrow) + ', ' + n2 + ' = ' + createExpression(recurmax, NO_COMMA, stmtDepth, canThrow) + ';'; } case STMT_RETURN_ETC: @@ -531,8 +551,11 @@ function createStatement(recurmax, canThrow, canBreak, canContinue, cannotReturn var nameLenBefore = VAR_NAMES.length; var catchName = createVarName(MANDATORY); var freshCatchName = VAR_NAMES.length !== nameLenBefore; + if (!catch_redef) unique_vars.push(catchName); s += ' catch (' + catchName + ') { ' + createStatements(3, recurmax, canThrow, canBreak, canContinue, cannotReturn, stmtDepth) + ' }'; - if (freshCatchName) VAR_NAMES.splice(nameLenBefore, 1); // remove catch name + // remove catch name + if (!catch_redef) unique_vars.pop(); + if (freshCatchName) VAR_NAMES.splice(nameLenBefore, 1); } if (n !== 0) s += ' finally { ' + createStatements(3, recurmax, canThrow, canBreak, canContinue, cannotReturn, stmtDepth) + ' }'; return s; @@ -610,8 +633,9 @@ function _createExpression(recurmax, noComma, stmtDepth, canThrow) { case p++: case p++: var nameLenBefore = VAR_NAMES.length; + unique_vars.push('c'); var name = createVarName(MAYBE); // note: this name is only accessible from _within_ the function. and immutable at that. - if (name == 'c') name = 'a'; + unique_vars.pop(); var s = []; switch (rng(5)) { case 0: @@ -663,7 +687,7 @@ function _createExpression(recurmax, noComma, stmtDepth, canThrow) { break; } VAR_NAMES.length = nameLenBefore; - return s.join('\n'); + return filterDirective(s).join('\n'); case p++: case p++: return createTypeofExpr(recurmax, stmtDepth, canThrow); @@ -782,7 +806,7 @@ function createAccessor(recurmax, stmtDepth, canThrow) { 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 { @@ -794,10 +818,10 @@ function createAccessor(recurmax, stmtDepth, canThrow) { 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; + return filterDirective(s).join('\n'); } function createObjectLiteral(recurmax, stmtDepth, canThrow) { @@ -899,17 +923,24 @@ function getVarName() { function createVarName(maybe, dontStore) { if (!maybe || rng(2)) { - var name = VAR_NAMES[rng(VAR_NAMES.length)]; var suffix = rng(3); - if (suffix) { - name += '_' + suffix; - if (!dontStore) VAR_NAMES.push(name); - } + var name; + do { + name = VAR_NAMES[rng(VAR_NAMES.length)]; + if (suffix) name += '_' + suffix; + } while (unique_vars.indexOf(name) >= 0); + if (suffix && !dontStore) VAR_NAMES.push(name); return name; } return ''; } +if (require.main !== module) { + exports.createTopLevelCode = createTopLevelCode; + exports.num_iterations = num_iterations; + return; +} + function try_beautify(code, result) { var beautified = UglifyJS.minify(code, { compress: false, @@ -1028,10 +1059,6 @@ var uglify_code, uglify_result, ok; for (var round = 1; round <= num_iterations; round++) { process.stdout.write(round + " of " + num_iterations + "\r"); - VAR_NAMES.length = INITIAL_NAMES_LEN; // prune any previous names still in the list - loops = 0; - funcs = 0; - original_code = createTopLevelCode(); original_result = sandbox.run_code(original_code); (typeof original_result != "string" ? fallback_options : minify_options).forEach(function(options) {