From afecc106ff2b7682cd3cb35c8c4b4f50d032552b Mon Sep 17 00:00:00 2001
From: Chad Killingsworth <chadkillingsworth@gmail.com>
Date: Wed, 2 Aug 2017 11:30:03 -0700
Subject: [PATCH] Because ProcessCommonJSModules has a post-order traversal, we
 have to split var declarations and then re-visit all the new nodes.

Closes https://github.com/google/closure-compiler/pull/2596

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164013228
---
 .../jscomp/ProcessCommonJSModules.java        | 33 ++++++++++++++-----
 .../jscomp/ProcessCommonJSModulesTest.java    | 21 ++++++++++++
 2 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/src/com/google/javascript/jscomp/ProcessCommonJSModules.java b/src/com/google/javascript/jscomp/ProcessCommonJSModules.java
index beea0c6a414..9bf82dd5aa1 100644
--- a/src/com/google/javascript/jscomp/ProcessCommonJSModules.java
+++ b/src/com/google/javascript/jscomp/ProcessCommonJSModules.java
@@ -694,8 +694,28 @@ public void visit(NodeTraversal t, Node n, Node parent) {
           }
           break;
 
+        case VAR:
+        case LET:
+        case CONST:
+          // Multiple declarations need split apart so that they can be refactored into
+          // property assignments or removed altogether.
+          if (n.hasMoreThanOneChild()) {
+            List<Node> vars = splitMultipleDeclarations(n);
+            t.reportCodeChange();
+            for (Node var : vars) {
+              visit(t, var.getFirstChild(), var);
+            }
+          }
+          break;
+
         case NAME:
           {
+            // If this is a name declaration with multiple names, it will be split apart when
+            // the parent is visited and then revisit the children.
+            if (NodeUtil.isNameDeclaration(n.getParent()) && n.getParent().hasMoreThanOneChild()) {
+              break;
+            }
+
             String qName = n.getQualifiedName();
             if (qName == null) {
               break;
@@ -1071,13 +1091,6 @@ private void updateNameReference(
         case VAR:
         case LET:
         case CONST:
-          // Multiple declaration - needs split apart.
-          if (parent.getChildCount() > 1) {
-            splitMultipleDeclarations(parent);
-            parent = nameRef.getParent();
-            newNameDeclaration = t.getScope().getVar(newName);
-          }
-
           if (newNameIsQualified) {
             // Refactor a var declaration to a getprop assignment
             Node getProp = NodeUtil.newQName(compiler, newName, nameRef, originalName);
@@ -1407,13 +1420,17 @@ private void fixTypeNode(NodeTraversal t, Node typeNode) {
       }
     }
 
-    private void splitMultipleDeclarations(Node var) {
+    private List<Node> splitMultipleDeclarations(Node var) {
       checkState(NodeUtil.isNameDeclaration(var));
+      List<Node> vars = new ArrayList<>();
       while (var.getSecondChild() != null) {
         Node newVar = new Node(var.getToken(), var.removeFirstChild());
         newVar.useSourceInfoFrom(var);
         var.getParent().addChildBefore(newVar, var);
+        vars.add(newVar);
       }
+      vars.add(var);
+      return vars;
     }
   }
 }
diff --git a/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java b/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java
index 12af2e13fc9..a5ddd25c2a0 100644
--- a/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java
+++ b/test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java
@@ -907,4 +907,25 @@ public void testWebpackAmdPattern() {
             "        .apply(module$test,__WEBPACK_AMD_DEFINE_ARRAY__$$module$test),",
             "    module$test!==undefined && module$test)"));
   }
+
+  public void testIssue2593() {
+    testModules(
+        "test.js",
+        LINE_JOINER.join(
+            "var first = 1,",
+            "    second = 2,",
+            "    third = 3,",
+            "    fourth = 4,",
+            "    fifth = 5;",
+            "",
+            "module.exports = {};"),
+        LINE_JOINER.join(
+            "goog.provide('module$test');",
+            "/** @const */ var module$test={};",
+            "var first$$module$test=1;",
+            "var second$$module$test=2;",
+            "var third$$module$test=3;",
+            "var fourth$$module$test=4;",
+            "var fifth$$module$test=5;"));
+  }
 }