Make sure we never create a scope rooted at a node which can't be a s…

…cope root.

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

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

@@ -2543,14 +2543,18 @@ public boolean apply(Node n) {
     }
   };
 
+  static boolean createsScope(Node n) {
+    return createsBlockScope(n) || n.isFunction() || n.isModuleBody()
+        // The ROOT nodes that are the root of the externs tree or main JS tree do not
+        // create scopes. The parent of those two, which is the root of the entire AST and
+        // therefore has no parent, is the only ROOT node that creates a scope.
+        || (n.isRoot() && n.getParent() == null);
+  }
+
   static final Predicate<Node> createsScope = new Predicate<Node>() {
     @Override
     public boolean apply(Node n) {
-      return createsBlockScope(n) || n.isFunction() || n.isModuleBody()
-          // The ROOT nodes that are the root of the externs tree or main JS tree do not
-          // create scopes. The parent of those two, which is the root of the entire AST and
-          // therefore has no parent, is the only ROOT node that creates a scope.
-          || (n.isRoot() && n.getParent() == null);
+      return createsScope(n);
     }
   };
 

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

@@ -47,7 +47,13 @@ public class Scope implements StaticScope {
    */
   Scope(Scope parent, Node rootNode) {
     Preconditions.checkNotNull(parent);
-    Preconditions.checkNotNull(rootNode);
+    // TODO(tbreisacher): Can we tighten this to just NodeUtil.createsScope?
+    Preconditions.checkState(
+        NodeUtil.createsScope(rootNode)
+            || rootNode.isScript()
+            || rootNode.isRoot()
+            || rootNode.isNormalBlock(),
+        rootNode);
     Preconditions.checkArgument(
         rootNode != parent.rootNode, "rootNode should not be the parent's root node", rootNode);
 
@@ -57,7 +63,13 @@ public class Scope implements StaticScope {
   }
 
   protected Scope(Node rootNode) {
-    Preconditions.checkNotNull(rootNode);
+    // TODO(tbreisacher): Can we tighten this to just NodeUtil.createsScope?
+    Preconditions.checkState(
+        NodeUtil.createsScope(rootNode)
+            || rootNode.isScript()
+            || rootNode.isRoot()
+            || rootNode.isNormalBlock(),
+        rootNode);
     this.parent = null;
     this.rootNode = rootNode;
     this.depth = 0;

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

@@ -30,30 +30,30 @@
 public final class MemoizedScopeCreatorTest extends TestCase {
 
   public void testMemoization() throws Exception {
-    Node trueNode = new Node(Token.TRUE);
-    Node falseNode = new Node(Token.FALSE);
+    Node block1 = new Node(Token.BLOCK);
+    Node block2 = new Node(Token.BLOCK);
     // Wow, is there really a circular dependency between JSCompiler and
     // SyntacticScopeCreator?
     Compiler compiler = new Compiler();
     compiler.initOptions(new CompilerOptions());
     ScopeCreator creator = new MemoizedScopeCreator(
         SyntacticScopeCreator.makeTyped(compiler));
-    Scope scopeA = creator.createScope(trueNode, null);
-    assertSame(scopeA, creator.createScope(trueNode, null));
-    assertNotSame(scopeA, creator.createScope(falseNode, null));
+    Scope scopeA = creator.createScope(block1, null);
+    assertSame(scopeA, creator.createScope(block1, null));
+    assertNotSame(scopeA, creator.createScope(block2, null));
   }
 
   public void testPreconditionCheck() throws Exception {
     Compiler compiler = new Compiler();
     compiler.initOptions(new CompilerOptions());
-    Node trueNode = new Node(Token.TRUE);
+    Node block = new Node(Token.BLOCK);
     ScopeCreator creator = new MemoizedScopeCreator(
         SyntacticScopeCreator.makeTyped(compiler));
-    Scope scopeA = creator.createScope(trueNode, null);
+    Scope scopeA = creator.createScope(block, null);
 
     boolean handled = false;
     try {
-      creator.createScope(trueNode, scopeA);
+      creator.createScope(block, scopeA);
     } catch (IllegalStateException e) {
       handled = true;
     }