[Pkg-javascript-commits] [uglifyjs] 355/491: make comments output more robust (#2633)
Jonas Smedegaard
dr at jones.dk
Wed Feb 14 19:51:52 UTC 2018
This is an automated email from the git hooks/post-receive script.
js pushed a commit to annotated tag debian/3.3.10-1
in repository uglifyjs.
commit edb4e3bd52e1623425927a7d63963ba3b87a3ec2
Author: Alex Lam S.L <alexlamsl at gmail.com>
Date: Fri Dec 22 04:59:54 2017 +0800
make comments output more robust (#2633)
- improve handling of comments right after `return`
- retain comments after `OutputStream`
- preserve trailing comments
- fix handling of new line before comments
- handle comments around parentheses
fixes #88
fixes #112
fixes #218
fixes #372
fixes #2629
---
lib/ast.js | 2 +-
lib/output.js | 212 +++++++++++++++++++++++++++----------------
lib/parse.js | 15 +--
lib/utils.js | 2 +-
test/compress/pure_funcs.js | 82 +++++++++++++++++
test/mocha/comment-filter.js | 6 +-
test/mocha/comment.js | 172 +++++++++++++++++++++++++++++++++++
7 files changed, 401 insertions(+), 90 deletions(-)
diff --git a/lib/ast.js b/lib/ast.js
index 997486c..6591867 100644
--- a/lib/ast.js
+++ b/lib/ast.js
@@ -87,7 +87,7 @@ function DEFNODE(type, props, methods, base) {
return ctor;
};
-var AST_Token = DEFNODE("Token", "type value line col pos endline endcol endpos nlb comments_before file raw", {
+var AST_Token = DEFNODE("Token", "type value line col pos endline endcol endpos nlb comments_before comments_after file raw", {
}, null);
var AST_Node = DEFNODE("Node", "start end", {
diff --git a/lib/output.js b/lib/output.js
index a4c41f1..b0cecf0 100644
--- a/lib/output.js
+++ b/lib/output.js
@@ -52,6 +52,7 @@ function is_some_comments(comment) {
function OutputStream(options) {
+ var readonly = !options;
options = defaults(options, {
ascii_only : false,
beautify : false,
@@ -199,6 +200,7 @@ function OutputStream(options) {
var might_need_space = false;
var might_need_semicolon = false;
var might_add_newline = 0;
+ var need_newline_indented = false;
var last = "";
var mapping_token, mapping_name, mappings = options.source_map && [];
@@ -257,6 +259,13 @@ function OutputStream(options) {
function print(str) {
str = String(str);
var ch = str.charAt(0);
+ if (need_newline_indented && ch) {
+ need_newline_indented = false;
+ if (ch != "\n") {
+ print("\n");
+ indent();
+ }
+ }
var prev = last.charAt(last.length - 1);
if (might_need_semicolon) {
might_need_semicolon = false;
@@ -427,6 +436,109 @@ function OutputStream(options) {
return OUTPUT;
};
+ function prepend_comments(node) {
+ var self = this;
+ var start = node.start;
+ if (!(start.comments_before && start.comments_before._dumped === self)) {
+ var comments = start.comments_before;
+ if (!comments) {
+ comments = start.comments_before = [];
+ }
+ comments._dumped = self;
+
+ if (node instanceof AST_Exit && node.value) {
+ var tw = new TreeWalker(function(node) {
+ var parent = tw.parent();
+ if (parent instanceof AST_Exit
+ || parent instanceof AST_Binary && parent.left === node
+ || parent.TYPE == "Call" && parent.expression === node
+ || parent instanceof AST_Conditional && parent.condition === node
+ || parent instanceof AST_Dot && parent.expression === node
+ || parent instanceof AST_Sequence && parent.expressions[0] === node
+ || parent instanceof AST_Sub && parent.expression === node
+ || parent instanceof AST_UnaryPostfix) {
+ var text = node.start.comments_before;
+ if (text && text._dumped !== self) {
+ text._dumped = self;
+ comments = comments.concat(text);
+ }
+ } else {
+ return true;
+ }
+ });
+ tw.push(node);
+ node.value.walk(tw);
+ }
+
+ if (current_pos == 0) {
+ if (comments.length > 0 && options.shebang && comments[0].type == "comment5") {
+ print("#!" + comments.shift().value + "\n");
+ indent();
+ }
+ var preamble = options.preamble;
+ if (preamble) {
+ print(preamble.replace(/\r\n?|[\n\u2028\u2029]|\s*$/g, "\n"));
+ }
+ }
+
+ comments = comments.filter(comment_filter, node);
+ if (comments.length == 0) return;
+ var last_nlb = /(^|\n) *$/.test(OUTPUT);
+ comments.forEach(function(c, i) {
+ if (!last_nlb) {
+ if (c.nlb) {
+ print("\n");
+ indent();
+ last_nlb = true;
+ } else if (i > 0) {
+ space();
+ }
+ }
+ if (/comment[134]/.test(c.type)) {
+ print("//" + c.value + "\n");
+ indent();
+ last_nlb = true;
+ } else if (c.type == "comment2") {
+ print("/*" + c.value + "*/");
+ last_nlb = false;
+ }
+ });
+ if (!last_nlb) {
+ if (start.nlb) {
+ print("\n");
+ indent();
+ } else {
+ space();
+ }
+ }
+ }
+ }
+
+ function append_comments(node, tail) {
+ var self = this;
+ var token = node.end;
+ if (!token) return;
+ var comments = token[tail ? "comments_before" : "comments_after"];
+ if (comments && comments._dumped !== self) {
+ comments._dumped = self;
+ comments.filter(comment_filter, node).forEach(function(c, i) {
+ if (need_newline_indented || c.nlb) {
+ print("\n");
+ indent();
+ need_newline_indented = false;
+ } else if (i > 0 || !tail) {
+ space();
+ }
+ if (/comment[134]/.test(c.type)) {
+ print("//" + c.value);
+ need_newline_indented = true;
+ } else if (c.type == "comment2") {
+ print("/*" + c.value + "*/");
+ }
+ });
+ }
+ }
+
var stack = [];
return {
get : get,
@@ -464,7 +576,8 @@ function OutputStream(options) {
with_square : with_square,
add_mapping : add_mapping,
option : function(opt) { return options[opt] },
- comment_filter : comment_filter,
+ prepend_comments: readonly ? noop : prepend_comments,
+ append_comments : readonly ? noop : append_comments,
line : function() { return current_line },
col : function() { return current_col },
pos : function() { return current_pos },
@@ -500,9 +613,10 @@ function OutputStream(options) {
use_asm = active_scope;
}
function doit() {
- self.add_comments(stream);
+ stream.prepend_comments(self);
self.add_source_map(stream);
generator(self, stream);
+ stream.append_comments(self);
}
stream.push_node(self);
if (force_parens || self.needs_parens(stream)) {
@@ -519,77 +633,10 @@ function OutputStream(options) {
AST_Node.DEFMETHOD("print_to_string", function(options){
var s = OutputStream(options);
- if (!options) s._readonly = true;
this.print(s);
return s.get();
});
- /* -----[ comments ]----- */
-
- AST_Node.DEFMETHOD("add_comments", function(output){
- if (output._readonly) return;
- var self = this;
- var start = self.start;
- if (start && !start._comments_dumped) {
- start._comments_dumped = true;
- var comments = start.comments_before || [];
-
- // XXX: ugly fix for https://github.com/mishoo/UglifyJS2/issues/112
- // and https://github.com/mishoo/UglifyJS2/issues/372
- if (self instanceof AST_Exit && self.value) {
- self.value.walk(new TreeWalker(function(node){
- if (node.start && node.start.comments_before) {
- comments = comments.concat(node.start.comments_before);
- node.start.comments_before = [];
- }
- if (node instanceof AST_Function ||
- node instanceof AST_Array ||
- node instanceof AST_Object)
- {
- return true; // don't go inside.
- }
- }));
- }
-
- if (output.pos() == 0) {
- if (comments.length > 0 && output.option("shebang") && comments[0].type == "comment5") {
- output.print("#!" + comments.shift().value + "\n");
- output.indent();
- }
- var preamble = output.option("preamble");
- if (preamble) {
- output.print(preamble.replace(/\r\n?|[\n\u2028\u2029]|\s*$/g, "\n"));
- }
- }
-
- comments = comments.filter(output.comment_filter, self);
-
- // Keep single line comments after nlb, after nlb
- if (!output.option("beautify") && comments.length > 0 &&
- /comment[134]/.test(comments[0].type) &&
- output.col() !== 0 && comments[0].nlb)
- {
- output.print("\n");
- }
-
- comments.forEach(function(c){
- if (/comment[134]/.test(c.type)) {
- output.print("//" + c.value + "\n");
- output.indent();
- }
- else if (c.type == "comment2") {
- output.print("/*" + c.value + "*/");
- if (start.nlb) {
- output.print("\n");
- output.indent();
- } else {
- output.space();
- }
- }
- });
- }
- });
-
/* -----[ PARENTHESES ]----- */
function PARENS(nodetype, func) {
@@ -811,14 +858,21 @@ function OutputStream(options) {
self.body.print(output);
output.semicolon();
});
- function print_bracketed(body, output, allow_directives) {
- if (body.length > 0) output.with_block(function(){
- display_body(body, false, output, allow_directives);
- });
- else output.print("{}");
+ function print_bracketed(self, output, allow_directives) {
+ if (self.body.length > 0) {
+ output.with_block(function() {
+ display_body(self.body, false, output, allow_directives);
+ });
+ } else {
+ output.print("{");
+ output.with_indent(output.next_indent(), function() {
+ output.append_comments(self, true);
+ });
+ output.print("}");
+ }
};
DEFPRINT(AST_BlockStatement, function(self, output){
- print_bracketed(self.body, output);
+ print_bracketed(self, output);
});
DEFPRINT(AST_EmptyStatement, function(self, output){
output.semicolon();
@@ -913,7 +967,7 @@ function OutputStream(options) {
});
});
output.space();
- print_bracketed(self.body, output, true);
+ print_bracketed(self, output, true);
});
DEFPRINT(AST_Lambda, function(self, output){
self._do_print(output);
@@ -1044,7 +1098,7 @@ function OutputStream(options) {
DEFPRINT(AST_Try, function(self, output){
output.print("try");
output.space();
- print_bracketed(self.body, output);
+ print_bracketed(self, output);
if (self.bcatch) {
output.space();
self.bcatch.print(output);
@@ -1061,12 +1115,12 @@ function OutputStream(options) {
self.argname.print(output);
});
output.space();
- print_bracketed(self.body, output);
+ print_bracketed(self, output);
});
DEFPRINT(AST_Finally, function(self, output){
output.print("finally");
output.space();
- print_bracketed(self.body, output);
+ print_bracketed(self, output);
});
/* -----[ var/const ]----- */
diff --git a/lib/parse.js b/lib/parse.js
index f0098c7..41aa988 100644
--- a/lib/parse.js
+++ b/lib/parse.js
@@ -317,11 +317,7 @@ function tokenizer($TEXT, filename, html5_comments, shebang) {
}
if (!is_comment) {
ret.comments_before = S.comments_before;
- S.comments_before = [];
- // make note of any newlines in the comments that came before
- for (var i = 0, len = ret.comments_before.length; i < len; i++) {
- ret.nlb = ret.nlb || ret.comments_before[i].nlb;
- }
+ ret.comments_after = S.comments_before = [];
}
S.newline_before = false;
return new AST_Token(ret);
@@ -1280,9 +1276,16 @@ function parse($TEXT, options) {
case "(":
next();
var ex = expression(true);
+ [].push.apply(start.comments_before, ex.start.comments_before);
+ ex.start.comments_before = start.comments_before;
+ start.comments_after = ex.start.comments_after;
ex.start = start;
- ex.end = S.token;
expect(")");
+ var end = prev();
+ end.comments_before = ex.end.comments_before;
+ [].push.apply(ex.end.comments_after, end.comments_after);
+ end.comments_after = ex.end.comments_after;
+ ex.end = end;
return subscripts(ex, allow_calls);
case "[":
return subscripts(array_(), allow_calls);
diff --git a/lib/utils.js b/lib/utils.js
index 102c478..dab7f56 100644
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -324,7 +324,7 @@ function first_in_statement(stack) {
if (p instanceof AST_Statement && p.body === node)
return true;
if ((p instanceof AST_Sequence && p.expressions[0] === node) ||
- (p instanceof AST_Call && p.expression === node && !(p instanceof AST_New) ) ||
+ (p.TYPE == "Call" && p.expression === node ) ||
(p instanceof AST_Dot && p.expression === node ) ||
(p instanceof AST_Sub && p.expression === node ) ||
(p instanceof AST_Conditional && p.condition === node ) ||
diff --git a/test/compress/pure_funcs.js b/test/compress/pure_funcs.js
index 3cc529a..6f3bbb2 100644
--- a/test/compress/pure_funcs.js
+++ b/test/compress/pure_funcs.js
@@ -293,3 +293,85 @@ unary: {
bar();
}
}
+
+issue_2629_1: {
+ options = {
+ side_effects: true,
+ }
+ input: {
+ /*@__PURE__*/ a();
+ /*@__PURE__*/ (b());
+ (/*@__PURE__*/ c)();
+ (/*@__PURE__*/ d());
+ }
+ expect: {}
+}
+
+issue_2629_2: {
+ options = {
+ side_effects: true,
+ }
+ input: {
+ /*@__PURE__*/ a(1)(2)(3);
+ /*@__PURE__*/ (b(1))(2)(3);
+ /*@__PURE__*/ (c(1)(2))(3);
+ /*@__PURE__*/ (d(1)(2)(3));
+ (/*@__PURE__*/ e)(1)(2)(3);
+ (/*@__PURE__*/ f(1))(2)(3);
+ (/*@__PURE__*/ g(1)(2))(3);
+ (/*@__PURE__*/ h(1)(2)(3));
+ }
+ expect: {}
+}
+
+issue_2629_3: {
+ options = {
+ side_effects: true,
+ }
+ input: {
+ /*@__PURE__*/ a.x(1).y(2).z(3);
+ /*@__PURE__*/ (a.x)(1).y(2).z(3);
+ /*@__PURE__*/ (a.x(1)).y(2).z(3);
+ /*@__PURE__*/ (a.x(1).y)(2).z(3);
+ /*@__PURE__*/ (a.x(1).y(2)).z(3);
+ /*@__PURE__*/ (a.x(1).y(2).z)(3);
+ /*@__PURE__*/ (a.x(1).y(2).z(3));
+ (/*@__PURE__*/ a).x(1).y(2).z(3);
+ (/*@__PURE__*/ a.x)(1).y(2).z(3);
+ (/*@__PURE__*/ a.x(1)).y(2).z(3);
+ (/*@__PURE__*/ a.x(1).y)(2).z(3);
+ (/*@__PURE__*/ a.x(1).y(2)).z(3);
+ (/*@__PURE__*/ a.x(1).y(2).z)(3);
+ (/*@__PURE__*/ a.x(1).y(2).z(3));
+ }
+ expect: {}
+}
+
+issue_2629_4: {
+ options = {
+ side_effects: true,
+ }
+ input: {
+ (/*@__PURE__*/ x(), y());
+ (w(), /*@__PURE__*/ x(), y());
+ }
+ expect: {
+ y();
+ w(), y();
+ }
+}
+
+issue_2629_5: {
+ options = {
+ side_effects: true,
+ }
+ input: {
+ [ /*@__PURE__*/ x() ];
+ [ /*@__PURE__*/ x(), y() ];
+ [ w(), /*@__PURE__*/ x(), y() ];
+ }
+ expect: {
+ y();
+ w(), y();
+ }
+}
diff --git a/test/mocha/comment-filter.js b/test/mocha/comment-filter.js
index 0e4f3df..25233d1 100644
--- a/test/mocha/comment-filter.js
+++ b/test/mocha/comment-filter.js
@@ -14,7 +14,7 @@ describe("comment filters", function() {
it("Should be able to filter commments with the 'some' option", function() {
var ast = UglifyJS.parse("// foo\n/*@preserve*/\n// bar\n/*@license*/\n//@license with the wrong comment type\n/*@cc_on something*/");
- assert.strictEqual(ast.print_to_string({comments: "some"}), "/*@preserve*/\n/*@license*/\n/*@cc_on something*/\n");
+ assert.strictEqual(ast.print_to_string({comments: "some"}), "/*@preserve*/\n/*@license*/\n/*@cc_on something*/");
});
it("Should be able to filter comments by passing a function", function() {
@@ -55,12 +55,12 @@ describe("comment filters", function() {
return true;
};
- assert.strictEqual(ast.print_to_string({comments: f}), "#!Random comment\n//test1\n/*test2*/\n");
+ assert.strictEqual(ast.print_to_string({comments: f}), "#!Random comment\n//test1\n/*test2*/");
});
it("Should never be able to filter comment5 when using 'some' as filter", function() {
var ast = UglifyJS.parse("#!foo\n//foo\n/*@preserve*/\n/* please hide me */");
- assert.strictEqual(ast.print_to_string({comments: "some"}), "#!foo\n/*@preserve*/\n");
+ assert.strictEqual(ast.print_to_string({comments: "some"}), "#!foo\n/*@preserve*/");
});
it("Should have no problem on multiple calls", function() {
diff --git a/test/mocha/comment.js b/test/mocha/comment.js
index 6b5428d..3bad9e0 100644
--- a/test/mocha/comment.js
+++ b/test/mocha/comment.js
@@ -47,4 +47,176 @@ describe("Comment", function() {
}, fail, tests[i]);
}
});
+
+ it("Should handle comment within return correctly", function() {
+ var result = uglify.minify([
+ "function unequal(x, y) {",
+ " return (",
+ " // Either one",
+ " x < y",
+ " ||",
+ " y < x",
+ " );",
+ "}",
+ ].join("\n"), {
+ compress: false,
+ mangle: false,
+ output: {
+ beautify: true,
+ comments: "all",
+ },
+ });
+ if (result.error) throw result.error;
+ assert.strictEqual(result.code, [
+ "function unequal(x, y) {",
+ " // Either one",
+ " return x < y || y < x;",
+ "}",
+ ].join("\n"));
+ });
+
+ it("Should handle comment folded into return correctly", function() {
+ var result = uglify.minify([
+ "function f() {",
+ " /* boo */ x();",
+ " return y();",
+ "}",
+ ].join("\n"), {
+ mangle: false,
+ output: {
+ beautify: true,
+ comments: "all",
+ },
+ });
+ if (result.error) throw result.error;
+ assert.strictEqual(result.code, [
+ "function f() {",
+ " /* boo */",
+ " return x(), y();",
+ "}",
+ ].join("\n"));
+ });
+
+ it("Should not drop comments after first OutputStream", function() {
+ var code = "/* boo */\nx();";
+ var ast = uglify.parse(code);
+ var out1 = uglify.OutputStream({
+ beautify: true,
+ comments: "all",
+ });
+ ast.print(out1);
+ var out2 = uglify.OutputStream({
+ beautify: true,
+ comments: "all",
+ });
+ ast.print(out2);
+ assert.strictEqual(out1.get(), code);
+ assert.strictEqual(out2.get(), out1.get());
+ });
+
+ it("Should retain trailing comments", function() {
+ var code = [
+ "if (foo /* lost comment */ && bar /* lost comment */) {",
+ " // this one is kept",
+ " {/* lost comment */}",
+ " !function() {",
+ " // lost comment",
+ " }();",
+ " function baz() {/* lost comment */}",
+ " // lost comment",
+ "}",
+ "// comments right before EOF are lost as well",
+ ].join("\n");
+ var result = uglify.minify(code, {
+ compress: false,
+ mangle: false,
+ output: {
+ beautify: true,
+ comments: "all",
+ },
+ });
+ if (result.error) throw result.error;
+ assert.strictEqual(result.code, code);
+ });
+
+ it("Should correctly preserve new lines around comments", function() {
+ var tests = [
+ [
+ "// foo",
+ "// bar",
+ "x();",
+ ].join("\n"),
+ [
+ "// foo",
+ "/* bar */",
+ "x();",
+ ].join("\n"),
+ [
+ "// foo",
+ "/* bar */ x();",
+ ].join("\n"),
+ [
+ "/* foo */",
+ "// bar",
+ "x();",
+ ].join("\n"),
+ [
+ "/* foo */ // bar",
+ "x();",
+ ].join("\n"),
+ [
+ "/* foo */",
+ "/* bar */",
+ "x();",
+ ].join("\n"),
+ [
+ "/* foo */",
+ "/* bar */ x();",
+ ].join("\n"),
+ [
+ "/* foo */ /* bar */",
+ "x();",
+ ].join("\n"),
+ "/* foo */ /* bar */ x();",
+ ].forEach(function(code) {
+ var result = uglify.minify(code, {
+ compress: false,
+ mangle: false,
+ output: {
+ beautify: true,
+ comments: "all",
+ },
+ });
+ if (result.error) throw result.error;
+ assert.strictEqual(result.code, code);
+ });
+ });
+
+ it("Should preserve new line before comment without beautify", function() {
+ var code = [
+ "function f(){",
+ "/* foo */bar()}",
+ ].join("\n");
+ var result = uglify.minify(code, {
+ compress: false,
+ mangle: false,
+ output: {
+ comments: "all",
+ },
+ });
+ if (result.error) throw result.error;
+ assert.strictEqual(result.code, code);
+ });
+
+ it("Should preserve comments around IIFE", function() {
+ var result = uglify.minify("/*a*/(/*b*/function(){/*c*/}/*d*/)/*e*/();", {
+ compress: false,
+ mangle: false,
+ output: {
+ comments: "all",
+ },
+ });
+ if (result.error) throw result.error;
+ assert.strictEqual(result.code, "/*a*/ /*b*/(function(){/*c*/}/*d*/ /*e*/)();");
+ });
});
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-javascript/uglifyjs.git
More information about the Pkg-javascript-commits
mailing list