When validating per-scope changes skip inner change scopes instead of…

… short circuiting. Previous logic was considering changed scopes as unchanged.

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

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

@@ -4735,18 +4735,9 @@ private static void mtocHelper(Map<Node, Node> map, Node main, Node clone) {
 
   /** Checks that the scope roots marked as changed have indeed changed */
   public static void verifyScopeChanges(
-      String passName, Map<Node, Node> map, Node main) {
+      String passName, final Map<Node, Node> mtoc, Node main) {
     final String passNameMsg = passName.isEmpty() ? "" : passName + ": ";
 
-    // compiler is passed only to call compiler.toSource during debugging to see
-    // mismatches in scopes
-
-    // If verifyUnchangedNodes is false, we are comparing the initial AST to the
-    // final AST. Don't check unmarked nodes b/c they may have been changed by
-    // non-loopable passes.
-    // If verifyUnchangedNodes is true, we are comparing the ASTs before & after
-    // a pass. Check all scope roots.
-    final Map<Node, Node> mtoc = map;
     Node clone = mtoc.get(main);
     if (main.getChangeTime() > clone.getChangeTime()) {
       Preconditions.checkState(!isEquivalentToExcludingFunctions(main, clone));
@@ -4820,9 +4811,10 @@ private static boolean isEquivalentToExcludingFunctions(
     while (thisChild != null && thatChild != null) {
       if (thisChild.isFunction() || thisChild.isScript()) {
         //  don't compare function name, parameters or bodies.
-        return thatChild.getToken() == thisChild.getToken();
-      }
-      if (!isEquivalentToExcludingFunctions(thisChild, thatChild)) {
+        if (thatChild.getToken() != thisChild.getToken()) {
+          return false;
+        }
+      } else if (!isEquivalentToExcludingFunctions(thisChild, thatChild)) {
         return false;
       }
       thisChild = thisChild.getNext();

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

@@ -32,6 +32,7 @@
 import com.google.javascript.rhino.jstype.TernaryValue;
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 import junit.framework.TestCase;
 
@@ -2490,6 +2491,24 @@ public void testIsObjectDefinePropertyDefinition() {
         getCallNode("Object.defineProperty();")));
   }
 
+  public void testCorrectValidationOfScriptWithChangeAfterFunction() {
+    Node script = parse("function A() {} if (0) { A(); }");
+    Preconditions.checkState(script.isScript());
+
+    Node clone = script.cloneTree();
+    Map<Node, Node> mtoc = NodeUtil.mapMainToClone(script, clone);
+
+    // Here we make a change in that doesn't change the script node
+    // child count.
+    getCallNode(script).detach();
+
+    // Mark the script as changed
+    script.setChangeTime(100);
+
+    // will throw if no change is detected.
+    NodeUtil.verifyScopeChanges("test", mtoc, script);
+  }
+
   private boolean executedOnceTestCase(String code) {
     Node ast = parse(code);
     Node nameNode = getNameNode(ast, "x");