diff --git a/src/com/google/javascript/jscomp/Scope.java b/src/com/google/javascript/jscomp/Scope.java index 4c00fad23bc..932f9c5e1ab 100644 --- a/src/com/google/javascript/jscomp/Scope.java +++ b/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); } diff --git a/test/com/google/javascript/jscomp/LinkedFlowScopeTest.java b/test/com/google/javascript/jscomp/LinkedFlowScopeTest.java index 3fcc2799eb0..b88532cbf5a 100644 --- a/test/com/google/javascript/jscomp/LinkedFlowScopeTest.java +++ b/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); diff --git a/test/com/google/javascript/jscomp/MaybeReachingVariableUseTest.java b/test/com/google/javascript/jscomp/MaybeReachingVariableUseTest.java index 70238fa0965..603c644fc94 100644 --- a/test/com/google/javascript/jscomp/MaybeReachingVariableUseTest.java +++ b/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 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()); diff --git a/test/com/google/javascript/jscomp/MemoizedScopeCreatorTest.java b/test/com/google/javascript/jscomp/MemoizedScopeCreatorTest.java index 526a281b29d..8efff3a44ad 100644 --- a/test/com/google/javascript/jscomp/MemoizedScopeCreatorTest.java +++ b/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; } diff --git a/test/com/google/javascript/jscomp/MustBeReachingVariableDefTest.java b/test/com/google/javascript/jscomp/MustBeReachingVariableDefTest.java index 89080578656..b5766b96e78 100644 --- a/test/com/google/javascript/jscomp/MustBeReachingVariableDefTest.java +++ b/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 cfg = cfa.getCfg(); diff --git a/test/com/google/javascript/jscomp/NodeTraversalTest.java b/test/com/google/javascript/jscomp/NodeTraversalTest.java index abf915daefc..7cfce03f613 100644 --- a/test/com/google/javascript/jscomp/NodeTraversalTest.java +++ b/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);