From 79b98a9fe87f950607c601a45a3566a46c32f425 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Wed, 26 Oct 2016 17:34:30 +0200 Subject: [PATCH] Do not overwrite options.comments + cleanup --- lib/compress.js | 24 +++++------ lib/output.js | 77 ++++++++++++++++++------------------ lib/utils.js | 2 + test/mocha/comment-filter.js | 12 +++++- 4 files changed, 63 insertions(+), 52 deletions(-) diff --git a/lib/compress.js b/lib/compress.js index 1845717c..5879b93b 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -916,7 +916,7 @@ merge(Compressor.prototype, { (function (def){ var unary_bool = [ "!", "delete" ]; var binary_bool = [ "in", "instanceof", "==", "!=", "===", "!==", "<", "<=", ">=", ">" ]; - def(AST_Node, function(){ return false }); + def(AST_Node, return_false); def(AST_UnaryPrefix, function(){ return member(this.operator, unary_bool); }); @@ -934,16 +934,16 @@ merge(Compressor.prototype, { def(AST_Seq, function(){ return this.cdr.is_boolean(); }); - def(AST_True, function(){ return true }); - def(AST_False, function(){ return true }); + def(AST_True, return_true); + def(AST_False, return_true); })(function(node, func){ node.DEFMETHOD("is_boolean", func); }); // methods to determine if an expression has a string result type (function (def){ - def(AST_Node, function(){ return false }); - def(AST_String, function(){ return true }); + def(AST_Node, return_false); + def(AST_String, return_true); def(AST_UnaryPrefix, function(){ return this.operator == "typeof"; }); @@ -1197,11 +1197,11 @@ merge(Compressor.prototype, { // determine if expression has side effects (function(def){ - def(AST_Node, function(compressor){ return true }); + def(AST_Node, return_true); - def(AST_EmptyStatement, function(compressor){ return false }); - def(AST_Constant, function(compressor){ return false }); - def(AST_This, function(compressor){ return false }); + def(AST_EmptyStatement, return_false); + def(AST_Constant, return_false); + def(AST_This, return_false); def(AST_Call, function(compressor){ var pure = compressor.option("pure_funcs"); @@ -1221,13 +1221,13 @@ merge(Compressor.prototype, { def(AST_SimpleStatement, function(compressor){ return this.body.has_side_effects(compressor); }); - def(AST_Defun, function(compressor){ return true }); - def(AST_Function, function(compressor){ return false }); + def(AST_Defun, return_true); + def(AST_Function, return_false); def(AST_Binary, function(compressor){ return this.left.has_side_effects(compressor) || this.right.has_side_effects(compressor); }); - def(AST_Assign, function(compressor){ return true }); + def(AST_Assign, return_true); def(AST_Conditional, function(compressor){ return this.condition.has_side_effects(compressor) || this.consequent.has_side_effects(compressor) diff --git a/lib/output.js b/lib/output.js index 63a78c60..50e5aa43 100644 --- a/lib/output.js +++ b/lib/output.js @@ -45,6 +45,20 @@ var EXPECT_DIRECTIVE = /^$|[;{][\s\n]*$/; +function is_some_comments(comment) { + var text = comment.value; + var type = comment.type; + if (type == "comment2") { + // multiline comment + return /@preserve|@license|@cc_on/i.test(text); + } + return type == "comment5"; +} + +function is_comment5(comment) { + return comment.type == "comment5"; +} + function OutputStream(options) { options = defaults(options, { @@ -72,46 +86,30 @@ function OutputStream(options) { }, true); // Convert comment option to RegExp if neccessary and set up comments filter - if (typeof options.comments === "string" && /^\/.*\/[a-zA-Z]*$/.test(options.comments)) { - var regex_pos = options.comments.lastIndexOf("/"); - options.comments = new RegExp( - options.comments.substr(1, regex_pos - 1), - options.comments.substr(regex_pos + 1) - ); - } - if (options.comments instanceof RegExp) { - options.comments = (function(f) { - return function(comment) { - return comment.type == "comment5" || f.test(comment.value); - } - })(options.comments); - } - else if (typeof options.comments === "function") { - options.comments = (function(f) { - return function(comment) { - return comment.type == "comment5" || f(this, comment); - } - })(options.comments); - } - else if (options.comments === "some") { - options.comments = function(comment) { - var text = comment.value; - var type = comment.type; - if (type == "comment2") { - // multiline comment - return /@preserve|@license|@cc_on/i.test(text); - } - return type == "comment5"; + var comment_filter = options.shebang ? is_comment5 : return_false; // Default case, throw all comments away except shebangs + if (options.comments) { + var comments = options.comments; + if (typeof options.comments === "string" && /^\/.*\/[a-zA-Z]*$/.test(options.comments)) { + var regex_pos = options.comments.lastIndexOf("/"); + comments = new RegExp( + options.comments.substr(1, regex_pos - 1), + options.comments.substr(regex_pos + 1) + ); } - } - else if (options.comments){ // NOTE includes "all" option - options.comments = function() { - return true; + if (comments instanceof RegExp) { + comment_filter = function(comment) { + return comment.type == "comment5" || comments.test(comment.value); + }; + } + else if (typeof comments === "function") { + comment_filter = function(comment) { + return comment.type == "comment5" || comments(this, comment); + }; } - } else { - // Falsy case, so reject all comments, except shebangs - options.comments = function(comment) { - return comment.type == "comment5"; + else if (comments === "some") { + comment_filter = is_some_comments; + } else { // NOTE includes "all" option + comment_filter = return_true; } } @@ -421,6 +419,7 @@ function OutputStream(options) { with_square : with_square, add_mapping : add_mapping, option : function(opt) { return options[opt] }, + comment_filter : comment_filter, line : function() { return current_line }, col : function() { return current_col }, pos : function() { return current_pos }, @@ -503,7 +502,7 @@ function OutputStream(options) { })); } - comments = comments.filter(output.option("comments"), self); + comments = comments.filter(output.comment_filter, self); // Keep single line comments after nlb, after nlb if (!output.option("beautify") && comments.length > 0 && diff --git a/lib/utils.js b/lib/utils.js index 8ef61936..d0a21ac9 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -112,6 +112,8 @@ function merge(obj, ext) { }; function noop() {}; +function return_false() { return false; } +function return_true() { return true; } var MAP = (function(){ function MAP(a, f, backwards) { diff --git a/test/mocha/comment-filter.js b/test/mocha/comment-filter.js index 79162755..01580c87 100644 --- a/test/mocha/comment-filter.js +++ b/test/mocha/comment-filter.js @@ -2,7 +2,7 @@ var UglifyJS = require('../../'); var assert = require("assert"); describe("comment filters", function() { - it("Should be able to filter comments by passing regex", function() { + it("Should be able to filter comments by passing regexp", function() { var ast = UglifyJS.parse("/*!test1*/\n/*test2*/\n//!test3\n//test4\ntest7\n-->!test8"); assert.strictEqual(ast.print_to_string({comments: /^!/}), "/*!test1*/\n//!test3\n//!test6\n//!test8\n"); }); @@ -62,4 +62,14 @@ describe("comment filters", 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"); }); + + it("Should have no problem on multiple calls", function() { + const options = { + 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(){}"); + }); }); -- 2.34.1