pure annotation within parentheses not recognized

[kzc]

Bug report or feature request?

bug

ES5 or ES6+ input?

any

Uglify version (uglifyjs -V)

3.2.2

JavaScript input

$ bin/uglifyjs -V
uglify-js 3.2.2

$ echo 'a(); /*@__PURE__*/b(); c();' | bin/uglifyjs -c
a(),c();

$ echo 'a(); /*@__PURE__*/( b() ); c();' | bin/uglifyjs -c
a(),c();

$ echo 'a(); ( /*@__PURE__*/b() ); c();' | bin/uglifyjs -c
a(),b(),c();

original report: babel/babel#7044 (comment)

[kzc]

It works within parenthesized sequences:

$ echo 'a(); ( /*@__PURE__*/x(), y() ); c();' | bin/uglifyjs -c
a(),y(),c();

$ echo 'a(); ( w(), /*@__PURE__*/x(), y() ); c();' | bin/uglifyjs -c
a(),w(),y(),c();

Arrays are fine too:

$ echo 'a(); [ /*@__PURE__*/b() ]; c();' | bin/uglifyjs -c
a(),c();

$ echo 'a(); [ /*@__PURE__*/x(), y() ]; c();' | bin/uglifyjs -c
a(),y(),c();

$ echo 'a(); [ w(), /*@__PURE__*/x(), y() ]; c();' | bin/uglifyjs -c
a(),w(),y(),c();

[kzc]

This can be fixed in either the parser or the compressor. I think the compressor is the safer option given the complexity of parsing sequences, arrow expressions and parameters:

--- a/lib/compress.js
+++ b/lib/compress.js
@@ -2290,17 +2290,18 @@ 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;
+        return this.pure = pure_comment(this) || pure_comment(this.expression) || false;
+
+        function pure_comment(node) {
+            var comments;
+            if (node.start
+                && (comments = node.start.comments_before)
+                && comments.length) {
+                return find_if(function (comment) {
+                    return /[@#]__PURE__/.test(comment.value);
+                }, comments);
+            }
+        }
     });
 
     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");

[alexlamsl]

That patch would also make (/*@__PURE__*/b)(); disappear - is this expected behaviour?

[alexlamsl]

pure_comment(this.expression) may have a higher hit rate than pure_comment(this) 🤔

[kzc]

That patch would also make (/*@__PURE__*/b)(); disappear - is this expected behaviour?

It wouldn't for the same reason the pure annotation did not work in the top post. With patch:

$ echo '(/*@__PURE__*/b)();' | bin/uglifyjs -c toplevel
b();

[kzc]

pure_comment(this.expression) may have a higher hit rate than pure_comment(this)

Without the parens they will generally both share the same comment string instances.

The patch above is inefficient because in most non-pure cases this.start.comments_before is the same object instance as this.expression.start.comments_before.

[kzc]

@alexlamsl I'm not crazy about the patch above even if the noted inefficiency was addressed. If you like you can see if you can come up with an alternative parser-side fix, although it will be different for master and harmony as the logic diverges for sequence and arrow expression parsing.

[kzc]

Although the following is not considered a pure annotation with the patch above:

$ echo 'a(); ( /*@__PURE__*/b )(); c();' | bin/uglifyjs -c
a(),b(),c();

this unfortunately is:

$ echo 'a(); ( 0, /*@__PURE__*/b )(); c();' | bin/uglifyjs -c
a(),c();

We need a better solution.

[kzc]

The comments_before logic in parser.js and output.js is not exactly obvious.

[alexlamsl]

I guess it's AST_Node.DEFMETHOD() time for has_pure_annotation() 👻

[kzc]

I'm no longer in favor of the compressor fix. It should be fixed in the parser.

[kzc]

The annotation works in sequences. In the parser a parenthesized expression is similar to a degenerate sequence of 1 element. The point at which the decision is made to return a single element the .start.comments_before info is lost.

In harmony the fix would be somewhere around here:

https://github.com/mishoo/UglifyJS2/blob/85bfa171395da1815b5f91d820b07a50b0d3f249/lib/parse.js#L2051-L2062

In master it would be around here:

https://github.com/mishoo/UglifyJS2/blob/4113609dd4d782f0ceb9ec1c3e9c829e05a93aed/lib/parse.js#L1575-L1579

[alexlamsl]

I'm not sure why the parser is considered wrong in the case of uglify-js - (/* boo */a()) is marking:

• ( as start for AST_Call
• a as start for AST_SymbolRef
• /* boo */ precedes a but not (

which seems entirely reasonable to me.

[alexlamsl]

@kzc PTAL at #2633