improve comment formatting logic (#4794)

4 files changed, 44 insertions, 71 deletions

diff --git a/lib/output.js b/lib/output.js
index e6b49804..fef57bce 100644
--- a/lib/output.js
+++ b/lib/output.js
@@ -347,6 +347,7 @@ function OutputStream(options) {
     };
 
     var indent = options.beautify ? function(half) {
+        if (need_newline_indented) print("\n");
         print(repeat_string(" ", half ? indentation - (options.indent_level >> 1) : indentation));
     } : noop;
 
@@ -450,6 +451,27 @@ function OutputStream(options) {
         return /^ *$/.test(OUTPUT.slice(index + 1));
     }
 
+    function pad_comment(token, force) {
+        if (need_newline_indented) return;
+        if (token.nlb && (force || !has_nlb())) {
+            need_newline_indented = true;
+        } else if (force) {
+            need_space = true;
+        }
+    }
+
+    function print_comment(comment) {
+        var value = comment.value.replace(/[@#]__PURE__/g, " ");
+        if (/^\s*$/.test(value) && !/^\s*$/.test(comment.value)) return false;
+        if (/comment[134]/.test(comment.type)) {
+            print("//" + value);
+            need_newline_indented = true;
+        } else if (comment.type == "comment2") {
+            print("/*" + value + "*/");
+        }
+        return true;
+    }
+
     function prepend_comments(node) {
         var self = this;
         var scan = node instanceof AST_Exit && node.value;
@@ -489,37 +511,12 @@ function OutputStream(options) {
         }
 
         comments = comments.filter(comment_filter, node);
-        if (comments.length == 0) return;
-        var last_nlb = has_nlb();
-        comments.forEach(function(c, i) {
-            if (!last_nlb) {
-                if (c.nlb) {
-                    print("\n");
-                    indent();
-                    last_nlb = true;
-                } else if (i > 0) {
-                    space();
-                }
-            }
-            var value = c.value.replace(/[@#]__PURE__/g, " ");
-            if (/^\s*$/.test(value)) return;
-            if (/comment[134]/.test(c.type)) {
-                print("//" + value + "\n");
-                indent();
-                last_nlb = true;
-            } else if (c.type == "comment2") {
-                print("/*" + value + "*/");
-                last_nlb = false;
-            }
+        var printed = false;
+        comments.forEach(function(comment, index) {
+            pad_comment(comment, index);
+            if (print_comment(comment)) printed = true;
         });
-        if (!last_nlb) {
-            if (node.start.nlb) {
-                print("\n");
-                indent();
-            } else {
-                space();
-            }
-        }
+        if (printed) pad_comment(node.start, true);
 
         function dump(node) {
             var token = node.start;
@@ -549,27 +546,9 @@ function OutputStream(options) {
         }))) return;
         comments._dumped = self;
         var insert = OUTPUT.length;
-        comments.filter(comment_filter, node).forEach(function(c, i) {
-            need_space = false;
-            if (need_newline_indented) {
-                print("\n");
-                indent();
-                need_newline_indented = false;
-            } else if (c.nlb && (i > 0 || !has_nlb())) {
-                print("\n");
-                indent();
-            } else if (i > 0 || !tail) {
-                space();
-            }
-            var value = c.value.replace(/[@#]__PURE__/g, " ");
-            if (/^\s*$/.test(value)) return;
-            if (/comment[134]/.test(c.type)) {
-                print("//" + value);
-                need_newline_indented = true;
-            } else if (c.type == "comment2") {
-                print("/*" + value + "*/");
-                need_space = true;
-            }
+        comments.filter(comment_filter, node).forEach(function(comment, index) {
+            pad_comment(comment, index || !tail);
+            print_comment(comment);
         });
         if (OUTPUT.length > insert) newline_insert = insert;
     }
diff --git a/test/compress/annotations.js b/test/compress/annotations.js
index 8b6b0a4f..1d969c57 100644
--- a/test/compress/annotations.js
+++ b/test/compress/annotations.js
@@ -193,7 +193,7 @@ issue_2705_3: {
     }
     expect_exact: [
         "new h.x(1).y(2).z(3);",
-        "new i.x(1).y(2).z(3);",
+        "/* */new i.x(1).y(2).z(3);",
         "new j.x(1).y(2).z(3);",
         "new k.x(1).y(2).z(3);",
         "new l.x(1).y(2).z(3);",
@@ -246,7 +246,7 @@ issue_2705_6: {
     }
     expect_exact: [
         "x(),y();",
-        "new(a()||b())(c(),d());",
+        "/* */new(a()||b())(c(),d());",
     ]
 }
 
diff --git a/test/mocha/cli.js b/test/mocha/cli.js
index 9e5de83f..4d4972ec 100644
--- a/test/mocha/cli.js
+++ b/test/mocha/cli.js
@@ -29,7 +29,7 @@ describe("bin/uglifyjs", function() {
         var command = uglifyjscmd + ' test/input/comments/filter.js --comments all';
         exec(command, function(err, stdout) {
             if (err) throw err;
-            assert.strictEqual(stdout, "// foo\n/*@preserve*/\n// bar\n\n");
+            assert.strictEqual(stdout, "// foo\n/*@preserve*/\n// bar\n");
             done();
         });
     });
@@ -37,7 +37,7 @@ describe("bin/uglifyjs", function() {
         var command = uglifyjscmd + ' test/input/comments/filter.js --comments /r/';
         exec(command, function(err, stdout) {
             if (err) throw err;
-            assert.strictEqual(stdout, "/*@preserve*/\n// bar\n\n");
+            assert.strictEqual(stdout, "/*@preserve*/\n// bar\n");
             done();
         });
     });
@@ -45,7 +45,7 @@ describe("bin/uglifyjs", function() {
         var command = uglifyjscmd + ' test/input/comments/filter.js --comments';
         exec(command, function(err, stdout) {
             if (err) throw err;
-            assert.strictEqual(stdout, "/*@preserve*/\n\n");
+            assert.strictEqual(stdout, "/*@preserve*/\n");
             done();
         });
     });
diff --git a/test/mocha/comments.js b/test/mocha/comments.js
index 88cf08c4..830ca107 100644
--- a/test/mocha/comments.js
+++ b/test/mocha/comments.js
@@ -392,12 +392,12 @@ describe("comments", function() {
     describe("comment filters", function() {
         it("Should be able to filter comments by passing regexp", function() {
             var ast = UglifyJS.parse("/*!test1*/\n/*test2*/\n//!test3\n//test4\n<!--test5\n<!--!test6\n-->test7\n-->!test8");
-            assert.strictEqual(ast.print_to_string({comments: /^!/}), "/*!test1*/\n//!test3\n//!test6\n//!test8\n");
+            assert.strictEqual(ast.print_to_string({comments: /^!/}), "/*!test1*/\n//!test3\n//!test6\n//!test8");
         });
 
         it("Should be able to filter comments with the 'all' option", function() {
             var ast = UglifyJS.parse("/*!test1*/\n/*test2*/\n//!test3\n//test4\n<!--test5\n<!--!test6\n-->test7\n-->!test8");
-            assert.strictEqual(ast.print_to_string({comments: "all"}), "/*!test1*/\n/*test2*/\n//!test3\n//test4\n//test5\n//!test6\n//test7\n//!test8\n");
+            assert.strictEqual(ast.print_to_string({comments: "all"}), "/*!test1*/\n/*test2*/\n//!test3\n//test4\n//test5\n//!test6\n//test7\n//!test8");
         });
 
         it("Should be able to filter commments with the 'some' option", function() {
@@ -410,13 +410,12 @@ describe("comments", function() {
             var f = function(node, comment) {
                 return comment.value.length === 8;
             };
-
-            assert.strictEqual(ast.print_to_string({comments: f}), "/*TEST 123*/\n//8 chars.\n");
+            assert.strictEqual(ast.print_to_string({comments: f}), "/*TEST 123*/\n//8 chars.");
         });
 
         it("Should be able to filter comments by passing regex in string format", function() {
             var ast = UglifyJS.parse("/*!test1*/\n/*test2*/\n//!test3\n//test4\n<!--test5\n<!--!test6\n-->test7\n-->!test8");
-            assert.strictEqual(ast.print_to_string({comments: "/^!/"}), "/*!test1*/\n//!test3\n//!test6\n//!test8\n");
+            assert.strictEqual(ast.print_to_string({comments: "/^!/"}), "/*!test1*/\n//!test3\n//!test6\n//!test8");
         });
 
         it("Should be able to get the comment and comment type when using a function", function() {
@@ -424,14 +423,12 @@ describe("comments", function() {
             var f = function(node, comment) {
                 return comment.type == "comment1" || comment.type == "comment3";
             };
-
-            assert.strictEqual(ast.print_to_string({comments: f}), "//!test3\n//test4\n//test5\n//!test6\n");
+            assert.strictEqual(ast.print_to_string({comments: f}), "//!test3\n//test4\n//test5\n//!test6");
         });
 
         it("Should be able to filter comments by passing a boolean", function() {
             var ast = UglifyJS.parse("/*!test1*/\n/*test2*/\n//!test3\n//test4\n<!--test5\n<!--!test6\n-->test7\n-->!test8");
-
-            assert.strictEqual(ast.print_to_string({comments: true}), "/*!test1*/\n/*test2*/\n//!test3\n//test4\n//test5\n//!test6\n//test7\n//!test8\n");
+            assert.strictEqual(ast.print_to_string({comments: true}), "/*!test1*/\n/*test2*/\n//!test3\n//test4\n//test5\n//!test6\n//test7\n//!test8");
             assert.strictEqual(ast.print_to_string({comments: false}), "");
         });
 
@@ -439,10 +436,8 @@ describe("comments", function() {
             var ast = UglifyJS.parse("#!Random comment\n//test1\n/*test2*/");
             var f = function(node, comment) {
                 assert.strictEqual(comment.type === "comment5", false);
-
                 return true;
             };
-
             assert.strictEqual(ast.print_to_string({comments: f}), "#!Random comment\n//test1\n/*test2*/");
         });
 
@@ -453,9 +448,8 @@ describe("comments", function() {
 
         it("Should have no problem on multiple calls", function() {
             const options = {
-                comments: /ok/
+                comments: /ok/,
             };
-
             assert.strictEqual(UglifyJS.parse("/* ok */ function a(){}").print_to_string(options), "/* ok */function a(){}");
             assert.strictEqual(UglifyJS.parse("/* ok */ function a(){}").print_to_string(options), "/* ok */function a(){}");
             assert.strictEqual(UglifyJS.parse("/* ok */ function a(){}").print_to_string(options), "/* ok */function a(){}");
@@ -463,14 +457,14 @@ describe("comments", function() {
 
         it("Should handle shebang and preamble correctly", function() {
             var code = UglifyJS.minify("#!/usr/bin/node\nvar x = 10;", {
-                output: { preamble: "/* Build */" }
+                output: { preamble: "/* Build */" },
             }).code;
             assert.strictEqual(code, "#!/usr/bin/node\n/* Build */\nvar x=10;");
         });
 
         it("Should handle preamble without shebang correctly", function() {
             var code = UglifyJS.minify("var x = 10;", {
-                output: { preamble: "/* Build */" }
+                output: { preamble: "/* Build */" },
             }).code;
             assert.strictEqual(code, "/* Build */\nvar x=10;");
         });