forbid block-scoped AST_Defun in strict mode (#2718)

lib/compress.js

@@ -1746,7 +1746,7 @@ merge(Compressor.prototype, {
                 target.push(node);
                 return true;
             }
-            if (node instanceof AST_Defun && (node === stat || !compressor.has_directive("use strict"))) {
+            if (node instanceof AST_Defun) {
                 target.push(node);
                 return true;
             }

lib/parse.js

@@ -800,7 +800,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;
@@ -815,7 +815,7 @@ function parse($TEXT, options) {
         }
     };
 
-    var statement = embed_tokens(function() {
+    var statement = embed_tokens(function(strict_defun) {
         handle_regexp();
         switch (S.token.type) {
           case "string":
@@ -901,6 +901,9 @@ function parse($TEXT, options) {
                 return for_();
 
               case "function":
+                if (!strict_defun && S.input.has_directive("use strict")) {
+                    croak("In strict mode code, functions can only be declared at top level or immediately within another function.");
+                }
                 next();
                 return function_(AST_Defun);
 
@@ -1083,7 +1086,7 @@ function parse($TEXT, options) {
         S.input.push_directives_stack();
         S.in_loop = 0;
         S.labels = [];
-        var body = block_();
+        var body = block_(true);
         if (S.input.has_directive("use strict")) {
             if (name) strict_verify_symbol(name);
             argnames.forEach(strict_verify_symbol);
@@ -1112,12 +1115,12 @@ function parse($TEXT, options) {
         });
     };
 
-    function block_() {
+    function block_(strict_defun) {
         expect("{");
         var a = [];
         while (!is("punc", "}")) {
             if (is("eof")) unexpected();
-            a.push(statement());
+            a.push(statement(strict_defun));
         }
         next();
         return a;
@@ -1630,7 +1633,7 @@ function parse($TEXT, options) {
         var body = [];
         S.input.push_directives_stack();
         while (!is("eof"))
-            body.push(statement());
+            body.push(statement(true));
         S.input.pop_directives_stack();
         var end = prev();
         var toplevel = options.toplevel;

test/compress/dead-code.js

@@ -62,46 +62,6 @@ dead_code_2_should_warn: {
     node_version: "<=4"
 }
 
-dead_code_2_should_warn_strict: {
-    options = {
-        dead_code: true
-    };
-    input: {
-        "use strict";
-        function f() {
-            g();
-            x = 10;
-            throw new Error("foo");
-            // completely discarding the `if` would introduce some
-            // bugs.  UglifyJS v1 doesn't deal with this issue; in v2
-            // we copy any declarations to the upper scope.
-            if (x) {
-                y();
-                var x;
-                function g(){};
-                // but nested declarations should not be kept.
-                (function(){
-                    var q;
-                    function y(){};
-                })();
-            }
-        }
-        f();
-    }
-    expect: {
-        "use strict";
-        function f() {
-            g();
-            x = 10;
-            throw new Error("foo");
-            var x;
-        }
-        f();
-    }
-    expect_stdout: true
-    node_version: ">=4"
-}
-
 dead_code_constant_boolean_should_warn_more: {
     options = {
         dead_code    : true,
@@ -137,42 +97,6 @@ dead_code_constant_boolean_should_warn_more: {
     node_version: "<=4"
 }
 
-dead_code_constant_boolean_should_warn_more_strict: {
-    options = {
-        dead_code    : true,
-        loops        : true,
-        booleans     : true,
-        conditionals : true,
-        evaluate     : true,
-        side_effects : true,
-    };
-    input: {
-        "use strict";
-        while (!((foo && bar) || (x + "0"))) {
-            console.log("unreachable");
-            var foo;
-            function bar() {}
-        }
-        for (var x = 10, y; x && (y || x) && (!typeof x); ++x) {
-            asdf();
-            foo();
-            var moo;
-        }
-        bar();
-    }
-    expect: {
-        "use strict";
-        var foo;
-        // nothing for the while
-        // as for the for, it should keep:
-        var moo;
-        var x = 10, y;
-        bar();
-    }
-    expect_stdout: true
-    node_version: ">=4"
-}
-
 try_catch_finally: {
     options = {
         conditionals: true,

test/compress/functions.js

@@ -214,46 +214,6 @@ hoist_funs: {
     node_version: "<=4"
 }
 
-hoist_funs_strict: {
-    options = {
-        hoist_funs: true,
-    }
-    input: {
-        "use strict";
-        console.log(1, typeof f, typeof g);
-        if (console.log(2, typeof f, typeof g))
-            console.log(3, typeof f, typeof g);
-        else {
-            console.log(4, typeof f, typeof g);
-            function f() {}
-            console.log(5, typeof f, typeof g);
-        }
-        function g() {}
-        console.log(6, typeof f, typeof g);
-    }
-    expect: {
-        "use strict";
-        function g() {}
-        console.log(1, typeof f, typeof g);
-        if (console.log(2, typeof f, typeof g))
-            console.log(3, typeof f, typeof g);
-        else {
-            console.log(4, typeof f, typeof g);
-            function f() {}
-            console.log(5, typeof f, typeof g);
-        }
-        console.log(6, typeof f, typeof g);
-    }
-    expect_stdout: [
-        "1 'undefined' 'function'",
-        "2 'undefined' 'function'",
-        "4 'function' 'function'",
-        "5 'function' 'function'",
-        "6 'undefined' 'function'",
-    ]
-    node_version: ">=4"
-}
-
 issue_203: {
     options = {
         keep_fargs: false,

test/run-tests.js

@@ -100,6 +100,15 @@ function run_compress_tests() {
                 quote_style: 3,
                 keep_quoted_props: true
             });
+            try {
+                U.parse(input_code);
+            } catch (ex) {
+                log("!!! Cannot parse input\n---INPUT---\n{input}\n--PARSE ERROR--\n{error}\n\n", {
+                    input: input_formatted,
+                    error: ex,
+                });
+                return false;
+            }
             var options = U.defaults(test.options, {
                 warnings: false
             });
@@ -139,78 +148,74 @@ function run_compress_tests() {
                     output: output,
                     expected: expect
                 });
-                failures++;
+                return false;
-                failed_files[file] = 1;
             }
-            else {
+            // expect == output
-                // expect == output
+            try {
-                try {
+                U.parse(output);
-                    var reparsed_ast = U.parse(output);
+            } catch (ex) {
-                } catch (ex) {
+                log("!!! Test matched expected result but cannot parse output\n---INPUT---\n{input}\n---OUTPUT---\n{output}\n--REPARSE ERROR--\n{error}\n\n", {
-                    log("!!! Test matched expected result but cannot parse output\n---INPUT---\n{input}\n---OUTPUT---\n{output}\n--REPARSE ERROR--\n{error}\n\n", {
+                    input: input_formatted,
+                    output: output,
+                    error: ex,
+                });
+                return false;
+            }
+            if (test.expect_warnings) {
+                U.AST_Node.warn_function = original_warn_function;
+                var expected_warnings = make_code(test.expect_warnings, {
+                    beautify: false,
+                    quote_style: 2, // force double quote to match JSON
+                });
+                warnings_emitted = warnings_emitted.map(function(input) {
+                    return input.split(process.cwd() + path.sep).join("").split(path.sep).join("/");
+                });
+                var actual_warnings = JSON.stringify(warnings_emitted);
+                if (expected_warnings != actual_warnings) {
+                    log("!!! failed\n---INPUT---\n{input}\n---EXPECTED WARNINGS---\n{expected_warnings}\n---ACTUAL WARNINGS---\n{actual_warnings}\n\n", {
                         input: input_formatted,
-                        output: output,
+                        expected_warnings: expected_warnings,
-                        error: ex.toString(),
+                        actual_warnings: actual_warnings,
                     });
-                    failures++;
+                    return false;
-                    failed_files[file] = 1;
-                }
-                if (test.expect_warnings) {
-                    U.AST_Node.warn_function = original_warn_function;
-                    var expected_warnings = make_code(test.expect_warnings, {
-                        beautify: false,
-                        quote_style: 2, // force double quote to match JSON
-                    });
-                    warnings_emitted = warnings_emitted.map(function(input) {
-                      return input.split(process.cwd() + path.sep).join("").split(path.sep).join("/");
-                    });
-                    var actual_warnings = JSON.stringify(warnings_emitted);
-                    if (expected_warnings != actual_warnings) {
-                        log("!!! failed\n---INPUT---\n{input}\n---EXPECTED WARNINGS---\n{expected_warnings}\n---ACTUAL WARNINGS---\n{actual_warnings}\n\n", {
-                            input: input_formatted,
-                            expected_warnings: expected_warnings,
-                            actual_warnings: actual_warnings,
-                        });
-                        failures++;
-                        failed_files[file] = 1;
-                    }
-                }
-                if (test.expect_stdout
-                    && (!test.node_version || semver.satisfies(process.version, test.node_version))) {
-                    var stdout = sandbox.run_code(input_code);
-                    if (test.expect_stdout === true) {
-                        test.expect_stdout = stdout;
-                    }
-                    if (!sandbox.same_stdout(test.expect_stdout, stdout)) {
-                        log("!!! Invalid input or expected stdout\n---INPUT---\n{input}\n---EXPECTED {expected_type}---\n{expected}\n---ACTUAL {actual_type}---\n{actual}\n\n", {
-                            input: input_formatted,
-                            expected_type: typeof test.expect_stdout == "string" ? "STDOUT" : "ERROR",
-                            expected: test.expect_stdout,
-                            actual_type: typeof stdout == "string" ? "STDOUT" : "ERROR",
-                            actual: stdout,
-                        });
-                        failures++;
-                        failed_files[file] = 1;
-                    } else {
-                        stdout = sandbox.run_code(output);
-                        if (!sandbox.same_stdout(test.expect_stdout, stdout)) {
-                            log("!!! failed\n---INPUT---\n{input}\n---EXPECTED {expected_type}---\n{expected}\n---ACTUAL {actual_type}---\n{actual}\n\n", {
-                                input: input_formatted,
-                                expected_type: typeof test.expect_stdout == "string" ? "STDOUT" : "ERROR",
-                                expected: test.expect_stdout,
-                                actual_type: typeof stdout == "string" ? "STDOUT" : "ERROR",
-                                actual: stdout,
-                            });
-                            failures++;
-                            failed_files[file] = 1;
-                        }
-                    }
                 }
             }
+            if (test.expect_stdout
+                && (!test.node_version || semver.satisfies(process.version, test.node_version))) {
+                var stdout = sandbox.run_code(input_code);
+                if (test.expect_stdout === true) {
+                    test.expect_stdout = stdout;
+                }
+                if (!sandbox.same_stdout(test.expect_stdout, stdout)) {
+                    log("!!! Invalid input or expected stdout\n---INPUT---\n{input}\n---EXPECTED {expected_type}---\n{expected}\n---ACTUAL {actual_type}---\n{actual}\n\n", {
+                        input: input_formatted,
+                        expected_type: typeof test.expect_stdout == "string" ? "STDOUT" : "ERROR",
+                        expected: test.expect_stdout,
+                        actual_type: typeof stdout == "string" ? "STDOUT" : "ERROR",
+                        actual: stdout,
+                    });
+                    return false;
+                }
+                stdout = sandbox.run_code(output);
+                if (!sandbox.same_stdout(test.expect_stdout, stdout)) {
+                    log("!!! failed\n---INPUT---\n{input}\n---EXPECTED {expected_type}---\n{expected}\n---ACTUAL {actual_type}---\n{actual}\n\n", {
+                        input: input_formatted,
+                        expected_type: typeof test.expect_stdout == "string" ? "STDOUT" : "ERROR",
+                        expected: test.expect_stdout,
+                        actual_type: typeof stdout == "string" ? "STDOUT" : "ERROR",
+                        actual: stdout,
+                    });
+                    return false;
+                }
+            }
+            return true;
         }
         var tests = parse_test(path.resolve(dir, file));
         for (var i in tests) if (tests.hasOwnProperty(i)) {
-            test_case(tests[i]);
+            if (!test_case(tests[i])) {
+                failures++;
+                failed_files[file] = 1;
+            }
         }
     }
     files.forEach(function(file){