Fixes change tracking in more passes and reenables tests.

------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=153763813
google · Apr 21, 2017 · cfad150 · cfad150
1 parent 499c4ee
commit cfad150
Show file tree

Hide file tree

Showing 18 changed files with 54 additions and 57 deletions.
diff --git a/src/com/google/javascript/jscomp/AggressiveInlineAliases.java b/src/com/google/javascript/jscomp/AggressiveInlineAliases.java
@@ -53,8 +53,7 @@ class AggressiveInlineAliases implements CompilerPass {
    * @param depth The chain depth.
    * @param newNodes Expression nodes that have been updated.
    */
-  private void rewriteAliasProps(
-      Name name, Node value, int depth, Set<AstChange> newNodes) {
+  private void rewriteAliasProps(Name name, Node value, int depth, Set<AstChange> newNodes) {
     if (name.props == null) {
       return;
     }

diff --git a/src/com/google/javascript/jscomp/CrossModuleMethodMotion.java b/src/com/google/javascript/jscomp/CrossModuleMethodMotion.java
@@ -21,7 +21,6 @@
 import com.google.javascript.jscomp.AnalyzePrototypeProperties.Symbol;
 import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.Node;
-
 import java.io.Serializable;
 import java.util.Collection;
 import java.util.Iterator;
@@ -161,6 +160,16 @@ private void moveMethods(Collection<NameInfo> allNameInfo) {
           }
 
           Node valueParent = value.getParent();
+          /**
+           * The logic here moves methods from some starting script node to some other script node.
+           * Both scripts need to be marked as changed. Locally the removal point in the starting
+           * script node is called 'valueParent' and the insertion point in the destination script
+           * is sometimes called 'unstubParent' and sometimes 'destParent'. The change on
+           * 'valueParent' is being reported before the change occurs since the change is guaranteed
+           * to occur and since after the change the 'valueParent' node has sometimes already been
+           * detached.
+           */
+          compiler.reportChangeToEnclosingScope(valueParent);
           Node proto = prop.getPrototype();
           int stubId = idGenerator.newId();
 
@@ -222,8 +231,9 @@ private void moveMethods(Collection<NameInfo> allNameInfo) {
         .hasGeneratedAnyIds()) {
       // Declare stub functions in the top-most module.
       Node declarations = compiler.parseSyntheticCode(STUB_DECLARATIONS);
-      compiler.getNodeForCodeInsertion(null).addChildrenToFront(
-          declarations.removeChildren());
+      Node firstScript = compiler.getNodeForCodeInsertion(null);
+      firstScript.addChildrenToFront(declarations.removeChildren());
+      compiler.reportChangeToEnclosingScope(firstScript);
     }
   }
 

diff --git a/src/com/google/javascript/jscomp/ExportTestFunctions.java b/src/com/google/javascript/jscomp/ExportTestFunctions.java
@@ -19,7 +19,6 @@
 import com.google.javascript.jscomp.parsing.parser.util.format.SimpleFormat;
 import com.google.javascript.rhino.IR;
 import com.google.javascript.rhino.Node;
-
 import java.util.regex.Pattern;
 
 /**
@@ -96,7 +95,7 @@ && isCallTargetQName(n.getParent(), "goog.testing.testSuite")) {
         for (Node c : n.children()) {
           if (c.isStringKey() && !c.isQuotedString()) {
             c.setQuotedString();
-            compiler.reportCodeChange();
+            compiler.reportChangeToEnclosingScope(c);
           } else if (c.isMemberFunctionDef()) {
             rewriteMemberDefInObjLit(c, n);
           }
@@ -130,7 +129,7 @@ private void exportClass(Node scriptNode, Node classNode) {
             Node expression = IR.exprResult(call);
 
             scriptNode.addChildAfter(expression, classNode);
-            compiler.reportCodeChange();
+            compiler.reportChangeToEnclosingScope(expression);
           }
         }
       }
@@ -141,7 +140,7 @@ private void rewriteMemberDefInObjLit(Node memberDef, Node objLit) {
       Node stringKey = IR.stringKey(name, memberDef.getFirstChild().detach());
       objLit.replaceChild(memberDef, stringKey);
       stringKey.setQuotedString();
-      compiler.reportCodeChange();
+      compiler.reportChangeToEnclosingScope(objLit);
     }
 
     // TODO(johnlenz): move test suite declaration into the
@@ -192,7 +191,7 @@ private void exportTestFunctionAsSymbol(String testFunctionName, Node node,
     Node expression = IR.exprResult(call);
 
     scriptNode.addChildAfter(expression, node);
-    compiler.reportCodeChange();
+    compiler.reportChangeToEnclosingScope(expression);
   }
 
 
@@ -216,7 +215,7 @@ private void exportTestFunctionAsProperty(String fullyQualifiedFunctionName,
     exportCall.useSourceInfoFromForTree(scriptNode);
 
     scriptNode.addChildrenAfter(exportCall, parent);
-    compiler.reportCodeChange();
+    compiler.reportChangeToEnclosingScope(exportCall);
   }
 
 

diff --git a/src/com/google/javascript/jscomp/ExtractPrototypeMemberDeclarations.java b/src/com/google/javascript/jscomp/ExtractPrototypeMemberDeclarations.java
@@ -147,7 +147,6 @@ public void process(Node externs, Node root) {
    * through all ExtractInstance and performs extraction there.
    */
   private void doExtraction(GatherExtractionInfo info) {
-
     // Insert a global temp if we are using the USE_GLOBAL_TEMP pattern.
     if (pattern == Pattern.USE_GLOBAL_TEMP) {
       Node injectionPoint = compiler.getNodeForCodeInsertion(null);
@@ -156,6 +155,7 @@ private void doExtraction(GatherExtractionInfo info) {
           .useSourceInfoIfMissingFromForTree(injectionPoint);
 
       injectionPoint.addChildToFront(var);
+      compiler.reportChangeToEnclosingScope(var);
     }
     // Go through all extraction instances and extract each of them.
     for (ExtractionInstance instance : info.instances) {
@@ -186,6 +186,7 @@ private void extractInstance(ExtractionInstance instance) {
               .useSourceInfoIfMissingFromForTree(first.node);
 
       instance.parent.addChildBefore(stmt, first.node);
+      compiler.reportChangeToEnclosingScope(stmt);
     } else if (pattern == Pattern.USE_IIFE){
       Node block = IR.block();
       Node func = IR.function(
@@ -202,7 +203,9 @@ private void extractInstance(ExtractionInstance instance) {
       Node stmt = new Node(first.node.getToken(), call);
       stmt.useSourceInfoIfMissingFromForTree(first.node);
       instance.parent.addChildBefore(stmt, first.node);
+      compiler.reportChangeToEnclosingScope(stmt);
       for (PrototypeMemberDeclaration declar : instance.declarations) {
+        compiler.reportChangeToEnclosingScope(declar.node);
         block.addChildToBack(declar.node.detach());
       }
     }
@@ -238,6 +241,7 @@ private void replacePrototypeMemberDeclaration(
     name.getFirstChild().setOriginalName(className + ".prototype");
 
     assignment.replaceChild(lhs, name);
+    compiler.reportChangeToEnclosingScope(name);
   }
 
   /**

diff --git a/src/com/google/javascript/jscomp/MarkNoSideEffectCalls.java b/src/com/google/javascript/jscomp/MarkNoSideEffectCalls.java
@@ -181,8 +181,10 @@ public void visit(NodeTraversal traversal, Node node, Node parent) {
       }
 
       if (maybeFunction) {
-        node.setSideEffectFlags(Node.NO_SIDE_EFFECTS);
-        compiler.reportChangeToEnclosingScope(node);
+        if (node.getSideEffectFlags() != Node.NO_SIDE_EFFECTS) {
+          node.setSideEffectFlags(Node.NO_SIDE_EFFECTS);
+          compiler.reportChangeToEnclosingScope(node);
+        }
       }
     }
   }

diff --git a/src/com/google/javascript/jscomp/NameAnonymousFunctions.java b/src/com/google/javascript/jscomp/NameAnonymousFunctions.java
@@ -88,7 +88,7 @@ public final void setFunctionName(String name, Node fnNode) {
       Node fnNameNode = fnNode.getFirstChild();
       String uniqueName = getLikelyNonConflictingName(name);
       fnNameNode.setString(uniqueName);
-      compiler.reportCodeChange();
+      compiler.reportChangeToEnclosingScope(fnNameNode);
       namedCount++;
       bytesUsed += uniqueName.length();
     }

diff --git a/src/com/google/javascript/jscomp/Normalize.java b/src/com/google/javascript/jscomp/Normalize.java
@@ -89,12 +89,12 @@ static Node parseAndNormalizeTestCode(
     return js;
   }
 
-  private void reportCodeChange(String changeDescription) {
+  private void reportCodeChange(String changeDescription, Node n) {
     if (assertOnChange) {
       throw new IllegalStateException(
           "Normalize constraints violated:\n" + changeDescription);
     }
-    compiler.reportCodeChange();
+    compiler.reportChangeToEnclosingScope(n);
   }
 
   @Override
@@ -158,20 +158,21 @@ private class RewriteExposedProperties
       this.exposedProperties = exposedProperties;
     }
 
-    @Override public void visit(NodeTraversal t, Node n, Node parent) {
+    @Override
+    public void visit(NodeTraversal t, Node n, Node parent) {
       if (n.isGetProp()) {
         String propName = n.getLastChild().getString();
         if (exposedProperties.contains(propName)) {
           Node obj = n.removeFirstChild();
           Node prop = n.removeFirstChild();
+          compiler.reportChangeToEnclosingScope(n);
           n.replaceWith(IR.getelem(obj, prop));
-          compiler.reportCodeChange();
         }
       } else if (n.isStringKey()) {
         String propName = n.getString();
         if (exposedProperties.contains(propName)) {
           n.setQuotedString();
-          compiler.reportCodeChange();
+          compiler.reportChangeToEnclosingScope(n);
         }
       }
     }
@@ -669,7 +670,7 @@ private void normalizeAssignShorthand(Node shorthand) {
         assign.setJSDocInfo(shorthand.getJSDocInfo());
         shorthand.setJSDocInfo(null);
         parent.replaceChild(insertPoint, assign);
-        compiler.reportCodeChange();
+        compiler.reportChangeToEnclosingScope(assign);
       }
     }
 
@@ -770,7 +771,9 @@ private void replaceVarWithAssignment(Node n, Node parent, Node grandparent) {
         Node replacement = IR.assign(n, value);
         replacement.setJSDocInfo(parent.getJSDocInfo());
         replacement.useSourceInfoIfMissingFrom(parent);
-        grandparent.replaceChild(parent, NodeUtil.newExpr(replacement));
+        Node statement = NodeUtil.newExpr(replacement);
+        grandparent.replaceChild(parent, statement);
+        reportCodeChange("Duplicate VAR declaration", statement);
       } else {
         // It is an empty reference remove it.
         if (NodeUtil.isStatementBlock(grandparent)) {
@@ -787,8 +790,8 @@ private void replaceVarWithAssignment(Node n, Node parent, Node grandparent) {
           // already have been normalized to have a BLOCK.
           throw new IllegalStateException("Unexpected LABEL");
         }
+        reportCodeChange("Duplicate VAR declaration", grandparent);
       }
-      reportCodeChange("Duplicate VAR declaration");
     }
   }
 

diff --git a/src/com/google/javascript/jscomp/OptimizeParameters.java b/src/com/google/javascript/jscomp/OptimizeParameters.java
@@ -485,7 +485,7 @@ private void addVariableToFunction(Node function, Node varName, Node value) {
       stmt = IR.exprResult(value).useSourceInfoFrom(value);
     }
     block.addChildToFront(stmt);
-    compiler.reportCodeChange();
+    compiler.reportChangeToEnclosingScope(stmt);
   }
 
   /**
@@ -506,29 +506,32 @@ private boolean eliminateParamsAfter(Node fnNode, Node argNode) {
     if (argNode != null) {
       // Keep the args in the same order, do the last first.
       eliminateParamsAfter(fnNode, argNode.getNext());
+      compiler.reportChangeToEnclosingScope(argNode);
       argNode.detach();
       Node var = IR.var(argNode).useSourceInfoIfMissingFrom(argNode);
       fnNode.getLastChild().addChildToFront(var);
-      compiler.reportCodeChange();
+      compiler.reportChangeToEnclosingScope(var);
       return true;
     }
     return false;
   }
 
   /**
    * Eliminates the parameter from a function definition.
+   *
    * @param function The function node
    * @param argIndex The index of the the argument to remove.
    * @return The Node of the argument removed.
    */
-  private static Node eliminateFunctionParamAt(Node function, int argIndex) {
+  private Node eliminateFunctionParamAt(Node function, int argIndex) {
     Preconditions.checkArgument(function.isFunction(),
         "Node must be a function.");
 
     Node formalArgPtr = NodeUtil.getArgumentForFunction(
         function, argIndex);
 
     if (formalArgPtr != null) {
+      compiler.reportChangeToEnclosingScope(formalArgPtr);
       function.getSecondChild().removeChild(formalArgPtr);
     }
     return formalArgPtr;
@@ -552,13 +555,13 @@ private Node eliminateCallParamAt(
 
     if (formalArgPtr != null) {
       call.removeChild(formalArgPtr);
+      compiler.reportChangeToEnclosingScope(call);
       // The value in the parameter object is the one that is being moved into
       // function definition leave that one's references.  For everything else,
       // remove any references.
       if (p.getArg() != formalArgPtr) {
         removedNodes.add(formalArgPtr);
       }
-      compiler.reportCodeChange();
     }
     return formalArgPtr;
   }

diff --git a/src/com/google/javascript/jscomp/PolymerClassRewriter.java b/src/com/google/javascript/jscomp/PolymerClassRewriter.java
@@ -138,6 +138,7 @@ void rewritePolymerClass(
         parent.addChildrenAfter(statements, beforeRoot);
       }
     }
+    compiler.reportChangeToEnclosingScope(statements);
 
     // Since behavior files might contain more language features than the class file, we need to
     // update the feature sets.
@@ -146,14 +147,14 @@ void rewritePolymerClass(
       Node scriptNode = NodeUtil.getEnclosingScript(parent);
       FeatureSet oldFeatures = (FeatureSet) scriptNode.getProp(Node.FEATURE_SET);
       scriptNode.putProp(Node.FEATURE_SET, oldFeatures.union(newFeatures));
+      compiler.reportChangeToEnclosingScope(scriptNode);
     }
 
     if (NodeUtil.isNameDeclaration(exprRoot)) {
       Node assignExpr = varToAssign(exprRoot);
       parent.replaceChild(exprRoot, assignExpr);
+      compiler.reportChangeToEnclosingScope(assignExpr);
     }
-
-    compiler.reportCodeChange();
   }
 
   /**
@@ -402,7 +403,7 @@ private void addInterfaceExterns(
     Node stmts = block.removeChildren();
     parent.addChildrenToBack(stmts);
 
-    compiler.reportCodeChange();
+    compiler.reportChangeToEnclosingScope(stmts);
   }
 
   private static boolean hasShorthandAssignment(Node objLit) {

diff --git a/src/com/google/javascript/jscomp/PolymerPass.java b/src/com/google/javascript/jscomp/PolymerPass.java
@@ -29,7 +29,6 @@
 import com.google.javascript.rhino.JSTypeExpression;
 import com.google.javascript.rhino.Node;
 import com.google.javascript.rhino.Token;
-
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
@@ -165,7 +164,7 @@ private void appendPolymerElementExterns(final PolymerClassDefinition def) {
     Node stmts = block.removeChildren();
     parent.addChildrenAfter(stmts, polymerElementExterns);
 
-    compiler.reportCodeChange();
+    compiler.reportChangeToEnclosingScope(stmts);
   }
 
   /**

diff --git a/src/com/google/javascript/jscomp/PolymerPassStaticUtils.java b/src/com/google/javascript/jscomp/PolymerPassStaticUtils.java
@@ -50,7 +50,7 @@ public void visit(Node n) {
                     && n.getGrandparent().isGetProp()) {
                   Node dollarChildProp = n.getGrandparent();
                   dollarChildProp.setToken(Token.GETELEM);
-                  compiler.reportCodeChange();
+                  compiler.reportChangeToEnclosingScope(dollarChildProp);
                 }
               }
             },

diff --git a/test/com/google/javascript/jscomp/CrossModuleMethodMotionTest.java b/test/com/google/javascript/jscomp/CrossModuleMethodMotionTest.java
@@ -45,7 +45,6 @@ protected CompilerPass getProcessor(Compiler compiler) {
   public void setUp() {
     canMoveExterns = false;
     noStubs = false;
-    validateAstChangeMarking(false);
   }
 
   public void testMovePrototypeMethod1() {

diff --git a/test/com/google/javascript/jscomp/ExportTestFunctionsTest.java b/test/com/google/javascript/jscomp/ExportTestFunctionsTest.java
@@ -39,7 +39,6 @@ public ExportTestFunctionsTest() {
   @Override
   public void setUp() {
     super.enableLineNumberCheck(false);
-    validateAstChangeMarking(false);
   }
 
   @Override

diff --git a/test/com/google/javascript/jscomp/FlowSensitiveInlineVariablesTest.java b/test/com/google/javascript/jscomp/FlowSensitiveInlineVariablesTest.java
@@ -33,7 +33,6 @@ public final class FlowSensitiveInlineVariablesTest extends CompilerTestCase  {
   @Override
   public void setUp() {
     enableNormalize();
-    validateAstChangeMarking(false);
   }
 
   @Override

diff --git a/test/com/google/javascript/jscomp/NameAnonymousFunctionsTest.java b/test/com/google/javascript/jscomp/NameAnonymousFunctionsTest.java
@@ -29,12 +29,7 @@ public NameAnonymousFunctionsTest() {
   }
 
   @Override
-  protected void setUp() throws Exception {
-    super.setUp();
-    validateAstChangeMarking(false);
-  }
-
-  @Override public CompilerPass getProcessor(Compiler compiler) {
+  public CompilerPass getProcessor(Compiler compiler) {
     return new NameAnonymousFunctions(compiler);
   }