Skip to content

Commit

Permalink
Because ProcessCommonJSModules has a post-order traversal, we have to…
Browse files Browse the repository at this point in the history
… split var declarations and then re-visit all the new nodes.

Closes #2596

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164013228
  • Loading branch information
ChadKillingsworth authored and dimvar committed Aug 2, 2017
1 parent 05042a7 commit afecc10
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 8 deletions.
33 changes: 25 additions & 8 deletions src/com/google/javascript/jscomp/ProcessCommonJSModules.java
Expand Up @@ -694,8 +694,28 @@ public void visit(NodeTraversal t, Node n, Node parent) {
} }
break; 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: 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(); String qName = n.getQualifiedName();
if (qName == null) { if (qName == null) {
break; break;
Expand Down Expand Up @@ -1071,13 +1091,6 @@ private void updateNameReference(
case VAR: case VAR:
case LET: case LET:
case CONST: case CONST:
// Multiple declaration - needs split apart.
if (parent.getChildCount() > 1) {
splitMultipleDeclarations(parent);
parent = nameRef.getParent();
newNameDeclaration = t.getScope().getVar(newName);
}

if (newNameIsQualified) { if (newNameIsQualified) {
// Refactor a var declaration to a getprop assignment // Refactor a var declaration to a getprop assignment
Node getProp = NodeUtil.newQName(compiler, newName, nameRef, originalName); Node getProp = NodeUtil.newQName(compiler, newName, nameRef, originalName);
Expand Down Expand Up @@ -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)); checkState(NodeUtil.isNameDeclaration(var));
List<Node> vars = new ArrayList<>();
while (var.getSecondChild() != null) { while (var.getSecondChild() != null) {
Node newVar = new Node(var.getToken(), var.removeFirstChild()); Node newVar = new Node(var.getToken(), var.removeFirstChild());
newVar.useSourceInfoFrom(var); newVar.useSourceInfoFrom(var);
var.getParent().addChildBefore(newVar, var); var.getParent().addChildBefore(newVar, var);
vars.add(newVar);
} }
vars.add(var);
return vars;
} }
} }
} }
21 changes: 21 additions & 0 deletions test/com/google/javascript/jscomp/ProcessCommonJSModulesTest.java
Expand Up @@ -907,4 +907,25 @@ public void testWebpackAmdPattern() {
" .apply(module$test,__WEBPACK_AMD_DEFINE_ARRAY__$$module$test),", " .apply(module$test,__WEBPACK_AMD_DEFINE_ARRAY__$$module$test),",
" module$test!==undefined && 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;"));
}
} }

0 comments on commit afecc10

Please sign in to comment.