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

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) {
+            if (M.kind == "init") return new AST_ObjectKeyVal(args);
-              case "init":
+            args.key = new AST_SymbolAccessor({
-                return new AST_ObjectKeyVal(args);
+                name: args.key
-              case "set":
+            });
-                args.value.name = from_moz(key);
+            args.value = new AST_Accessor(args.value);
-                return new AST_ObjectSetter(args);
+            if (M.kind == "get") return new AST_ObjectGetter(args);
-              case "get":
+            if (M.kind == "set") return new AST_ObjectSetter(args);
-                args.value.name = from_moz(key);
-                return new AST_ObjectGetter(args);
-            }
         },
         ArrayExpression: function(M) {
             return new AST_Array({
@@ -256,10 +252,7 @@
     map("CallExpression", AST_Call, "callee>expression, arguments@args");
 
     def_to_moz(AST_Toplevel, function To_Moz_Program(M) {
-        return {
+        return to_moz_scope("Program", M);
-            type: "Program",
-            body: M.body.map(to_moz)
-        };
     });
 
     def_to_moz(AST_Defun, function To_Moz_FunctionDeclaration(M) {
@@ -267,7 +260,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)
         }
     });
 
@@ -276,7 +269,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)
         }
     });
 
@@ -382,11 +375,10 @@
     });
 
     def_to_moz(AST_ObjectProperty, function To_Moz_Property(M) {
-        var key = (
+        var key = {
-            is_identifier(M.key)
+            type: "Literal",
-            ? {type: "Identifier", name: M.key}
+            value: M.key instanceof AST_SymbolAccessor ? M.key.name : M.key
-            : {type: "Literal", value: M.key}
+        };
-        );
         var kind;
         if (M instanceof AST_ObjectKeyVal) {
             kind = "init";
@@ -547,8 +539,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 + ")")(
+        me_to_moz = new Function("to_moz", "to_moz_block", "to_moz_scope", "return(" + me_to_moz + ")")(
-            to_moz, to_moz_block
+            to_moz, to_moz_block, to_moz_scope
         );
         MOZ_TO_ME[moztype] = moz_to_me;
         def_to_moz(mytype, me_to_moz);
@@ -606,4 +598,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
+        };
+    };
 })();

lib/parse.js

@@ -807,24 +807,20 @@ function parse($TEXT, options) {
         handle_regexp();
         switch (S.token.type) {
           case "string":
-            var dir = false;
+            if (S.in_directives) {
-            if (S.in_directives === true) {
+                tmp = peek();
-                if ((is_token(peek(), "punc", ";") || peek().nlb) && S.token.raw.indexOf("\\") === -1) {
+                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 dir ? new AST_Directive(stat.body) : stat;
-                return new AST_Directive({
-                    start : stat.body.start,
-                    end   : stat.body.end,
-                    quote : stat.body.quote,
-                    value : stat.body.value,
-                });
-            }
-            return stat;
           case "num":
           case "regexp":
           case "operator":

package.json

@@ -33,10 +33,7 @@
     "yargs": "~3.10.0"
   },
   "devDependencies": {
-    "acorn": "~0.6.0",
+    "acorn": "~5.0.3",
-    "escodegen": "~1.3.3",
-    "esfuzz": "~0.3.1",
-    "estraverse": "~1.5.1",
     "mocha": "~2.3.4"
   },
   "optionalDependencies": {

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], {fromString: true, compress: {collapse_vars: true, side_effects: true}}).code,
+                uglify.minify(tests[i][0], {fromString: true}).code,
                 tests[i][1],
                 tests[i][0]
             );

test/mozilla-ast.js

@@ -1,103 +1,87 @@
 // Testing UglifyJS <-> SpiderMonkey AST conversion
-// through generative testing.
+"use strict";
 
-var UglifyJS = require(".."),
+var acorn = require("acorn");
-    escodegen = require("escodegen"),
+var ufuzz = require("./ufuzz");
-    esfuzz = require("esfuzz"),
+var UglifyJS = require("..");
-    estraverse = require("estraverse"),
-    prefix = "\r    ";
 
-// Normalizes input AST for UglifyJS in order to get correct comparison.
+function try_beautify(code) {
-
+    var beautified;
-function normalizeInput(ast) {
+    try {
-    return estraverse.replace(ast, {
+        beautified = UglifyJS.minify(code, {
-        enter: function(node, parent) {
+            fromString: true,
-            switch (node.type) {
+            compress: false,
-                // Internally mark all the properties with semi-standard type "Property".
+            mangle: false,
-                case "ObjectExpression":
+            output: {
-                    node.properties.forEach(function (property) {
+                beautify: true,
-                        property.type = "Property";
+                bracketize: true
-                    });
-                    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;
-            }
-        }
-    });
-}
-
-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)
             }
         });
-
+    } catch (ex) {
-        if (jsPair[0].value !== jsPair[1].value) {
+        beautified = { error: ex };
-            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.");
-        }
     }
+    if (beautified.error) {
+        console.log("// !!! beautify failed !!!");
+        console.log(beautified.error.stack);
+        console.log(code);
+    } else {
+        console.log("// (beautified)");
+        console.log(beautified.code);
+    }
+}
 
-    process.stdout.write(prefix + "Probability of error is less than " + (100 / options.iterations) + "%, stopping.\n");
+function test(original, estree, description) {
-};
+    var transformed;
+    try {
+        transformed = UglifyJS.minify(estree, {
+            fromString: true,
+            compress: false,
+            mangle: false,
+            spidermonkey: true
+        });
+    } catch (ex) {
+        transformed = { error: ex };
+    }
+    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);
+    }
+}
+
+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 = {
+        ast: UglifyJS.parse(code),
+        code: UglifyJS.minify(code, {
+            fromString: true,
+            compress: false,
+            mangle: false
+        }).code
+    };
+    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();

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() {

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 <int>: maximum recursion depth for generator (higher takes longer)');
         console.log('-s1 <statement name>: force the first level statement to be this one (see list below)');
         console.log('-s2 <statement name>: 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 <statement names>: 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);
+    var name;
-    if (name === 'a' || name === 'b' || name === 'c') name = 'f' + func; // quick hack to prevent assignment to func names of being called
+    if (inGlobal || rng(5) > 0) name = 'f' + func;
-    var s = '';
+    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 = [
+        s.push(createFunctions(rng(5) + 1, Math.ceil(recurmax * 0.7), NOT_GLOBAL, ANY_TYPE, canThrow, stmtDepth));
-            'function ' + name + '(' + createParams() + '){',
-            strictMode(),
-            createFunctions(rng(5) + 1, Math.ceil(recurmax * 0.7), NOT_GLOBAL, ANY_TYPE, canThrow, stmtDepth),
-            '}',
-            ''
-        ].join('\n');
     } else {
         // functions with statements
-        s = [
+        s.push(createStatements(3, recurmax, canThrow, CANNOT_BREAK, CANNOT_CONTINUE, CAN_RETURN, stmtDepth));
-            'function ' + name + '(' + createParams() + '){',
-            strictMode(),
-            createStatements(3, recurmax, canThrow, CANNOT_BREAK, CANNOT_CONTINUE, CAN_RETURN, stmtDepth),
-            '}',
-            ''
-        ].join('\n');
     }
+    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) {
+        var name;
-            name += '_' + suffix;
+        do {
-            if (!dontStore) VAR_NAMES.push(name);
+            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) {
     try {
         var beautified = UglifyJS.minify(code, {
@@ -1033,10 +1064,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) {