fix corner case in `directives` (#3645)

5 files changed, 124 insertions, 103 deletions

diff --git a/lib/output.js b/lib/output.js
index d97778b2..cc39c2df 100644
--- a/lib/output.js
+++ b/lib/output.js
@@ -43,8 +43,6 @@
 
 "use strict";
 
-var EXPECT_DIRECTIVE = /^$|[;{][\s\n]*$/;
-
 function is_some_comments(comment) {
     // multiline comment
     return comment.type == "comment2" && /@preserve|@license|@cc_on/i.test(comment.value);
@@ -378,7 +376,7 @@ function OutputStream(options) {
     };
 
     function force_semicolon() {
-        might_need_semicolon = false;
+        if (might_need_semicolon) print(";");
         print(";");
     }
 
@@ -585,17 +583,7 @@ function OutputStream(options) {
         force_semicolon : force_semicolon,
         to_utf8         : to_utf8,
         print_name      : function(name) { print(make_name(name)) },
-        print_string    : function(str, quote, escape_directive) {
-            var encoded = encode_string(str, quote);
-            if (escape_directive === true && encoded.indexOf("\\") === -1) {
-                // Insert semicolons to break directive prologue
-                if (!EXPECT_DIRECTIVE.test(OUTPUT)) {
-                    force_semicolon();
-                }
-                force_semicolon();
-            }
-            print(encoded);
-        },
+        print_string    : function(str, quote) { print(encode_string(str, quote)) },
         next_indent     : next_indent,
         with_indent     : with_indent,
         with_block      : with_block,
@@ -633,17 +621,10 @@ function OutputStream(options) {
         nodetype.DEFMETHOD("_codegen", generator);
     }
 
-    var in_directive = false;
-    var active_scope = null;
-    var use_asm = null;
+    var use_asm = false;
 
     AST_Node.DEFMETHOD("print", function(stream, force_parens) {
         var self = this, generator = self._codegen;
-        if (self instanceof AST_Scope) {
-            active_scope = self;
-        } else if (!use_asm && self instanceof AST_Directive && self.value == "use asm") {
-            use_asm = active_scope;
-        }
         function doit() {
             stream.prepend_comments(self);
             self.add_source_map(stream);
@@ -657,9 +638,6 @@ function OutputStream(options) {
             doit();
         }
         stream.pop_node();
-        if (self === use_asm) {
-            use_asm = null;
-        }
     });
     AST_Node.DEFMETHOD("_print", AST_Node.prototype.print);
 
@@ -828,7 +806,18 @@ function OutputStream(options) {
     /* -----[ PRINTERS ]----- */
 
     DEFPRINT(AST_Directive, function(self, output) {
-        output.print_string(self.value, self.quote);
+        var quote = self.quote;
+        var value = self.value;
+        switch (output.option("quote_style")) {
+          case 0:
+          case 2:
+            if (value.indexOf('"') == -1) quote = '"';
+            break;
+          case 1:
+            if (value.indexOf("'") == -1) quote = "'";
+            break;
+        }
+        output.print(quote + value + quote);
         output.semicolon();
     });
     DEFPRINT(AST_Debugger, function(self, output) {
@@ -840,30 +829,27 @@ function OutputStream(options) {
 
     function display_body(body, is_toplevel, output, allow_directives) {
         var last = body.length - 1;
-        in_directive = allow_directives;
+        var in_directive = allow_directives;
+        var was_asm = use_asm;
         body.forEach(function(stmt, i) {
-            if (in_directive === true && !(stmt instanceof AST_Directive ||
-                stmt instanceof AST_EmptyStatement ||
-                (stmt instanceof AST_SimpleStatement && stmt.body instanceof AST_String)
-            )) {
-                in_directive = false;
-            }
-            if (!(stmt instanceof AST_EmptyStatement)) {
-                output.indent();
-                stmt.print(output);
-                if (!(i == last && is_toplevel)) {
-                    output.newline();
-                    if (is_toplevel) output.newline();
+            if (in_directive) {
+                if (stmt instanceof AST_Directive) {
+                    if (stmt.value == "use asm") use_asm = true;
+                } else if (!(stmt instanceof AST_EmptyStatement)) {
+                    if (stmt instanceof AST_SimpleStatement && stmt.body instanceof AST_String) {
+                        output.force_semicolon();
+                    }
+                    in_directive = false;
                 }
             }
-            if (in_directive === true &&
-                stmt instanceof AST_SimpleStatement &&
-                stmt.body instanceof AST_String
-            ) {
-                in_directive = false;
-            }
+            if (stmt instanceof AST_EmptyStatement) return;
+            output.indent();
+            stmt.print(output);
+            if (i == last && is_toplevel) return;
+            output.newline();
+            if (is_toplevel) output.newline();
         });
-        in_directive = false;
+        use_asm = was_asm;
     }
 
     AST_StatementWithBody.DEFMETHOD("_do_print_body", function(output) {
@@ -1348,7 +1334,7 @@ function OutputStream(options) {
         output.print(self.getValue());
     });
     DEFPRINT(AST_String, function(self, output) {
-        output.print_string(self.getValue(), self.quote, in_directive);
+        output.print_string(self.getValue(), self.quote);
     });
     DEFPRINT(AST_Number, function(self, output) {
         if (use_asm && self.start && self.start.raw != null) {
diff --git a/lib/parse.js b/lib/parse.js
index c9ee01fd..740ef5f2 100644
--- a/lib/parse.js
+++ b/lib/parse.js
@@ -790,9 +790,10 @@ function parse($TEXT, options) {
             var dir = S.in_directives;
             var body = expression(true);
             if (dir) {
-                var token = body.start;
-                if (body instanceof AST_String && token.raw.indexOf("\\") == -1) {
-                    S.input.add_directive(token.value);
+                if (body instanceof AST_String) {
+                    var value = body.start.raw.slice(1, -1);
+                    S.input.add_directive(value);
+                    body.value = value;
                 } else {
                     S.in_directives = dir = false;
                 }
diff --git a/test/compress/directives.js b/test/compress/directives.js
index 590d1623..69ecfdc4 100644
--- a/test/compress/directives.js
+++ b/test/compress/directives.js
@@ -93,3 +93,41 @@ issue_3166: {
         }
     }
 }
+
+valid_after_invalid_1: {
+    input: {
+        console.log(typeof function() {
+            "use\x20strict";
+            "use strict";
+            return this;
+        }());
+    }
+    expect: {
+        console.log(typeof function() {
+            "use\x20strict";
+            "use strict";
+            return this;
+        }());
+    }
+    expect_stdout: "undefined"
+}
+
+valid_after_invalid_2: {
+    options = {
+        directives: true,
+    }
+    input: {
+        console.log(typeof function() {
+            "use\x20strict";
+            "use strict";
+            return this;
+        }());
+    }
+    expect: {
+        console.log(typeof function() {
+            "use strict";
+            return this;
+        }());
+    }
+    expect_stdout: "undefined"
+}
diff --git a/test/mocha/directives.js b/test/mocha/directives.js
index e872747a..524fbc48 100644
--- a/test/mocha/directives.js
+++ b/test/mocha/directives.js
@@ -69,13 +69,13 @@ describe("Directives", function() {
             ],
             [
                 '"use \\\nstrict";"use strict";',
-                [],
-                [ "use strict", "use\nstrict", "use \nstrict", "use asm" ]
+                [ "use strict" ],
+                [ "use\nstrict", "use \nstrict", "use asm" ]
             ],
             [
                 '"\\76";',
-                [],
-                [ ">", "\\76" ]
+                [ "\\76" ],
+                [ ">" ]
             ],
             [
                 // no ; or newline
@@ -106,13 +106,13 @@ describe("Directives", function() {
             ],
             [
                 'function foo() {"use \\\nstrict";"use strict";',
-                [],
-                [ "use strict", "use\nstrict", "use \nstrict", "use asm" ]
+                [ "use strict" ],
+                [ "use\nstrict", "use \nstrict", "use asm" ]
             ],
             [
                 'var foo = function() {"\\76";',
-                [],
-                [ ">", "\\76" ]
+                [ "\\76" ],
+                [ ">" ]
             ],
             [
                 'var foo = function() {"use strict"', // no ; or newline
@@ -156,21 +156,24 @@ describe("Directives", function() {
             });
         });
     });
-    it("Should test EXPECT_DIRECTIVE RegExp", function() {
+    it("Should print semicolon to separate strings from directives", function() {
         [
-            [ "", true ],
-            [ "'test';", true ],
-            [ "'test';;", true ],
-            [ "'tests';\n", true ],
-            [ "'tests'", false ],
-            [ "'tests';   \n\t", true ],
-            [ "'tests';\n\n", true ],
-            [ "\n\n\"use strict\";\n\n", true ],
+            [ "", ';"";' ],
+            [ '"test";', '"test";;"";' ],
+            [ '"test";;', '"test";;"";' ],
+            [ '"tests";\n', '"tests";;"";' ],
+            [ '"tests"', '"tests";;"";' ],
+            [ '"tests";   \n\t', '"tests";;"";' ],
+            [ '"tests";\n\n', '"tests";;"";' ],
+            [ '\n\n"use strict";\n\n', '"use strict";;"";' ],
         ].forEach(function(test) {
+            var ast = UglifyJS.parse(test[0]);
+            ast.body.push(new UglifyJS.AST_SimpleStatement({
+                body: new UglifyJS.AST_String({ value: "" })
+            }));
             var out = UglifyJS.OutputStream();
-            out.print(test[0]);
-            out.print_string("", null, true);
-            assert.strictEqual(out.get() === test[0] + ';""', test[1], test[0]);
+            ast.print(out);
+            assert.strictEqual(out.get(), test[1], test[0]);
         });
     });
     it("Should only print 2 semicolons spread over 2 lines in beautify mode", function() {
@@ -178,8 +181,8 @@ describe("Directives", function() {
             '"use strict";',
             "'use strict';",
             '"use strict";',
-            '"use strict";;',
-            "'use strict';",
+            '"use strict";',
+            ";'use strict';",
             "console.log('use strict');"
         ].join(""), {
             compress: false,
@@ -201,19 +204,23 @@ describe("Directives", function() {
     it("Should not add double semicolons in non-scoped block statements to avoid strings becoming directives", function() {
         [
             [
-                '{"use\x20strict"}',
+                '"use strict";"use\\x20strict";',
+                '"use strict";"use\\x20strict";'
+            ],
+            [
+                '{"use\\x20strict"}',
                 '{"use strict"}'
             ],
             [
-                'function foo(){"use\x20strict";}', // Valid place for directives
-                'function foo(){"use strict"}'
+                'function foo(){"use\\x20strict";}', // Valid place for directives
+                'function foo(){"use\\x20strict"}'
             ],
             [
-                'try{"use\x20strict"}catch(e){}finally{"use\x20strict"}',
+                'try{"use\\x20strict"}catch(e){}finally{"use\\x20strict"}',
                 'try{"use strict"}catch(e){}finally{"use strict"}'
             ],
             [
-                'if(1){"use\x20strict"} else {"use strict"}',
+                'if(1){"use\\x20strict"} else {"use strict"}',
                 'if(1){"use strict"}else{"use strict"}'
             ]
         ].forEach(function(test) {
@@ -225,16 +232,6 @@ describe("Directives", function() {
             assert.strictEqual(result.code, test[1], test[0]);
         });
     });
-    it("Should add double semicolon when relying on automatic semicolon insertion", function() {
-        var result = UglifyJS.minify('"use strict";"use\\x20strict";', {
-            compress: false,
-            output: {
-                semicolons: false
-            }
-        });
-        if (result.error) throw result.error;
-        assert.strictEqual(result.code, '"use strict";;"use strict"\n');
-    });
     it("Should check quote style of directives", function() {
         [
             // 0. Prefer double quotes, unless string contains more double quotes than single quotes
@@ -249,9 +246,9 @@ describe("Directives", function() {
                 '"use strict";'
             ],
             [
-                '"\\\'use strict\\\'";', // Not a directive as it contains quotes
+                '"\\\'use strict\\\'";',
                 0,
-                ';"\'use strict\'";',
+                '"\\\'use strict\\\'";',
             ],
             [
                 "'\"use strict\"';",
@@ -273,7 +270,7 @@ describe("Directives", function() {
                 '"\'use strict\'";',
                 1,
                 // Intentionally causes directive breakage at cost of less logic, usage should be rare anyway
-                "'\\'use strict\\'';",
+                '"\'use strict\'";',
             ],
             [
                 "'\\'use strict\\'';", // Not a valid directive
@@ -305,7 +302,7 @@ describe("Directives", function() {
                 "'\"use strict\"';",
                 2,
                 // Intentionally causes directive breakage at cost of less logic, usage should be rare anyway
-                '"\\\"use strict\\\"";',
+                "'\"use strict\"';",
             ],
             [
                 '"\\"use strict\\"";', // Not a valid directive
@@ -353,8 +350,7 @@ describe("Directives", function() {
             [
                 // Nothing gets optimised in the compressor because "use asm" is the first statement
                 '"use asm";"use\\x20strict";1+1;',
-                // Yet, the parser noticed that "use strict" wasn't a directive
-                '"use asm";;"use strict";1+1;',
+                '"use asm";"use\\x20strict";1+1;'
             ],
             [
                 'function f(){ "use strict" }',
diff --git a/test/mocha/string-literal.js b/test/mocha/string-literal.js
index aca915b7..7b62c90f 100644
--- a/test/mocha/string-literal.js
+++ b/test/mocha/string-literal.js
@@ -59,13 +59,13 @@ describe("String literals", function() {
 
     it("Should not throw error outside strict mode if string contains escaped octalIntegerLiteral", function() {
         var tests = [
-            ['"\\76";', ';">";'],
-            ['"\\0"', '"\\0";'],
-            ['"\\08"', '"\\x008";'],
-            ['"\\008"', '"\\x008";'],
-            ['"\\0008"', '"\\x008";'],
-            ['"use strict" === "use strict";\n"\\76";', '"use strict"==="use strict";">";'],
-            ['"use\\\n strict";\n"\\07";', ';"use strict";"\07";']
+            [ ';"\\76";', ';">";' ],
+            [ ';"\\0";', ';"\\0";' ],
+            [ ';"\\08"', ';"\\x008";' ],
+            [ ';"\\008"', ';"\\x008";' ],
+            [ ';"\\0008"', ';"\\x008";' ],
+            [ ';"use\\\n strict";\n"\\07";', ';"use strict";"\07";' ],
+            [ '"use strict" === "use strict";\n"\\76";', '"use strict"==="use strict";">";' ],
         ];
 
         for (var test in tests) {
@@ -75,8 +75,8 @@ describe("String literals", function() {
     });
 
     it("Should not throw error when digit is 8 or 9", function() {
-        assert.equal(UglifyJS.parse('"use strict";"\\08"').print_to_string(), '"use strict";"\\x008";');
-        assert.equal(UglifyJS.parse('"use strict";"\\09"').print_to_string(), '"use strict";"\\x009";');
+        assert.equal(UglifyJS.parse('"use strict";;"\\08"').print_to_string(), '"use strict";;"\\x008";');
+        assert.equal(UglifyJS.parse('"use strict";;"\\09"').print_to_string(), '"use strict";;"\\x009";');
     });
 
     it("Should not unescape unpaired surrogates", function() {
@@ -93,7 +93,7 @@ describe("String literals", function() {
         for (; i <= 0xFFFF; i++) {
             code.push("\\u" + i.toString(16));
         }
-        code = '"' + code.join() + '"';
+        code = ';"' + code.join() + '"';
         var normal = UglifyJS.minify(code, {
             compress: false,
             mangle: false,