Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

make comments output more robust #2633

Merged
merged 5 commits into from
Dec 21, 2017
Merged

make comments output more robust #2633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ec85120
make comments output more robust
alexlamsl Dec 21, 2017
054330d
preserve trailing comments
alexlamsl Dec 21, 2017
dd36b19
handle comments around parentheses
alexlamsl Dec 21, 2017
28b697f
add tests
alexlamsl Dec 21, 2017
fd267ad
add tests
alexlamsl Dec 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/ast.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand Down
212 changes: 133 additions & 79 deletions lib/output.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ function is_some_comments(comment) {

function OutputStream(options) {

var readonly = !options;
options = defaults(options, {
ascii_only : false,
beautify : false,
Expand Down Expand Up @@ -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 && [];

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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)) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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 ]----- */
Expand Down
15 changes: 9 additions & 6 deletions lib/parse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) ||
Expand Down