Fix handling node types on JSDCE() in js-optimizer.js (#4695)

* Add a test case of js-optimizer for special object keys

* Check variable names and object keys strictly on JSDCE

* Move a variable definition and add comments to clarify what's done

* Add Tasuku SUENAGA to AUTHORS

AUTHORS

@@ -277,3 +277,4 @@ a license to everyone to use it as detailed in LICENSE.)
 * Joshua Lind <joshualind007@hotmail.com>
 * Hiroaki GOTO as "GORRY" <gorry@hauN.org>
 * Mikhail Kremnyov <mkremnyov@gmail.com> (copyright owned by XCDS International)
+* Tasuku SUENAGA a.k.a. gunyarakun <tasuku-s-github@titech.ac>

tests/optimizer/JSDCE-uglifyjsNodeTypes-output.js

@@ -0,0 +1,52 @@
+var defun = (function() {
+});
+var name = (function() {
+});
+var object = (function() {
+});
+var non_reserved = (function() {
+});
+function func_1() {
+}
+function func_2() {
+}
+function func_3() {
+}
+function func_4() {
+}
+function func_5() {
+}
+function func_6() {
+}
+function func_7() {
+}
+function func_8() {
+}
+function func_9() {
+}
+function func_10() {
+}
+var quotedObject = {
+ "var": func_1,
+ "defun": func_2,
+ "function": func_3,
+ "name": func_4,
+ "non_reserved": func_5
+};
+var unquotedObject = {
+ "var": func_6,
+ defun: func_7,
+ "function": func_8,
+ name: func_9,
+ non_reserved: func_10
+};
+var recursiveObject = {
+ object: {
+  func: (function() {
+  }),
+  object: {
+   func: (function() {
+   })
+  }
+ }
+};

tests/optimizer/JSDCE-uglifyjsNodeTypes.js

@@ -0,0 +1,45 @@
+var defun = function () { var a = 1; };
+var name = function () { var a = 1; };
+var object = function () { var a = 1; };
+var non_reserved = function () { var a = 1; };
+
+function func_1() { var a = 1; }
+function func_2() { var a = 1; }
+function func_3() { var a = 1; }
+function func_4() { var a = 1; }
+function func_5() { var a = 1; }
+function func_6() { var a = 1; }
+function func_7() { var a = 1; }
+function func_8() { var a = 1; }
+function func_9() { var a = 1; }
+function func_10() { var a = 1; }
+function func_deleted() { var a = 1; }
+
+var quotedObject = {
+  "var": func_1,
+  "defun": func_2,
+  "function": func_3,
+  "name": func_4,
+  "non_reserved": func_5
+};
+
+var unquotedObject = {
+  var: func_6,
+  defun: func_7,
+  function: func_8,
+  name: func_9,
+  non_reserved: func_10
+};
+
+var recursiveObject = {
+  object: {
+    func: function () {
+      var a = 1;
+    },
+    object: {
+      func: function () {
+        var b = 1;
+      }
+    }
+  }
+};

tests/test_other.py

@@ -2075,6 +2075,8 @@ int f() {
        ['splitMemory']),
       (path_from_root('tests', 'optimizer', 'JSDCE.js'), open(path_from_root('tests', 'optimizer', 'JSDCE-output.js')).read(),
        ['JSDCE']),
+      (path_from_root('tests', 'optimizer', 'JSDCE-uglifyjsNodeTypes.js'), open(path_from_root('tests', 'optimizer', 'JSDCE-uglifyjsNodeTypes-output.js')).read(),
+       ['JSDCE']),
     ]:
       print input, passes
 

tools/js-optimizer.js

@@ -7800,14 +7800,31 @@ function JSDCE(ast) {
     });
     return ast;
   }
+  var isVarNameOrObjectKeys = [];
+  // isVarNameOrObjectKeys is a stack which saves the state the node is defining a variable or in an object literal.
+  // the second argument `type` passed into the callback function called by traverse() could be a variable name or object key name.
+  // You cannot distinguish the `type` is a real type or not without isVarNameOrObjectKeys.
+  // ex.) var name = true;          // `type` can be 'name'
+  //      var obj = { defun: true } // `type` can be 'defun'
   traverse(ast, function(node, type) {
+    if (isVarNameOrObjectKeys[isVarNameOrObjectKeys.length - 1]) { // check parent node defines a variable or is an object literal
+      // `type` is a variable name or an object key name
+      isVarNameOrObjectKeys.push(false); // doesn't define a variable nor be an object literal
+      return;
+    }
     if (type === 'var') {
       node[1].forEach(function(varItem, j) {
         var name = varItem[0];
         ensureData(scopes[scopes.length-1], name).def = 1;
       });
+      isVarNameOrObjectKeys.push(true); // this `node` defines a varible
       return;
     }
+    if (type === 'object') {
+      isVarNameOrObjectKeys.push(true); // this `node` is an object literal
+      return;
+    }
+    isVarNameOrObjectKeys.push(false); // doesn't define a variable nor be an object literal
     if (type === 'defun' || type === 'function') {
       if (node[1]) ensureData(scopes[scopes.length-1], node[1]).def = 1;
       var scope = {};
@@ -7822,6 +7839,8 @@ function JSDCE(ast) {
       ensureData(scopes[scopes.length-1], node[1]).use = 1;
     }
   }, function(node, type) {
+    isVarNameOrObjectKeys.pop();
+    if (isVarNameOrObjectKeys[isVarNameOrObjectKeys.length - 1]) return; // `type` is a variable name or an object key name
     if (type === 'defun' || type === 'function') {
       var scope = scopes.pop();
       var names = set();