From 6f3e35bb3f04303e6b7cc74cfc25bfee3c792a98 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Sun, 27 Dec 2015 22:24:37 +0100 Subject: [PATCH 01/15] Fix ch that could contain other newline characters --- lib/parse.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/parse.js b/lib/parse.js index 7e7b2272..2218c00c 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -399,7 +399,7 @@ function tokenizer($TEXT, filename, html5_comments, shebang) { if (octal_len > 0) ch = String.fromCharCode(parseInt(ch, 8)); else ch = read_escaped_char(true); } - else if (ch == "\n") parse_error("Unterminated string constant"); + else if ("\r\n\u2028\u2029".indexOf(ch) >= 0) parse_error("Unterminated string constant"); else if (ch == quote) break; ret += ch; } From 8c6af09ae014eb2370349fb7b419ee912abac64f Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Sun, 27 Dec 2015 22:28:03 +0100 Subject: [PATCH 02/15] Add mocha tests --- package.json | 3 ++- test/mocha.js | 29 +++++++++++++++++++++++++++++ test/mocha/string-literal.js | 29 +++++++++++++++++++++++++++++ test/run-tests.js | 3 +++ 4 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 test/mocha.js create mode 100644 test/mocha/string-literal.js diff --git a/package.json b/package.json index 6b0d2f40..bd4fb3e7 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,8 @@ "acorn": "~0.6.0", "escodegen": "~1.3.3", "esfuzz": "~0.3.1", - "estraverse": "~1.5.1" + "estraverse": "~1.5.1", + "mocha": "~2.3.4" }, "browserify": { "transform": [ diff --git a/test/mocha.js b/test/mocha.js new file mode 100644 index 00000000..411f52c5 --- /dev/null +++ b/test/mocha.js @@ -0,0 +1,29 @@ +var Mocha = require('mocha'), + fs = require('fs'), + path = require('path'); + +// Instantiate a Mocha instance. +var mocha = new Mocha({}); + +var testDir = __dirname + '/mocha/'; + +// Add each .js file to the mocha instance +fs.readdirSync(testDir).filter(function(file){ + // Only keep the .js files + return file.substr(-3) === '.js'; + +}).forEach(function(file){ + mocha.addFile( + path.join(testDir, file) + ); +}); + +module.exports = function() { + mocha.run(function(failures) { + if (failures !== 0) { + process.on('exit', function () { + process.exit(failures); + }); + } + }); +}; \ No newline at end of file diff --git a/test/mocha/string-literal.js b/test/mocha/string-literal.js new file mode 100644 index 00000000..64933632 --- /dev/null +++ b/test/mocha/string-literal.js @@ -0,0 +1,29 @@ +var UglifyJS = require('../../'); +var assert = require("assert"); + +describe("String literals", function() { + it("Should throw syntax error if a string literal contains a newline", function() { + var inputs = [ + "'\n'", + "'\r'", + '"\r\n"', + "'\u2028'", + '"\u2029"' + ]; + + var test = function(input) { + return function() { + var ast = UglifyJS.parse(input); + } + }; + + var error = function(e) { + return e instanceof UglifyJS.JS_Parse_Error && + e.message === "Unterminated string constant" + }; + + for (var input in inputs) { + assert.throws(test(inputs[input]), error); + } + }); +}); \ No newline at end of file diff --git a/test/run-tests.js b/test/run-tests.js index 3ec04fda..b9a0f825 100755 --- a/test/run-tests.js +++ b/test/run-tests.js @@ -16,6 +16,9 @@ if (failures) { process.exit(1); } +var mocha_tests = require("./mocha.js"); +mocha_tests(); + var run_sourcemaps_tests = require('./sourcemaps'); run_sourcemaps_tests(); From fe4e9f9d97dcc6594a8fc49e04630aa619ff1866 Mon Sep 17 00:00:00 2001 From: Mihai Bazon Date: Tue, 5 Jan 2016 13:56:52 +0200 Subject: [PATCH 03/15] Fix hoisting the var in ForIn Close #913 --- lib/compress.js | 5 ++++- test/compress/issue-913.js | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 test/compress/issue-913.js diff --git a/lib/compress.js b/lib/compress.js index 44e19799..1f5988f7 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -1276,7 +1276,10 @@ merge(Compressor.prototype, { var seq = node.to_assignments(); var p = tt.parent(); if (p instanceof AST_ForIn && p.init === node) { - if (seq == null) return node.definitions[0].name; + if (seq == null) { + var def = node.definitions[0].name; + return make_node(AST_SymbolRef, def, def); + } return seq; } if (p instanceof AST_For && p.init === node) { diff --git a/test/compress/issue-913.js b/test/compress/issue-913.js new file mode 100644 index 00000000..9d34d9d9 --- /dev/null +++ b/test/compress/issue-913.js @@ -0,0 +1,20 @@ +keep_var_for_in: { + options = { + hoist_vars: true, + unused: true + }; + input: { + (function(obj){ + var foo = 5; + for (var i in obj) + return foo; + })(); + } + expect: { + (function(obj){ + var i, foo = 5; + for (i in obj) + return foo; + })(); + } +} From 88b77ddaa9d6b3d55e537dc21030ac58ddfcb86e Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Wed, 13 Jan 2016 00:30:32 +0100 Subject: [PATCH 04/15] Add test case for line continuation --- test/mocha/string-literal.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/mocha/string-literal.js b/test/mocha/string-literal.js index 64933632..84aaad7e 100644 --- a/test/mocha/string-literal.js +++ b/test/mocha/string-literal.js @@ -14,16 +14,21 @@ describe("String literals", function() { var test = function(input) { return function() { var ast = UglifyJS.parse(input); - } + }; }; var error = function(e) { return e instanceof UglifyJS.JS_Parse_Error && - e.message === "Unterminated string constant" + e.message === "Unterminated string constant"; }; for (var input in inputs) { assert.throws(test(inputs[input]), error); } }); + + it("Should not throw syntax error if a string has a line continuation", function() { + var output = UglifyJS.parse('var a = "a\\\nb";').print_to_string(); + assert.equal(output, 'var a="ab";'); + }); }); \ No newline at end of file From 6605d1578351939ee0e39a13bf68cc9c1708c918 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Sun, 10 Jan 2016 23:33:54 +0100 Subject: [PATCH 05/15] Never mangle arguments and keep them in their scope Fixes #892 Helped-by: kzc --- lib/scope.js | 8 ++++++++ test/compress/issue-892.js | 32 ++++++++++++++++++++++++++++++++ test/run-tests.js | 4 ++++ 3 files changed, 44 insertions(+) create mode 100644 test/compress/issue-892.js diff --git a/lib/scope.js b/lib/scope.js index 1f0986c4..5e93020f 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -237,6 +237,10 @@ AST_Scope.DEFMETHOD("init_scope_vars", function(nesting){ AST_Lambda.DEFMETHOD("init_scope_vars", function(){ AST_Scope.prototype.init_scope_vars.apply(this, arguments); this.uses_arguments = false; + + var symbol = new AST_VarDef({ name: "arguments" }); + var def = new SymbolDef(this, this.variables.size(), symbol); + this.variables.set(symbol.name, def); }); AST_SymbolRef.DEFMETHOD("reference", function() { @@ -366,6 +370,10 @@ AST_Toplevel.DEFMETHOD("_default_mangler_options", function(options){ AST_Toplevel.DEFMETHOD("mangle_names", function(options){ options = this._default_mangler_options(options); + + // Never mangle arguments + options.except.push('arguments'); + // We only need to mangle declaration nodes. Special logic wired // into the code generator will display the mangled name if it's // present (and for AST_SymbolRef-s it'll use the mangled name of diff --git a/test/compress/issue-892.js b/test/compress/issue-892.js new file mode 100644 index 00000000..2dab420f --- /dev/null +++ b/test/compress/issue-892.js @@ -0,0 +1,32 @@ +dont_mangle_arguments: { + mangle = { + }; + options = { + sequences : true, + properties : true, + dead_code : true, + drop_debugger : true, + conditionals : true, + comparisons : true, + evaluate : true, + booleans : true, + loops : true, + unused : true, + hoist_funs : true, + keep_fargs : true, + keep_fnames : false, + hoist_vars : true, + if_return : true, + join_vars : true, + cascade : true, + side_effects : true, + negate_iife : false + }; + input: { + (function(){ + var arguments = arguments, not_arguments = 9; + console.log(not_arguments, arguments); + })(5,6,7); + } + expect_exact: "(function(){var arguments=arguments,o=9;console.log(o,arguments)})(5,6,7);" +} diff --git a/test/run-tests.js b/test/run-tests.js index b9a0f825..fcb1b375 100755 --- a/test/run-tests.js +++ b/test/run-tests.js @@ -103,6 +103,10 @@ function run_compress_tests() { } var output = input.transform(cmp); output.figure_out_scope(); + if (test.mangle) { + output.compute_char_frequency(test.mangle); + output.mangle_names(test.mangle); + } output = make_code(output, output_options); if (expect != output) { log("!!! failed\n---INPUT---\n{input}\n---OUTPUT---\n{output}\n---EXPECTED---\n{expected}\n\n", { From 5c4e470d43438e359fbdb93950e5d37d4df45a69 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Thu, 14 Jan 2016 22:32:46 +0100 Subject: [PATCH 06/15] Add scope test for arguments --- test/mocha/arguments.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 test/mocha/arguments.js diff --git a/test/mocha/arguments.js b/test/mocha/arguments.js new file mode 100644 index 00000000..294a6c16 --- /dev/null +++ b/test/mocha/arguments.js @@ -0,0 +1,19 @@ +var UglifyJS = require('../../'); +var assert = require("assert"); + +describe("arguments", function() { + it("Should known that arguments in functions are local scoped", function() { + var ast = UglifyJS.parse("var arguments; var f = function() {arguments.length}"); + ast.figure_out_scope(); + + // Select symbol in function + var symbol = ast.body[1].definitions[0].value.find_variable("arguments"); + + assert.strictEqual(symbol.global, false); + assert.strictEqual(symbol.scope, ast. // From ast + body[1]. // Select 2nd statement (equals to `var f ...`) + definitions[0]. // First definition of selected statement + value // Select function as scope + ); + }); +}); \ No newline at end of file From 8439c8ba9813faaea062b64306cbd0b2a448bb20 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Fri, 15 Jan 2016 00:04:05 +0100 Subject: [PATCH 07/15] Make arguments test slightly more strict --- test/mocha/arguments.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/mocha/arguments.js b/test/mocha/arguments.js index 294a6c16..089826fc 100644 --- a/test/mocha/arguments.js +++ b/test/mocha/arguments.js @@ -6,7 +6,10 @@ describe("arguments", function() { var ast = UglifyJS.parse("var arguments; var f = function() {arguments.length}"); ast.figure_out_scope(); - // Select symbol in function + // Test scope of `var arguments` + assert.strictEqual(ast.find_variable("arguments").global, true); + + // Select arguments symbol in function var symbol = ast.body[1].definitions[0].value.find_variable("arguments"); assert.strictEqual(symbol.global, false); From 70e5b6f15b130eb1366ff81e0a8a7f187e9cf427 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Tue, 19 Jan 2016 14:00:22 +0100 Subject: [PATCH 08/15] Add some tests for comment-filters through api Also never bother comment options to filter comment5/shebang comments as they have their custom filter. --- lib/output.js | 4 ++-- test/mocha/comment-filter.js | 45 ++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 test/mocha/comment-filter.js diff --git a/lib/output.js b/lib/output.js index f10c918a..dceece34 100644 --- a/lib/output.js +++ b/lib/output.js @@ -444,11 +444,11 @@ function OutputStream(options) { }); } else if (c.test) { comments = comments.filter(function(comment){ - return c.test(comment.value) || comment.type == "comment5"; + return comment.type == "comment5" || c.test(comment.value); }); } else if (typeof c == "function") { comments = comments.filter(function(comment){ - return c(self, comment) || comment.type == "comment5"; + return comment.type == "comment5" || c(self, comment); }); } diff --git a/test/mocha/comment-filter.js b/test/mocha/comment-filter.js new file mode 100644 index 00000000..ea2ec2eb --- /dev/null +++ b/test/mocha/comment-filter.js @@ -0,0 +1,45 @@ +var UglifyJS = require('../../'); +var assert = require("assert"); + +describe("comment filters", function() { + it("Should be able to filter comments by passing regex", 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"); + }); + + it("Should be able to filter comments by passing a function", function() { + var ast = UglifyJS.parse("/*TEST 123*/\n//An other comment\n//8 chars."); + var f = function(node, comment) { + return comment.value.length === 8; + }; + + assert.strictEqual(ast.print_to_string({comments: f}), "/*TEST 123*/\n//8 chars.\n"); + }); + + it("Should be able to get the comment and comment type when using a function", function() { + var ast = UglifyJS.parse("/*!test1*/\n/*test2*/\n//!test3\n//test4\ntest7\n-->!test8"); + 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"); + }); + + it("Should be able to filter comments by passing a boolean", function() { + var ast = UglifyJS.parse("/*!test1*/\n/*test2*/\n//!test3\n//test4\ntest7\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: false}), ""); + }); + + it("Should never be able to filter comment5 (shebangs)", 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*/\n"); + }); +}); From ebe118dc79aca8b143409f13b336c2a04f028fa4 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Sat, 16 Jan 2016 21:35:39 +0100 Subject: [PATCH 09/15] Add keywords to package.json Should hopefully bump up on the results of the npm site when searching `uglify` --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index bd4fb3e7..da0f835e 100644 --- a/package.json +++ b/package.json @@ -49,5 +49,6 @@ "scripts": { "shrinkwrap": "rm ./npm-shrinkwrap.json; rm -rf ./node_modules; npm i && npm shrinkwrap && npm outdated", "test": "node test/run-tests.js" - } + }, + "keywords": ["uglify", "uglify-js", "minify", "minifier"] } From 26641f3fb20bce9394c3989bea0099dcd209be61 Mon Sep 17 00:00:00 2001 From: Anthony Van de Gejuchte Date: Fri, 15 Jan 2016 15:58:15 +0100 Subject: [PATCH 10/15] Allow operator names as getters/setters Fixes #919 Fix provided by @kzc --- lib/parse.js | 7 +++ test/compress/issue-12.js | 47 ++++++++++++++++++++ test/mocha/getter-setter.js | 89 +++++++++++++++++++++++++++++++++++++ 3 files changed, 143 insertions(+) create mode 100644 test/mocha/getter-setter.js diff --git a/lib/parse.js b/lib/parse.js index 2218c00c..f1495153 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -1178,6 +1178,13 @@ function parse($TEXT, options) { break; } break; + case "operator": + if (!is_identifier_string(tok.value)) { + throw new JS_Parse_Error("Invalid getter/setter name: " + tok.value, + tok.file, tok.line, tok.col, tok.pos); + } + ret = _make_symbol(AST_SymbolRef); + break; } next(); return ret; diff --git a/test/compress/issue-12.js b/test/compress/issue-12.js index bf87d5c0..e2d8bda7 100644 --- a/test/compress/issue-12.js +++ b/test/compress/issue-12.js @@ -9,3 +9,50 @@ keep_name_of_setter: { input: { a = { set foo () {} } } expect: { a = { set foo () {} } } } + +setter_with_operator_keys: { + input: { + var tokenCodes = { + get instanceof(){ + return test0; + }, + set instanceof(value){ + test0 = value; + }, + set typeof(value){ + test1 = value; + }, + get typeof(){ + return test1; + }, + set else(value){ + test2 = value; + }, + get else(){ + return test2; + } + }; + } + expect: { + var tokenCodes = { + get instanceof(){ + return test0; + }, + set instanceof(value){ + test0 = value; + }, + set typeof(value){ + test1 = value; + }, + get typeof(){ + return test1; + }, + set else(value){ + test2 = value; + }, + get else(){ + return test2; + } + }; + } +} \ No newline at end of file diff --git a/test/mocha/getter-setter.js b/test/mocha/getter-setter.js new file mode 100644 index 00000000..641a2026 --- /dev/null +++ b/test/mocha/getter-setter.js @@ -0,0 +1,89 @@ +var UglifyJS = require('../../'); +var assert = require("assert"); + +describe("Getters and setters", function() { + it("Should not accept operator symbols as getter/setter name", function() { + var illegalOperators = [ + "++", + "--", + "+", + "-", + "!", + "~", + "&", + "|", + "^", + "*", + "/", + "%", + ">>", + "<<", + ">>>", + "<", + ">", + "<=", + ">=", + "==", + "===", + "!=", + "!==", + "?", + "=", + "+=", + "-=", + "/=", + "*=", + "%=", + ">>=", + "<<=", + ">>>=", + "|=", + "^=", + "&=", + "&&", + "||" + ]; + var generator = function() { + var results = []; + + for (var i in illegalOperators) { + results.push({ + code: "var obj = { get " + illegalOperators[i] + "() { return test; }};", + operator: illegalOperators[i], + method: "get" + }); + results.push({ + code: "var obj = { set " + illegalOperators[i] + "(value) { test = value}};", + operator: illegalOperators[i], + method: "set" + }); + } + + return results; + }; + + var testCase = function(data) { + return function() { + UglifyJS.parse(data.code); + }; + }; + + var fail = function(data) { + return function (e) { + return e instanceof UglifyJS.JS_Parse_Error && + e.message === "Invalid getter/setter name: " + data.operator; + }; + }; + + var errorMessage = function(data) { + return "Expected but didn't get a syntax error while parsing following line:\n" + data.code; + }; + + var tests = generator(); + for (var i = 0; i < tests.length; i++) { + var test = tests[i]; + assert.throws(testCase(test), fail(test), errorMessage(test)); + } + }); + +}); From 8b71c6559b0e1773bb3455c68701ff512fc18277 Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Tue, 19 Jan 2016 13:12:32 -0600 Subject: [PATCH 11/15] Mark vars with /** @const */ pragma as consts so they can be eliminated. Fixes older browser support for consts and allows more flexibility in dead code removal. --- lib/scope.js | 12 +++++++++- test/compress/dead-code.js | 46 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/lib/scope.js b/lib/scope.js index 5e93020f..22fb150d 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -154,7 +154,7 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){ else if (node instanceof AST_SymbolVar || node instanceof AST_SymbolConst) { var def = defun.def_variable(node); - def.constant = node instanceof AST_SymbolConst; + def.constant = node instanceof AST_SymbolConst || node.has_const_pragma(); def.init = tw.parent().value; } else if (node instanceof AST_SymbolCatch) { @@ -357,6 +357,16 @@ AST_Symbol.DEFMETHOD("global", function(){ return this.definition().global; }); +AST_Symbol.DEFMETHOD("has_const_pragma", function() { + var token = this.scope.body[0] && this.scope.body[0].start; + var comments = token && token.comments_before; + if (comments && comments.length > 0) { + var last = comments[comments.length - 1]; + return /@const/.test(last.value); + } + return false; +}) + AST_Toplevel.DEFMETHOD("_default_mangler_options", function(options){ return defaults(options, { except : [], diff --git a/test/compress/dead-code.js b/test/compress/dead-code.js index 5009ae1e..f79b04de 100644 --- a/test/compress/dead-code.js +++ b/test/compress/dead-code.js @@ -87,3 +87,49 @@ dead_code_constant_boolean_should_warn_more: { var moo; } } + +dead_code_const_declaration: { + options = { + dead_code : true, + loops : true, + booleans : true, + conditionals : true, + evaluate : true + }; + input: { + const CONST_FOO = false; + if (CONST_FOO) { + console.log("unreachable"); + var moo; + function bar() {} + } + } + expect: { + const CONST_FOO = !1; + var moo; + function bar() {} + } +} + +dead_code_const_annotation: { + options = { + dead_code : true, + loops : true, + booleans : true, + conditionals : true, + evaluate : true + }; + input: { + /** @const*/ var CONST_FOO_ANN = false; + if (CONST_FOO_ANN) { + console.log("unreachable"); + var moo; + function bar() {} + } + } + expect: { + var CONST_FOO_ANN = !1; + var moo; + function bar() {} + } +} From 918c17bd88647899be7fa1d9adabfe635cd6102d Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Tue, 19 Jan 2016 13:24:36 -0600 Subject: [PATCH 12/15] Update README for /** @const */ --- README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 67324dbd..6dea439a 100644 --- a/README.md +++ b/README.md @@ -395,6 +395,8 @@ separate file and include it into the build. For example you can have a ```javascript const DEBUG = false; const PRODUCTION = true; +// Alternative for environments that don't support `const` +/** @const */ var STAGING = false; // etc. ``` @@ -404,8 +406,8 @@ and build your code like this: UglifyJS will notice the constants and, since they cannot be altered, it will evaluate references to them to the value itself and drop unreachable -code as usual. The possible downside of this approach is that the build -will contain the `const` declarations. +code as usual. The build will contain the `const` declarations if you use +them. If you are targeting < ES6 environments, use `/** @const */ var`. ## Beautifier options From f97da4294a7e9adbd560ecafd94ec697de35affc Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Wed, 20 Jan 2016 10:52:48 -0600 Subject: [PATCH 13/15] Use TreeWalker for more accurate @const results and update tests --- lib/scope.js | 32 +++++++++++++++++++------ test/compress/dead-code.js | 49 +++++++++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/lib/scope.js b/lib/scope.js index 22fb150d..144ae48e 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -358,13 +358,31 @@ AST_Symbol.DEFMETHOD("global", function(){ }); AST_Symbol.DEFMETHOD("has_const_pragma", function() { - var token = this.scope.body[0] && this.scope.body[0].start; - var comments = token && token.comments_before; - if (comments && comments.length > 0) { - var last = comments[comments.length - 1]; - return /@const/.test(last.value); - } - return false; + var symbol = this; + var symbol_has_pragma = false; + var pragma_found = false; + var found_symbol = false; + // Walk the current scope, looking for a comment with the @const pragma. + // If it exists, mark a bool that will remain true only for the next iteration. + // If the next iteration is this symbol, then we return true. + // Otherwise we stop descending and get out of here. + var tw = new TreeWalker(function(node, descend){ + // This is our symbol. Was the pragma before this? + if (node.name === symbol) { + found_symbol = true; + symbol_has_pragma = pragma_found; + } + + // Look for the /** @const */ pragma + var comments_before = node.start && node.start.comments_before; + var lastComment = comments_before && comments_before[comments_before.length - 1]; + pragma_found = lastComment && /@const/.test(lastComment.value); + + // no need to descend after finding our node + return found_symbol; + }); + this.scope.walk(tw); + return symbol_has_pragma; }) AST_Toplevel.DEFMETHOD("_default_mangler_options", function(options){ diff --git a/test/compress/dead-code.js b/test/compress/dead-code.js index f79b04de..8aad336c 100644 --- a/test/compress/dead-code.js +++ b/test/compress/dead-code.js @@ -97,6 +97,7 @@ dead_code_const_declaration: { evaluate : true }; input: { + var unused; const CONST_FOO = false; if (CONST_FOO) { console.log("unreachable"); @@ -105,6 +106,7 @@ dead_code_const_declaration: { } } expect: { + var unused; const CONST_FOO = !1; var moo; function bar() {} @@ -120,7 +122,8 @@ dead_code_const_annotation: { evaluate : true }; input: { - /** @const*/ var CONST_FOO_ANN = false; + var unused; + /** @const */ var CONST_FOO_ANN = false; if (CONST_FOO_ANN) { console.log("unreachable"); var moo; @@ -128,8 +131,52 @@ dead_code_const_annotation: { } } expect: { + var unused; var CONST_FOO_ANN = !1; var moo; function bar() {} } } + +dead_code_const_annotation_complex_scope: { + options = { + dead_code : true, + loops : true, + booleans : true, + conditionals : true, + evaluate : true + }; + input: { + var unused_var; + /** @const */ var test = 'test'; + /** @const */ var CONST_FOO_ANN = false; + var unused_var_2; + if (CONST_FOO_ANN) { + console.log("unreachable"); + var moo; + function bar() {} + } + if (test === 'test') { + var beef = 'good'; + /** @const */ var meat = 'beef'; + var pork = 'bad'; + if (meat === 'pork') { + console.log('also unreachable'); + } else if (pork === 'good') { + console.log('reached, not const'); + } + } + } + expect: { + var unused_var; + var test = 'test'; + var CONST_FOO_ANN = !1; + var unused_var_2; + var moo; + function bar() {} + var beef = 'good'; + var meat = 'beef'; + var pork = 'bad'; + 'good' === pork && console.log('reached, not const'); + } +} From 4a7179ff9183cd27d036043c0bbcb01d1604d824 Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Wed, 20 Jan 2016 11:03:41 -0600 Subject: [PATCH 14/15] Simplify by skipping extra tree walk. --- lib/scope.js | 38 ++++++++++---------------------------- 1 file changed, 10 insertions(+), 28 deletions(-) diff --git a/lib/scope.js b/lib/scope.js index 144ae48e..794254d4 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -94,6 +94,7 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){ var scope = self.parent_scope = null; var labels = new Dictionary(); var defun = null; + var last_var_had_const_pragma = false; var nesting = 0; var tw = new TreeWalker(function(node, descend){ if (options.screw_ie8 && node instanceof AST_Catch) { @@ -151,10 +152,13 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){ // later. (node.scope = defun.parent_scope).def_function(node); } + else if (node instanceof AST_Var) { + last_var_had_const_pragma = node.has_const_pragma(); + } else if (node instanceof AST_SymbolVar || node instanceof AST_SymbolConst) { var def = defun.def_variable(node); - def.constant = node instanceof AST_SymbolConst || node.has_const_pragma(); + def.constant = node instanceof AST_SymbolConst || last_var_had_const_pragma; def.init = tw.parent().value; } else if (node instanceof AST_SymbolCatch) { @@ -357,33 +361,11 @@ AST_Symbol.DEFMETHOD("global", function(){ return this.definition().global; }); -AST_Symbol.DEFMETHOD("has_const_pragma", function() { - var symbol = this; - var symbol_has_pragma = false; - var pragma_found = false; - var found_symbol = false; - // Walk the current scope, looking for a comment with the @const pragma. - // If it exists, mark a bool that will remain true only for the next iteration. - // If the next iteration is this symbol, then we return true. - // Otherwise we stop descending and get out of here. - var tw = new TreeWalker(function(node, descend){ - // This is our symbol. Was the pragma before this? - if (node.name === symbol) { - found_symbol = true; - symbol_has_pragma = pragma_found; - } - - // Look for the /** @const */ pragma - var comments_before = node.start && node.start.comments_before; - var lastComment = comments_before && comments_before[comments_before.length - 1]; - pragma_found = lastComment && /@const/.test(lastComment.value); - - // no need to descend after finding our node - return found_symbol; - }); - this.scope.walk(tw); - return symbol_has_pragma; -}) +AST_Var.DEFMETHOD("has_const_pragma", function() { + var comments_before = this.start && this.start.comments_before; + var lastComment = comments_before && comments_before[comments_before.length - 1]; + return lastComment && /@const/.test(lastComment.value); +}); AST_Toplevel.DEFMETHOD("_default_mangler_options", function(options){ return defaults(options, { From 1b703349cf824020c4dc64a58aa6d0dc3b809cea Mon Sep 17 00:00:00 2001 From: Samuel Reed Date: Wed, 20 Jan 2016 11:35:45 -0600 Subject: [PATCH 15/15] Tighten up @const regex. --- lib/scope.js | 2 +- test/compress/dead-code.js | 26 +++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/lib/scope.js b/lib/scope.js index 794254d4..4cea5176 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -364,7 +364,7 @@ AST_Symbol.DEFMETHOD("global", function(){ AST_Var.DEFMETHOD("has_const_pragma", function() { var comments_before = this.start && this.start.comments_before; var lastComment = comments_before && comments_before[comments_before.length - 1]; - return lastComment && /@const/.test(lastComment.value); + return lastComment && /@const\b/.test(lastComment.value); }); AST_Toplevel.DEFMETHOD("_default_mangler_options", function(options){ diff --git a/test/compress/dead-code.js b/test/compress/dead-code.js index 8aad336c..fa4b37d6 100644 --- a/test/compress/dead-code.js +++ b/test/compress/dead-code.js @@ -138,6 +138,29 @@ dead_code_const_annotation: { } } +dead_code_const_annotation_regex: { + options = { + dead_code : true, + loops : true, + booleans : true, + conditionals : true, + evaluate : true + }; + input: { + var unused; + // @constraint this shouldn't be a constant + var CONST_FOO_ANN = false; + if (CONST_FOO_ANN) { + console.log("reachable"); + } + } + expect: { + var unused; + var CONST_FOO_ANN = !1; + CONST_FOO_ANN && console.log('reachable'); + } +} + dead_code_const_annotation_complex_scope: { options = { dead_code : true, @@ -149,7 +172,8 @@ dead_code_const_annotation_complex_scope: { input: { var unused_var; /** @const */ var test = 'test'; - /** @const */ var CONST_FOO_ANN = false; + // @const + var CONST_FOO_ANN = false; var unused_var_2; if (CONST_FOO_ANN) { console.log("unreachable");