Skip to content

Commit

Permalink
fix comments output & improve /*@__PURE__*/
Browse files Browse the repository at this point in the history 
- fix whitespace around comments
- fix comment parsing around parentheses
- consider parentheses when parsing `/*@__PURE__*/`
- remove all `/*@__PURE__*/` on output

fixes #2638
  • Loading branch information
@alexlamsl
alexlamsl committed Dec 24, 2017
1 parent 202f90e commit efffb81
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 48 deletions.
21 changes: 2 additions & 19 deletions lib/compress.js
View file
Expand Up @@ -2292,29 +2292,13 @@ merge(Compressor.prototype, {
});
});

AST_Call.DEFMETHOD("has_pure_annotation", function(compressor) {
if (!compressor.option("side_effects")) return false;
if (this.pure !== undefined) return this.pure;
var pure = false;
var comments, pure_comment;
if (this.start
&& (comments = this.start.comments_before)
&& comments.length
&& (pure_comment = find_if(function (comment) {
return /[@#]__PURE__/.test(comment.value);
}, comments))) {
pure = pure_comment;
}
return this.pure = pure;
});

var global_pure_fns = makePredicate("Boolean decodeURI decodeURIComponent Date encodeURI encodeURIComponent Error escape EvalError isFinite isNaN Number Object parseFloat parseInt RangeError ReferenceError String SyntaxError TypeError unescape URIError");
AST_Call.DEFMETHOD("is_expr_pure", function(compressor) {
if (compressor.option("unsafe")) {
var expr = this.expression;
if (is_undeclared_ref(expr) && global_pure_fns(expr.name)) return true;
}
return this.has_pure_annotation(compressor) || !compressor.pure_funcs(this);
return this.pure || !compressor.pure_funcs(this);
});

// determine if expression has side effects
Expand Down Expand Up @@ -3164,7 +3148,6 @@ merge(Compressor.prototype, {
}
if (this.pure) {
compressor.warn("Dropping __PURE__ call [{file}:{line},{col}]", this.start);
this.pure.value = this.pure.value.replace(/[@#]__PURE__/g, ' ');
}
var args = trim(this.args, compressor, first_in_statement);
return args && make_sequence(this, args);
Expand Down Expand Up @@ -3961,7 +3944,7 @@ merge(Compressor.prototype, {
&& (def = exp.definition()).references.length == 1
&& !recursive_ref(compressor, def)
&& fn.is_constant_expression(exp.scope))
&& !self.has_pure_annotation(compressor)
&& !self.pure
&& !fn.contains_this()
&& (scope = can_flatten_args(fn))
&& (value = flatten_body(stat))) {
Expand Down
34 changes: 28 additions & 6 deletions lib/output.js
View file
Expand Up @@ -201,6 +201,8 @@ function OutputStream(options) {
var might_need_semicolon = false;
var might_add_newline = 0;
var need_newline_indented = false;
var need_space = false;
var newline_insert = -1;
var last = "";
var mapping_token, mapping_name, mappings = options.source_map && [];

Expand Down Expand Up @@ -266,6 +268,13 @@ function OutputStream(options) {
indent();
}
}
if (need_space && ch) {
need_space = false;
if (!/[\s;})]/.test(ch)) {
space();
}
}
newline_insert = -1;
var prev = last.charAt(last.length - 1);
if (might_need_semicolon) {
might_need_semicolon = false;
Expand Down Expand Up @@ -364,7 +373,13 @@ function OutputStream(options) {
} : function(col, cont) { return cont() };

var newline = options.beautify ? function() {
print("\n");
if (newline_insert < 0) return print("\n");
if (OUTPUT[newline_insert] != "\n") {
OUTPUT = OUTPUT.slice(0, newline_insert) + "\n" + OUTPUT.slice(newline_insert);
current_pos++;
current_line++;
}
newline_insert++;
} : options.max_line_len ? function() {
ensure_line_len();
might_add_newline = OUTPUT.length;
Expand Down Expand Up @@ -495,11 +510,11 @@ function OutputStream(options) {
}
}
if (/comment[134]/.test(c.type)) {
print("//" + c.value + "\n");
print("//" + c.value.replace(/[@#]__PURE__/g, ' ') + "\n");
indent();
last_nlb = true;
} else if (c.type == "comment2") {
print("/*" + c.value + "*/");
print("/*" + c.value.replace(/[@#]__PURE__/g, ' ') + "*/");
last_nlb = false;
}
});
Expand All @@ -521,21 +536,28 @@ function OutputStream(options) {
var comments = token[tail ? "comments_before" : "comments_after"];
if (comments && comments._dumped !== self) {
comments._dumped = self;
var insert = OUTPUT.length;
comments.filter(comment_filter, node).forEach(function(c, i) {
if (need_newline_indented || c.nlb) {
need_space = false;
if (need_newline_indented) {
print("\n");
indent();
need_newline_indented = false;
} else if (c.nlb && (i > 0 || !/(^|\n) *$/.test(OUTPUT))) {
print("\n");
indent();
} else if (i > 0 || !tail) {
space();
}
if (/comment[134]/.test(c.type)) {
print("//" + c.value);
print("//" + c.value.replace(/[@#]__PURE__/g, ' '));
need_newline_indented = true;
} else if (c.type == "comment2") {
print("/*" + c.value + "*/");
print("/*" + c.value.replace(/[@#]__PURE__/g, ' ') + "*/");
need_space = true;
}
});
if (OUTPUT.length > insert) newline_insert = insert;
}
}

Expand Down
33 changes: 29 additions & 4 deletions lib/parse.js
View file
Expand Up @@ -1276,8 +1276,17 @@ 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;
var len = start.comments_before.length;
[].unshift.apply(ex.start.comments_before, start.comments_before);
start.comments_before = ex.start.comments_before;
start.comments_before_length = len;
if (len == 0 && start.comments_before.length > 0) {
var comment = start.comments_before[0];
if (!comment.nlb) {
comment.nlb = start.nlb;
start.nlb = false;
}
}
start.comments_after = ex.start.comments_after;
ex.start = start;
expect(")");
Expand All @@ -1286,6 +1295,7 @@ function parse($TEXT, options) {
[].push.apply(ex.end.comments_after, end.comments_after);
end.comments_after = ex.end.comments_after;
ex.end = end;
if (ex instanceof AST_Call) mark_pure(ex);
return subscripts(ex, allow_calls);
case "[":
return subscripts(array_(), allow_calls);
Expand Down Expand Up @@ -1433,6 +1443,19 @@ function parse($TEXT, options) {
return sym;
};

function mark_pure(call) {
var start = call.start;
var comments = start.comments_before;
var i = HOP(start, "comments_before_length") ? start.comments_before_length : comments.length;
while (--i >= 0) {
var comment = comments[i];
if (/[@#]__PURE__/.test(comment.value)) {
call.pure = comment;
break;
}
}
}

var subscripts = function(expr, allow_calls) {
var start = expr.start;
if (is("punc", ".")) {
Expand All @@ -1457,12 +1480,14 @@ function parse($TEXT, options) {
}
if (allow_calls && is("punc", "(")) {
next();
return subscripts(new AST_Call({
var call = new AST_Call({
start : start,
expression : expr,
args : expr_list(")"),
end : prev()
}), true);
});
mark_pure(call);
return subscripts(call, true);
}
return expr;
};
Expand Down
73 changes: 56 additions & 17 deletions test/compress/pure_funcs.js
View file
Expand Up @@ -298,19 +298,27 @@ issue_2629_1: {
options = {
side_effects: true,
}
beautify = {
comments: "all",
}
input: {
/*@__PURE__*/ a();
/*@__PURE__*/ (b());
(/*@__PURE__*/ c)();
(/*@__PURE__*/ d());
}
expect: {}
expect_exact: [
"/* */c();",
]
}

issue_2629_2: {
options = {
side_effects: true,
}
beautify = {
comments: "all",
}
input: {
/*@__PURE__*/ a(1)(2)(3);
/*@__PURE__*/ (b(1))(2)(3);
Expand All @@ -321,30 +329,44 @@ issue_2629_2: {
(/*@__PURE__*/ g(1)(2))(3);
(/*@__PURE__*/ h(1)(2)(3));
}
expect: {}
expect_exact: [
"/* */e(1)(2)(3);",
"/* */f(1)(2)(3);",
"/* */g(1)(2)(3);",
]
}

issue_2629_3: {
options = {
side_effects: true,
}
beautify = {
comments: "all",
}
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: {}
/*@__PURE__*/ (b.x)(1).y(2).z(3);
/*@__PURE__*/ (c.x(1)).y(2).z(3);
/*@__PURE__*/ (d.x(1).y)(2).z(3);
/*@__PURE__*/ (e.x(1).y(2)).z(3);
/*@__PURE__*/ (f.x(1).y(2).z)(3);
/*@__PURE__*/ (g.x(1).y(2).z(3));
(/*@__PURE__*/ h).x(1).y(2).z(3);
(/*@__PURE__*/ i.x)(1).y(2).z(3);
(/*@__PURE__*/ j.x(1)).y(2).z(3);
(/*@__PURE__*/ k.x(1).y)(2).z(3);
(/*@__PURE__*/ l.x(1).y(2)).z(3);
(/*@__PURE__*/ m.x(1).y(2).z)(3);
(/*@__PURE__*/ n.x(1).y(2).z(3));
}
expect_exact: [
"/* */h.x(1).y(2).z(3);",
"/* */i.x(1).y(2).z(3);",
"/* */j.x(1).y(2).z(3);",
"/* */k.x(1).y(2).z(3);",
"/* */l.x(1).y(2).z(3);",
"/* */m.x(1).y(2).z(3);",
]
}

issue_2629_4: {
Expand Down Expand Up @@ -375,3 +397,20 @@ issue_2629_5: {
w(), y();
}
}

issue_2638: {
options = {
side_effects: true,
}
beautify = {
comments: "all",
}
input: {
/*@__PURE__*/(g() || h())(x(), y());
(/*@__PURE__*/ (a() || b()))(c(), d());
}
expect_exact: [
"/* */x(),y();",
"/* */(a()||b())(c(),d());",
]
}
4 changes: 2 additions & 2 deletions test/mocha/minify.js
View file
Expand Up @@ -247,15 +247,15 @@ describe("minify", function() {
var code = result.code;
assert.strictEqual(code, "// comment1 comment2\nbar();");
});
it("should not drop #__PURE__ hint if function is retained", function() {
it("should drop #__PURE__ hint if function is retained", function() {
var result = Uglify.minify("var a = /*#__PURE__*/(function(){ foo(); })();", {
output: {
comments: "all",
beautify: false,
}
});
var code = result.code;
assert.strictEqual(code, "var a=/*#__PURE__*/function(){foo()}();");
assert.strictEqual(code, "var a=/* */function(){foo()}();");
})
});

Expand Down

0 comments on commit efffb81

Please sign in to comment.