Update tests to stop creating some invalid scopes.

This lets tighten the Preconditions checks in Scope a little.

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

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

@@ -17,8 +17,9 @@
 package com.google.javascript.jscomp;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
 
-import com.google.common.base.Preconditions;
 import com.google.javascript.rhino.Node;
 import com.google.javascript.rhino.StaticScope;
 import java.util.Collections;
@@ -46,15 +47,9 @@ public class Scope implements StaticScope {
    * @param rootNode
    */
   Scope(Scope parent, Node rootNode) {
-    Preconditions.checkNotNull(parent);
-    // TODO(tbreisacher): Can we tighten this to just NodeUtil.createsScope?
-    Preconditions.checkState(
-        NodeUtil.createsScope(rootNode)
-            || rootNode.isScript()
-            || rootNode.isRoot()
-            || rootNode.isNormalBlock(),
-        rootNode);
-    Preconditions.checkArgument(
+    checkNotNull(parent);
+    checkArgument(NodeUtil.createsScope(rootNode), rootNode);
+    checkArgument(
         rootNode != parent.rootNode, "rootNode should not be the parent's root node", rootNode);
 
     this.parent = parent;
@@ -64,12 +59,8 @@ public class Scope implements StaticScope {
 
   protected Scope(Node rootNode) {
     // TODO(tbreisacher): Can we tighten this to just NodeUtil.createsScope?
-    Preconditions.checkState(
-        NodeUtil.createsScope(rootNode)
-            || rootNode.isScript()
-            || rootNode.isRoot()
-            || rootNode.isNormalBlock(),
-        rootNode);
+    checkArgument(
+        NodeUtil.createsScope(rootNode) || rootNode.isScript() || rootNode.isRoot(), rootNode);
     this.parent = null;
     this.rootNode = rootNode;
     this.depth = 0;
@@ -82,12 +73,7 @@ public String toString() {
 
   static Scope createGlobalScope(Node rootNode) {
     // TODO(tbreisacher): Can we tighten this to allow only ROOT nodes?
-    checkArgument(
-        rootNode.isRoot()
-            || rootNode.isScript()
-            || rootNode.isModuleBody()
-            || rootNode.isFunction(),
-        rootNode);
+    checkArgument(rootNode.isRoot() || rootNode.isScript(), rootNode);
     return new Scope(rootNode);
   }
 
@@ -130,9 +116,9 @@ public StaticScope getParentScope() {
    * @param input the input in which this variable is defined.
    */
   Var declare(String name, Node nameNode, CompilerInput input) {
-    Preconditions.checkState(name != null && !name.isEmpty());
+    checkState(name != null && !name.isEmpty());
     // Make sure that it's declared only once
-    Preconditions.checkState(vars.get(name) == null);
+    checkState(vars.get(name) == null);
     Var var = new Var(name, nameNode, this, vars.size(), input);
     vars.put(name, var);
     return var;
@@ -143,8 +129,8 @@ Var declare(String name, Node nameNode, CompilerInput input) {
    * a variable and removes it from the scope.
    */
   void undeclare(Var var) {
-    Preconditions.checkState(var.scope == this);
-    Preconditions.checkState(vars.get(var.name).equals(var));
+    checkState(var.scope == this);
+    checkState(vars.get(var.name).equals(var));
     vars.remove(var.name);
   }
 

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

@@ -29,9 +29,9 @@
 
 public final class LinkedFlowScopeTest extends CompilerTypeTestCase {
 
-  private final Node blockNode = new Node(Token.BLOCK);
+  private final Node rootNode = new Node(Token.ROOT);
   private final Node functionNode = new Node(Token.FUNCTION);
-  private final int LONG_CHAIN_LENGTH = 1050;
+  private static final int LONG_CHAIN_LENGTH = 1050;
 
   private TypedScope globalScope;
   private TypedScope localScope;
@@ -43,7 +43,7 @@ public final class LinkedFlowScopeTest extends CompilerTypeTestCase {
   public void setUp() throws Exception {
     super.setUp();
 
-    globalScope = TypedScope.createGlobalScope(blockNode);
+    globalScope = TypedScope.createGlobalScope(rootNode);
     globalScope.declare("globalA", null, null, null);
     globalScope.declare("globalB", null, null, null);
 

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

@@ -20,12 +20,10 @@
 
 import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
 import com.google.javascript.rhino.Node;
-
-import junit.framework.TestCase;
-
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import junit.framework.TestCase;
 
 /**
  * Tests for {@link MaybeReachingVariableUse}.
@@ -114,10 +112,10 @@ public void testForIn() {
   }
 
   public void testTryCatch() {
-    assertMatch(
-        "D: var x = 1; " +
-        "try { U: var y = foo() + x; } catch (e) {} " +
-        "U: var z = x;");
+    assertMatch(""
+        + "D: var x = 1; "
+        + "try { U: var y = foo() + x; } catch (e) {} "
+        + "U: var z = x;");
   }
 
   /**
@@ -143,18 +141,21 @@ private void assertNotMatch(String src) {
    */
   private void computeUseDef(String src) {
     Compiler compiler = new Compiler();
+    SyntacticScopeCreator scopeCreator = SyntacticScopeCreator.makeUntyped(compiler);
     src = "function _FUNCTION(param1, param2){" + src + "}";
-    Node n = compiler.parseTestCode(src).getFirstChild();
+    Node script = compiler.parseTestCode(src);
+    Node root = script.getFirstChild();
     assertEquals(0, compiler.getErrorCount());
-    Scope scope = SyntacticScopeCreator.makeUntyped(compiler).createScope(n, null);
+    Scope globalScope = scopeCreator.createScope(script, null);
+    Scope scope = scopeCreator.createScope(root, globalScope);
     ControlFlowAnalysis cfa = new ControlFlowAnalysis(compiler, false, true);
-    cfa.process(null, n);
+    cfa.process(null, root);
     ControlFlowGraph<Node> cfg = cfa.getCfg();
     useDef = new MaybeReachingVariableUse(cfg, scope, compiler);
     useDef.analyze();
     def = null;
     uses = new ArrayList<>();
-    new NodeTraversal(compiler,new LabelFinder()).traverse(n);
+    new NodeTraversal(compiler, new LabelFinder(), scopeCreator).traverse(root);
     assertNotNull("Code should have an instruction labeled D", def);
     assertFalse("Code should have an instruction labeled starting withing U",
         uses.isEmpty());

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

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

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

@@ -169,12 +169,13 @@ private void assertNotMatch(String src) {
    */
   private void computeDefUse(String src) {
     Compiler compiler = new Compiler();
+    SyntacticScopeCreator scopeCreator = SyntacticScopeCreator.makeUntyped(compiler);
     src = "function _FUNCTION(param1, param2){" + src + "}";
     Node script = compiler.parseTestCode(src);
     Node root = script.getFirstChild();
     assertEquals(0, compiler.getErrorCount());
-    Scope globalScope = SyntacticScopeCreator.makeUntyped(compiler).createScope(script, null);
-    Scope scope = SyntacticScopeCreator.makeUntyped(compiler).createScope(root, globalScope);
+    Scope globalScope = scopeCreator.createScope(script, null);
+    Scope scope = scopeCreator.createScope(root, globalScope);
     ControlFlowAnalysis cfa = new ControlFlowAnalysis(compiler, false, true);
     cfa.process(null, root);
     ControlFlowGraph<Node> cfg = cfa.getCfg();

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

@@ -298,8 +298,9 @@ public void testTraverseAtScopeWithModuleScope() {
         "var x;");
 
     Node tree = parse(compiler, code);
+    Scope globalScope = creator.createScope(tree, null);
     Node moduleBody = tree.getFirstChild();
-    Scope moduleScope = creator.createScope(moduleBody, null);
+    Scope moduleScope = creator.createScope(moduleBody, globalScope);
 
     callback.expect(moduleBody, moduleBody);