From e587f3b760aca9eae034b70b7f911b135d356c32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Thu, 16 Feb 2017 11:30:23 +0100 Subject: [PATCH 1/7] Fix: AST_Accessor missing start / end tokens - #1492 --- lib/parse.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/parse.js b/lib/parse.js index ec82d47d..718e457f 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -1320,11 +1320,17 @@ function parse($TEXT, options) { var type = start.type; var name = as_property_name(); if (type == "name" && !is("punc", ":")) { + var createAccessor = function() { + var func = function_(AST_Accessor); + func.start = start; + func.end = prev(); + return func; + }; if (name == "get") { a.push(new AST_ObjectGetter({ start : start, key : as_atom_node(), - value : function_(AST_Accessor), + value : createAccessor(), end : prev() })); continue; @@ -1333,7 +1339,7 @@ function parse($TEXT, options) { a.push(new AST_ObjectSetter({ start : start, key : as_atom_node(), - value : function_(AST_Accessor), + value : createAccessor(), end : prev() })); continue; From 84adda7c205e88db8e39417f7bb784551ee132ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Thu, 16 Feb 2017 12:21:07 +0100 Subject: [PATCH 2/7] Test case for the fix. --- test/mocha/accessorTokens-1492.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 test/mocha/accessorTokens-1492.js diff --git a/test/mocha/accessorTokens-1492.js b/test/mocha/accessorTokens-1492.js new file mode 100644 index 00000000..0fccd431 --- /dev/null +++ b/test/mocha/accessorTokens-1492.js @@ -0,0 +1,22 @@ +var UglifyJS = require('../../'); +var assert = require("assert"); + +describe("Accessor tokens", function() { + it("Should fill the token information for accessors (issue #1492)", function() { + var ast = UglifyJS.parse("var obj = { get latest() { return undefined; } }"); + + /* a possible way to test, but walking through the whole tree seems more robust against possible AST changes + var accessor = ast.body["0"].definitions["0"].value.properties["0"].value; + assert(accessor instanceof UglifyJS.AST_Accessor); + assert(accessor.start !== undefined); + assert(accessor.end !== undefined); + */ + + // test there are no nodes without tokens + var checkWalker = new UglifyJS.TreeWalker(function(node, descend) { + assert(node.start !== undefined); + assert(node.end !== undefined); + }); + ast.walk(checkWalker); + }); +}); \ No newline at end of file From 4394fe7dcf674e8a695c9c6a8dae7b614987266e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Thu, 16 Feb 2017 12:51:53 +0100 Subject: [PATCH 3/7] Improved the test case - test tokens against expected values. --- test/mocha/accessorTokens-1492.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/mocha/accessorTokens-1492.js b/test/mocha/accessorTokens-1492.js index 0fccd431..4428d76a 100644 --- a/test/mocha/accessorTokens-1492.js +++ b/test/mocha/accessorTokens-1492.js @@ -14,8 +14,10 @@ describe("Accessor tokens", function() { // test there are no nodes without tokens var checkWalker = new UglifyJS.TreeWalker(function(node, descend) { - assert(node.start !== undefined); - assert(node.end !== undefined); + if (node instanceof UglifyJS.AST_Accessor) { + assert.equal(node.start.pos, 12); + assert.equal(node.end.endpos, 46); + } }); ast.walk(checkWalker); }); From 37ea9df24fe7ada0b9bf08400d1eb29e2019c16d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Thu, 16 Feb 2017 13:55:24 +0100 Subject: [PATCH 4/7] Implement createAccessor using embed_tokens. Adjusted test to match expected result. --- lib/parse.js | 9 +++------ test/mocha/accessorTokens-1492.js | 11 +++-------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/lib/parse.js b/lib/parse.js index 718e457f..08de03f2 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -1320,12 +1320,9 @@ function parse($TEXT, options) { var type = start.type; var name = as_property_name(); if (type == "name" && !is("punc", ":")) { - var createAccessor = function() { - var func = function_(AST_Accessor); - func.start = start; - func.end = prev(); - return func; - }; + var createAccessor = embed_tokens(function() { + return function_(AST_Accessor); + }); if (name == "get") { a.push(new AST_ObjectGetter({ start : start, diff --git a/test/mocha/accessorTokens-1492.js b/test/mocha/accessorTokens-1492.js index 4428d76a..05300c31 100644 --- a/test/mocha/accessorTokens-1492.js +++ b/test/mocha/accessorTokens-1492.js @@ -3,19 +3,14 @@ var assert = require("assert"); describe("Accessor tokens", function() { it("Should fill the token information for accessors (issue #1492)", function() { + // location 0 1 2 3 4 + // 01234567890123456789012345678901234567890123456789 var ast = UglifyJS.parse("var obj = { get latest() { return undefined; } }"); - /* a possible way to test, but walking through the whole tree seems more robust against possible AST changes - var accessor = ast.body["0"].definitions["0"].value.properties["0"].value; - assert(accessor instanceof UglifyJS.AST_Accessor); - assert(accessor.start !== undefined); - assert(accessor.end !== undefined); - */ - // test there are no nodes without tokens var checkWalker = new UglifyJS.TreeWalker(function(node, descend) { if (node instanceof UglifyJS.AST_Accessor) { - assert.equal(node.start.pos, 12); + assert.equal(node.start.pos, 22); assert.equal(node.end.endpos, 46); } }); From 97e55c33729bcf0a774a4c99b4b9c391b1647ba5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Thu, 16 Feb 2017 15:39:55 +0100 Subject: [PATCH 5/7] Travis, please try again. From 452e2d37ed87825a41bf00cac4b3bb3be3ea39f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Thu, 16 Feb 2017 17:22:51 +0100 Subject: [PATCH 6/7] Testing AST_SymbolRef and AST_ObjectProperty as well. --- test/mocha/accessorTokens-1492.js | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/test/mocha/accessorTokens-1492.js b/test/mocha/accessorTokens-1492.js index 05300c31..861414ee 100644 --- a/test/mocha/accessorTokens-1492.js +++ b/test/mocha/accessorTokens-1492.js @@ -7,13 +7,26 @@ describe("Accessor tokens", function() { // 01234567890123456789012345678901234567890123456789 var ast = UglifyJS.parse("var obj = { get latest() { return undefined; } }"); - // test there are no nodes without tokens + // test all AST_ObjectProperty tokens are set as expected + var checkedAST_ObjectProperty = false; var checkWalker = new UglifyJS.TreeWalker(function(node, descend) { - if (node instanceof UglifyJS.AST_Accessor) { - assert.equal(node.start.pos, 22); + if (node instanceof UglifyJS.AST_ObjectProperty) { + checkedAST_ObjectProperty = true; + + assert.equal(node.start.pos, 12); assert.equal(node.end.endpos, 46); + + assert(node.key instanceof UglifyJS.AST_SymbolRef); + assert.equal(node.key.start.pos, 16); + assert.equal(node.key.end.endpos, 22); + + assert(node.value instanceof UglifyJS.AST_Accessor); + assert.equal(node.value.start.pos, 22); + assert.equal(node.value.end.endpos, 46); + } }); ast.walk(checkWalker); + assert(checkedAST_ObjectProperty, "AST_ObjectProperty not found"); }); }); \ No newline at end of file From 8aff2332e50acf7a49a26c99784e92e7daf339d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20=C5=A0pan=C4=9Bl?= Date: Mon, 20 Feb 2017 09:39:10 +0100 Subject: [PATCH 7/7] Refactor as required (scope, naming). --- lib/parse.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/parse.js b/lib/parse.js index 08de03f2..37f06df7 100644 --- a/lib/parse.js +++ b/lib/parse.js @@ -1308,6 +1308,10 @@ function parse($TEXT, options) { }); }); + var create_accessor = embed_tokens(function() { + return function_(AST_Accessor); + }); + var object_ = embed_tokens(function() { expect("{"); var first = true, a = []; @@ -1320,14 +1324,11 @@ function parse($TEXT, options) { var type = start.type; var name = as_property_name(); if (type == "name" && !is("punc", ":")) { - var createAccessor = embed_tokens(function() { - return function_(AST_Accessor); - }); if (name == "get") { a.push(new AST_ObjectGetter({ start : start, key : as_atom_node(), - value : createAccessor(), + value : create_accessor(), end : prev() })); continue; @@ -1336,7 +1337,7 @@ function parse($TEXT, options) { a.push(new AST_ObjectSetter({ start : start, key : as_atom_node(), - value : createAccessor(), + value : create_accessor(), end : prev() })); continue;