Skip to content

Commit

Permalink
fix corner case in directives (#3645)
Browse files Browse the repository at this point in the history
  • Loading branch information
@alexlamsl
alexlamsl committed Dec 25, 2019
1 parent 75aa6ef commit ab050e7
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 103 deletions.
80 changes: 33 additions & 47 deletions lib/output.js
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -378,7 +376,7 @@ function OutputStream(options) {
};

function force_semicolon() {
might_need_semicolon = false;
if (might_need_semicolon) print(";");
print(";");
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -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);

Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
7 changes: 4 additions & 3 deletions lib/parse.js
View file
Expand Up @@ -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;
}
Expand Down
38 changes: 38 additions & 0 deletions test/compress/directives.js
View file
Expand Up @@ -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"
}
82 changes: 39 additions & 43 deletions test/mocha/directives.js
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -156,30 +156,33 @@ 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() {
var result = UglifyJS.minify([
'"use strict";',
"'use strict';",
'"use strict";',
'"use strict";;',
"'use strict';",
'"use strict";',
";'use strict';",
"console.log('use strict');"
].join(""), {
compress: false,
Expand All @@ -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) {
Expand All @@ -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
Expand All @@ -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\"';",
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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" }',
Expand Down

0 comments on commit ab050e7

Please sign in to comment.