Make PureFunctionIdentifier compatible with destructuring assignment.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=173930812

src/com/google/javascript/jscomp/ClosureCheckModule.java

@@ -451,7 +451,7 @@ private void checkShortGoogRequireCall(NodeTraversal t, Node callNode, Node decl
       checkShortName(t, lhs, callNode.getLastChild().getString());
     }
     currentModule.importsByLongRequiredName.put(extractFirstArgumentName(callNode), lhs);
-    for (Node nameNode : NodeUtil.getLhsNodesOfDeclaration(declaration)) {
+    for (Node nameNode : NodeUtil.findLhsNodesInNode(declaration)) {
       String name = nameNode.getString();
       if (!currentModule.shortImportNames.add(name)) {
          t.report(nameNode, DUPLICATE_NAME_SHORT_REQUIRE, name);

src/com/google/javascript/jscomp/ClosureRewriteModule.java

@@ -1058,7 +1058,7 @@ private void recordTopLevelClassOrFunctionName(Node classOrFunctionNode) {
   }
 
   private void recordTopLevelVarNames(Node varNode) {
-    for (Node lhs : NodeUtil.getLhsNodesOfDeclaration(varNode)) {
+    for (Node lhs : NodeUtil.findLhsNodesInNode(varNode)) {
       String name = lhs.getString();
       currentScript.topLevelNames.add(name);
     }

src/com/google/javascript/jscomp/CoalesceVariableNames.java

@@ -284,7 +284,7 @@ private UndiGraph<Var, Void> computeVariableNamesInterferenceGraph(
       // incorrect semantics. See test case "testCapture".
       if (v.isLet() || v.isConst()) {
         Node nameDecl = NodeUtil.getEnclosingNode(v.getNode(), NodeUtil.isNameDeclaration);
-        if (NodeUtil.getLhsNodesOfDeclaration(nameDecl).size() > 1) {
+        if (NodeUtil.findLhsNodesInNode(nameDecl).size() > 1) {
           continue;
         }
       }

src/com/google/javascript/jscomp/ConvertToTypedInterface.java

@@ -335,7 +335,7 @@ void recordNameDeclaration(NodeTraversal t, Node decl) {
       checkArgument(NodeUtil.isNameDeclaration(decl));
       Node rhs = decl.getFirstChild().getLastChild();
       boolean isImport = isImportRhs(rhs);
-      for (Node name : NodeUtil.getLhsNodesOfDeclaration(decl)) {
+      for (Node name : NodeUtil.findLhsNodesInNode(decl)) {
         if (isImport) {
           currentFile.recordImport(name.getString());
         } else {

src/com/google/javascript/jscomp/Es6SyntacticScopeCreator.java

@@ -186,7 +186,7 @@ void populate() {
     }
 
     private void declareLHS(Scope s, Node n) {
-      for (Node lhs : NodeUtil.getLhsNodesOfDeclaration(n)) {
+      for (Node lhs : NodeUtil.findLhsNodesInNode(n)) {
         declareVar(s, lhs);
       }
     }

src/com/google/javascript/jscomp/LiveVariablesAnalysisEs6.java

@@ -286,7 +286,7 @@ private void computeGenKill(Node n, BitSet gen, BitSet kill, boolean conditional
                }
              }
            } else {
-             Iterable<Node> allVars = NodeUtil.getLhsNodesOfDeclaration(n);
+             Iterable<Node> allVars = NodeUtil.findLhsNodesInNode(n);
              for (Node child : allVars) {
                addToSetIfLocal(child, kill);
              }

src/com/google/javascript/jscomp/NameAnalyzer.java

@@ -551,7 +551,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
         checkNotNull(ns, "createNameInformation returned null for: %s", targetObject);
         recordDepScope(n, ns);
       } else if (n.isDestructuringLhs()) {
-        for (Node child : NodeUtil.getLhsNodesOfDeclaration(n)) {
+        for (Node child : NodeUtil.findLhsNodesInNode(n)) {
           if (!child.getParent().isComputedProp()) {
             checkState(child.isName());
             NameInformation ns = createNameInformation(t, child);
@@ -677,7 +677,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
           checkNotNull(ns, "createNameInformation returned null for: %s", n);
           recordSet(ns.name, n);
         } else if (NodeUtil.isNameDeclaration(parent) && n.isDestructuringLhs()){
-          for (Node child : NodeUtil.getLhsNodesOfDeclaration(n)) {
+          for (Node child : NodeUtil.findLhsNodesInNode(n)) {
             if (!child.getParent().isComputedProp()) {
               checkState(child.isName());
               NameInformation ns = createNameInformation(t, child);

src/com/google/javascript/jscomp/NodeUtil.java

@@ -3875,6 +3875,7 @@ private static void getLhsNodesHelper(Node n, List<Node> lhsNodes) {
       case DEFAULT_VALUE:
       case CATCH:
       case REST:
+      case ASSIGN:
         getLhsNodesHelper(n.getFirstChild(), lhsNodes);
         return;
       case IMPORT_SPEC:
@@ -3898,11 +3899,12 @@ private static void getLhsNodesHelper(Node n, List<Node> lhsNodes) {
     }
   }
 
-  /** Retrieves lhs nodes declared in the current declaration. */
-  static List<Node> getLhsNodesOfDeclaration(Node declNode) {
+  /** Retrieves lhs nodes declared in the current declaration or ASSIGN statement. */
+  static List<Node> findLhsNodesInNode(Node declNode) {
     checkArgument(
         isNameDeclaration(declNode)
             || declNode.isParamList()
+            || declNode.isAssign()
             || declNode.isCatch()
             || declNode.isDestructuringLhs()
             || declNode.isDefaultValue()

src/com/google/javascript/jscomp/Normalize.java

@@ -503,7 +503,7 @@ private void splitExportDeclaration(Node n) {
           names = Collections.singleton(c.getFirstChild());
           n.getParent().addChildBefore(c, n);
         } else {
-          names = NodeUtil.getLhsNodesOfDeclaration(c);
+          names = NodeUtil.findLhsNodesInNode(c);
           // Split up var declarations onto separate lines.
           for (Node child : c.children()) {
             c.removeChild(child);

src/com/google/javascript/jscomp/PureFunctionIdentifier.java

@@ -22,7 +22,6 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Preconditions;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableList;
@@ -61,7 +60,7 @@
  * <p>TODO: This pass could be greatly improved by proper tracking of locals within function bodies.
  * Every instance of the call to {@link NodeUtil#evaluatesToLocalValue(Node)} and {@link
  * NodeUtil#allArgsUnescapedLocal(Node)} do not actually take into account local variables. They
- * only assume literals, primatives, and operations on primatives are local.
+ * only assume literals, primitives, and operations on primitives are local.
  *
  * @author johnlenz@google.com (John Lenz)
  * @author tdeegan@google.com (Thomas Deegan)
@@ -283,7 +282,7 @@ private List<FunctionInformation> getSideEffectsForCall(Node call) {
   /**
    * When propagating side effects we construct a graph from every function definition A to every
    * function definition B that calls A(). Since the definition provider cannot always provide a
-   * unique defintion for a name, there may be many possible definitions for a given call site. In
+   * unique definition for a name, there may be many possible definitions for a given call site. In
    * the case where multiple defs share the same node in the graph.
    *
    * <p>We need to build the map {@link PureFunctionIdentifier#functionInfoByName} to get a
@@ -544,7 +543,7 @@ public void exitScope(NodeTraversal t) {
 
       // Handle deferred local variable modifications:
       for (FunctionInformation sideEffectInfo : functionSideEffectMap.get(function)) {
-        Preconditions.checkNotNull(sideEffectInfo, "%s has no side effect info.", function);
+        checkNotNull(sideEffectInfo, "%s has no side effect info.", function);
 
         if (sideEffectInfo.mutatesGlobalState()) {
           continue;
@@ -609,26 +608,7 @@ private boolean isVarDeclaredInScope(Var v, Scope scope) {
     private void visitAssignmentOrUnaryOperator(
         FunctionInformation sideEffectInfo, Scope scope, Node op, Node enclosingFunction) {
       Node lhs = op.getFirstChild();
-      Preconditions.checkState(
-          lhs.isName() || NodeUtil.isGet(lhs), "Unexpected LHS expression:", lhs);
-      if (lhs.isName()) {
-        Var var = scope.getVar(lhs.getString());
-        if (isVarDeclaredInScope(var, scope)) {
-          // Assignment to local, if the value isn't a safe local value,
-          // a literal or new object creation, add it to the local blacklist.
-          // parameter values depend on the caller.
-
-          // Note: other ops result in the name or prop being assigned a local
-          // value (x++ results in a number, for instance)
-          checkState(NodeUtil.isAssignmentOp(op) || isIncDec(op) || op.isDelProp());
-          Node rhs = op.getLastChild();
-          if (rhs != null && op.isAssign() && !NodeUtil.evaluatesToLocalValue(rhs)) {
-            blacklistedVarsByFunction.put(enclosingFunction, var);
-          }
-        } else {
-          sideEffectInfo.setTaintsGlobalState();
-        }
-      } else if (NodeUtil.isGet(lhs)) { // a['elem'] or a.elem
+      if (NodeUtil.isGet(lhs)) { // a['elem'] or a.elem
         if (lhs.getFirstChild().isThis()) {
           sideEffectInfo.setTaintsThis();
         } else {
@@ -647,6 +627,31 @@ private void visitAssignmentOrUnaryOperator(
             sideEffectInfo.setTaintsGlobalState();
           }
         }
+      } else {
+        Iterable<Node> names;
+        if (lhs.isName()) {
+          names = ImmutableList.of(lhs);
+        } else {
+          names = NodeUtil.findLhsNodesInNode(op);
+        }
+        for (Node name : names) {
+          Var var = scope.getVar(name.getString());
+          if (isVarDeclaredInScope(var, scope)) {
+            // Assignment to local, if the value isn't a safe local value,
+            // a literal or new object creation, add it to the local blacklist.
+            // parameter values depend on the caller.
+
+            // Note: other ops result in the name or prop being assigned a local
+            // value (x++ results in a number, for instance)
+            checkState(NodeUtil.isAssignmentOp(op) || isIncDec(op) || op.isDelProp());
+            Node rhs = NodeUtil.getRValueOfLValue(name);
+            if (rhs != null && op.isAssign() && !NodeUtil.evaluatesToLocalValue(rhs)) {
+              blacklistedVarsByFunction.put(enclosingFunction, var);
+            }
+          } else {
+            sideEffectInfo.setTaintsGlobalState();
+          }
+        }
       }
     }
 

src/com/google/javascript/jscomp/RemoveUnusedVars.java

@@ -408,7 +408,7 @@ private void traverseNode(Node n, Node parent, Scope scope) {
               && var.equals(scope.getArgumentsVar())) {
             Scope fnScope = var.getScope();
             Node paramList = NodeUtil.getFunctionParameters(fnScope.getRootNode());
-            for (Node p : NodeUtil.getLhsNodesOfDeclaration(paramList)) {
+            for (Node p : NodeUtil.findLhsNodesInNode(paramList)) {
               Var paramVar = fnScope.getOwnSlot(p.getString());
               checkNotNull(paramVar);
               markReferencedVar(paramVar);

test/com/google/javascript/jscomp/NodeUtilTest.java

@@ -2802,17 +2802,17 @@ public void testGetDeclaredTypeExpression4() {
     assertThat(typeExpr.getRoot().getFirstChild().getString()).isEqualTo("number");
   }
 
-  public void testGetLhsNodesOfDeclaration() {
-    assertThat(getLhsNodesOfDeclaration("var x;")).hasSize(1);
-    assertThat(getLhsNodesOfDeclaration("var x, y;")).hasSize(2);
-    assertThat(getLhsNodesOfDeclaration("var f = function(x, y, z) {};")).hasSize(1);
-    assertThat(getLhsNodesOfDeclaration("var [x=a => a, y = b=>b+1] = arr;")).hasSize(2);
-    assertThat(getLhsNodesOfDeclaration("var [x=a => a, y = b=>b+1, ...z] = arr;")).hasSize(3);
-    assertThat(getLhsNodesOfDeclaration("var [ , , , y = b=>b+1, ...z] = arr;")).hasSize(2);
-    assertThat(getLhsNodesOfDeclaration("var {x = a=>a, y = b=>b+1} = obj;")).hasSize(2);
-    assertThat(getLhsNodesOfDeclaration("var {p1: x = a=>a, p2: y = b=>b+1} = obj;")).hasSize(2);
-    assertThat(getLhsNodesOfDeclaration("var {[pname]: x = a=>a, [p2name]: y} = obj;")).hasSize(2);
-    assertThat(getLhsNodesOfDeclaration("var {lhs1 = a, p2: [lhs2, lhs3 = b] = [notlhs]} = obj;"))
+  public void testFindLhsNodesInNode() {
+    assertThat(findLhsNodesInNode("var x;")).hasSize(1);
+    assertThat(findLhsNodesInNode("var x, y;")).hasSize(2);
+    assertThat(findLhsNodesInNode("var f = function(x, y, z) {};")).hasSize(1);
+    assertThat(findLhsNodesInNode("var [x=a => a, y = b=>b+1] = arr;")).hasSize(2);
+    assertThat(findLhsNodesInNode("var [x=a => a, y = b=>b+1, ...z] = arr;")).hasSize(3);
+    assertThat(findLhsNodesInNode("var [ , , , y = b=>b+1, ...z] = arr;")).hasSize(2);
+    assertThat(findLhsNodesInNode("var {x = a=>a, y = b=>b+1} = obj;")).hasSize(2);
+    assertThat(findLhsNodesInNode("var {p1: x = a=>a, p2: y = b=>b+1} = obj;")).hasSize(2);
+    assertThat(findLhsNodesInNode("var {[pname]: x = a=>a, [p2name]: y} = obj;")).hasSize(2);
+    assertThat(findLhsNodesInNode("var {lhs1 = a, p2: [lhs2, lhs3 = b] = [notlhs]} = obj;"))
         .hasSize(3);
   }
 
@@ -2967,9 +2967,9 @@ private static Node getClassNode(String js) {
     return getClassNode(root);
   }
 
-  private static Iterable<Node> getLhsNodesOfDeclaration(String js) {
+  private static Iterable<Node> findLhsNodesInNode(String js) {
     Node root = parse(js);
-    return NodeUtil.getLhsNodesOfDeclaration(root.getFirstChild());
+    return NodeUtil.findLhsNodesInNode(root.getFirstChild());
   }
 
   private static Node getClassNode(Node n) {