diff --git a/src/com/google/javascript/jscomp/Es6SyntacticScopeCreator.java b/src/com/google/javascript/jscomp/Es6SyntacticScopeCreator.java index 63be9313d59..b89dc68bacf 100644 --- a/src/com/google/javascript/jscomp/Es6SyntacticScopeCreator.java +++ b/src/com/google/javascript/jscomp/Es6SyntacticScopeCreator.java @@ -120,8 +120,8 @@ private void scanRoot(Node n) { n.isRoot() || NodeUtil.isFunctionBlock(n) || n.isModuleBody(); scanVars(n, scanInnerBlocks, true); } else { - // n is the global SCRIPT node - checkState(scope.getParent() == null); + // n is the global scope + checkState(scope.isGlobal(), scope); scanVars(n, true, true); } } @@ -249,7 +249,7 @@ private void declareVar(Node n) { CompilerInput input = compiler.getInput(inputId); if (v != null || isShadowingDisallowed(name) - || (scope.isLocal() && name.equals(ARGUMENTS))) { + || ((scope.isFunctionScope() || scope.isFunctionBlockScope()) && name.equals(ARGUMENTS))) { redeclarationHandler.onRedeclaration(scope, name, n, input); } else { scope.declare(name, n, input); diff --git a/src/com/google/javascript/jscomp/Scope.java b/src/com/google/javascript/jscomp/Scope.java index 65ab5bda05e..b170e72a7bd 100644 --- a/src/com/google/javascript/jscomp/Scope.java +++ b/src/com/google/javascript/jscomp/Scope.java @@ -16,6 +16,8 @@ package com.google.javascript.jscomp; +import static com.google.common.base.Preconditions.checkArgument; + import com.google.common.base.Preconditions; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.StaticScope; @@ -41,7 +43,7 @@ public class Scope implements StaticScope { /** * Creates a Scope given the parent Scope and the root node of the scope. * @param parent The parent Scope. Cannot be null. - * @param rootNode Typically the FUNCTION node. + * @param rootNode */ Scope(Scope parent, Node rootNode) { Preconditions.checkNotNull(parent); @@ -67,6 +69,13 @@ 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); return new Scope(rootNode); } diff --git a/src/com/google/javascript/jscomp/VarCheck.java b/src/com/google/javascript/jscomp/VarCheck.java index 2892fd16697..cebb55b0886 100644 --- a/src/com/google/javascript/jscomp/VarCheck.java +++ b/src/com/google/javascript/jscomp/VarCheck.java @@ -23,7 +23,6 @@ import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.JSDocInfoBuilder; import com.google.javascript.rhino.Node; - import java.util.ArrayList; import java.util.HashSet; import java.util.Set; @@ -117,9 +116,8 @@ class VarCheck extends AbstractPostOrderCallback implements } /** - * Create a SyntacticScopeCreator. If not in sanity check mode, use a - * {@link RedeclarationCheckHandler} to check var redeclarations. - * @return the SyntacticScopeCreator + * Creates the scope creator used by this pass. If not in sanity check mode, use a {@link + * RedeclarationCheckHandler} to check var redeclarations. */ private ScopeCreator createScopeCreator() { if (sanityCheck) { @@ -194,11 +192,11 @@ public void visit(NodeTraversal t, Node n, Node parent) { Var var = scope.getVar(varName); if (var == null) { if (NodeUtil.isFunctionExpression(parent) - || NodeUtil.isClassExpression(parent) && n == parent.getFirstChild()) { + || (NodeUtil.isClassExpression(parent) && n == parent.getFirstChild())) { // e.g. [ function foo() {} ], it's okay if "foo" isn't defined in the // current scope. } else { - boolean isArguments = scope.isLocal() && ARGUMENTS.equals(varName); + boolean isArguments = scope.isFunctionScope() && ARGUMENTS.equals(varName); // The extern checks are stricter, don't report a second error. if (!isArguments && !(strictExternCheck && t.getInput().isExtern())) { t.report(n, UNDEFINED_VAR_ERROR, varName); diff --git a/test/com/google/javascript/jscomp/Es6SyntacticScopeCreatorTest.java b/test/com/google/javascript/jscomp/Es6SyntacticScopeCreatorTest.java index 1ca61008c1f..1b21aed2d6b 100644 --- a/test/com/google/javascript/jscomp/Es6SyntacticScopeCreatorTest.java +++ b/test/com/google/javascript/jscomp/Es6SyntacticScopeCreatorTest.java @@ -16,16 +16,16 @@ package com.google.javascript.jscomp; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static com.google.common.truth.Truth.assertThat; +import static com.google.javascript.jscomp.CompilerTestCase.LINE_JOINER; -import com.google.common.base.Joiner; import com.google.common.collect.HashMultiset; import com.google.common.collect.Multiset; import com.google.javascript.jscomp.CompilerOptions.LanguageMode; import com.google.javascript.jscomp.SyntacticScopeCreator.RedeclarationHandler; import com.google.javascript.rhino.Node; - import junit.framework.TestCase; /** @@ -268,7 +268,7 @@ public void testArrayDestructuringVarInBlock() { } public void testObjectDestructuring() { - String js = Joiner.on('\n').join( + String js = LINE_JOINER.join( "function foo() {", " var {a, b} = bar();", "}"); @@ -287,7 +287,7 @@ public void testObjectDestructuring() { } public void testObjectDestructuring2() { - String js = Joiner.on('\n').join( + String js = LINE_JOINER.join( "function foo() {", " var {a: b = 1} = bar();", "}"); @@ -306,7 +306,7 @@ public void testObjectDestructuring2() { } public void testObjectDestructuringComputedProp() { - String js = Joiner.on('\n').join( + String js = LINE_JOINER.join( "function foo() {", " var {['s']: a} = bar();", "}"); @@ -335,7 +335,7 @@ public void testObjectDestructuringComputedPropParam() { } public void testObjectDestructuringNested() { - String js = Joiner.on('\n').join( + String js = LINE_JOINER.join( "function foo() {", " var {a:{b}} = bar();", "}"); @@ -354,7 +354,7 @@ public void testObjectDestructuringNested() { } public void testObjectDestructuringWithInitializer() { - String js = Joiner.on('\n').join( + String js = LINE_JOINER.join( "function foo() {", " var {a=1} = bar();", "}"); @@ -431,8 +431,12 @@ public void testScopeRootNode() { assertFalse(globalScope.isBlockScope()); assertEquals(globalScope, globalScope.getClosestHoistScope()); - Node fooBlockNode = root.getFirstChild().getLastChild(); - Scope fooScope = scopeCreator.createScope(fooBlockNode, null); + Node function = root.getFirstChild(); + checkState(function.isFunction(), function); + Scope functionScope = scopeCreator.createScope(function, globalScope); + + Node fooBlockNode = NodeUtil.getFunctionBody(function); + Scope fooScope = scopeCreator.createScope(fooBlockNode, functionScope); assertEquals(fooBlockNode, fooScope.getRootNode()); assertTrue(fooScope.isBlockScope()); assertEquals(fooScope, fooScope.getClosestHoistScope()); @@ -522,22 +526,40 @@ public void testForOfLoopScope() { public void testFunctionArgument() { String js = "function f(x) { if (true) { let y = 3; } }"; - Node functionBlock = getRoot(js).getLastChild(); - Scope functionScope = scopeCreator.createScope(functionBlock, null); - assertTrue(functionScope.isDeclared("x", false)); - assertFalse(functionScope.isDeclared("y", false)); + Node root = getRoot(js); + Scope global = scopeCreator.createScope(root, null); + Node function = root.getLastChild(); + checkState(function.isFunction(), function); + Scope functionScope = scopeCreator.createScope(function, global); - Node ifBlock = functionBlock.getLastChild().getLastChild().getLastChild(); - Scope blockScope = scopeCreator.createScope(ifBlock, functionScope); - assertTrue(blockScope.isDeclared("x", false)); + Node functionBlock = NodeUtil.getFunctionBody(function); + Scope fBlockScope = scopeCreator.createScope(functionBlock, functionScope); + + assertTrue(fBlockScope.isDeclared("x", false)); + assertFalse(fBlockScope.isDeclared("y", false)); + + Node ifBlock = functionBlock.getLastChild().getLastChild(); + checkState(ifBlock.isNormalBlock(), ifBlock); + Scope blockScope = scopeCreator.createScope(ifBlock, fBlockScope); + assertFalse(blockScope.isDeclared("x", false)); + assertTrue(blockScope.isDeclared("x", true)); assertTrue(blockScope.isDeclared("y", false)); } public void testTheArgumentsVariable() { String js = "function f() { if (true) { let arguments = 3; } }"; - Node ifBlock = getRoot(js).getLastChild().getLastChild() - .getLastChild().getLastChild(); - Scope blockScope = scopeCreator.createScope(ifBlock, null); + Node root = getRoot(js); + Scope global = scopeCreator.createScope(root, null); + + Node function = root.getFirstChild(); + checkState(function.isFunction(), function); + Scope fScope = scopeCreator.createScope(function, global); + + Node fBlock = NodeUtil.getFunctionBody(function); + Scope fBlockScope = scopeCreator.createScope(fBlock, fScope); + + Node ifBlock = fBlock.getFirstChild().getLastChild(); + Scope blockScope = scopeCreator.createScope(ifBlock, fBlockScope); assertTrue(blockScope.isDeclared("arguments", false)); } @@ -588,17 +610,22 @@ public void testIsCatchBlockScoped() { } public void testVarAfterLet() { - String js = Joiner.on('\n').join( + String js = LINE_JOINER.join( "function f() {", " if (a) {", " let x;", " }", " var y;", - "}" - ); + "}"); + Node root = getRoot(js); + Scope global = scopeCreator.createScope(root, null); + Node function = root.getFirstChild(); + Scope fScope = scopeCreator.createScope(function, global); + Node fBlock = root.getFirstChild().getLastChild(); - Scope fBlockScope = scopeCreator.createScope(fBlock, null); + Scope fBlockScope = scopeCreator.createScope(fBlock, fScope); + checkNotNull(fBlockScope); assertFalse(fBlockScope.isDeclared("x", false)); assertTrue(fBlockScope.isDeclared("y", false)); @@ -611,15 +638,16 @@ public void testVarAfterLet() { public void testSimpleFunctionParam() { String js = "function f(x) {}"; Node root = getRoot(js); - Node fNode = root.getFirstChild(); - Scope globalScope = scopeCreator.createScope(root, null); + + Node fNode = root.getFirstChild(); + checkState(fNode.isFunction(), fNode); Scope fScope = scopeCreator.createScope(fNode, globalScope); assertTrue(fScope.isDeclared("x", false)); - Node fBlock = fNode.getLastChild(); - Scope fBlockScope = scopeCreator.createScope(fBlock, null); - assertFalse(fBlockScope.isDeclared("x", false)); + Node fBlock = NodeUtil.getFunctionBody(fNode); + Scope fBlockScope = scopeCreator.createScope(fBlock, fScope); + assertTrue(fBlockScope.isDeclared("x", false)); } public void testOnlyOneDeclaration() { diff --git a/test/com/google/javascript/jscomp/GlobalVarReferenceMapTest.java b/test/com/google/javascript/jscomp/GlobalVarReferenceMapTest.java index f3fb0236c03..54db352e4f0 100644 --- a/test/com/google/javascript/jscomp/GlobalVarReferenceMapTest.java +++ b/test/com/google/javascript/jscomp/GlobalVarReferenceMapTest.java @@ -50,7 +50,7 @@ public final class GlobalVarReferenceMapTest extends TestCase { private final GlobalVarReferenceMap map = new GlobalVarReferenceMap( ImmutableList.of(INPUT1, INPUT2, INPUT3), ImmutableList.of(EXTERN1)); private final Map globalMap = new HashMap<>(); - private final Node root = new Node(Token.BLOCK); + private final Node root = new Node(Token.ROOT); private final Scope globalScope = Scope.createGlobalScope(root); private Node scriptRoot = new Node(Token.SCRIPT); diff --git a/test/com/google/javascript/jscomp/IntegrationTest.java b/test/com/google/javascript/jscomp/IntegrationTest.java index 75ebdfa8b09..5a247bd7522 100644 --- a/test/com/google/javascript/jscomp/IntegrationTest.java +++ b/test/com/google/javascript/jscomp/IntegrationTest.java @@ -3051,11 +3051,11 @@ public void process(Node externs, Node root) { public void testSuppressEs5StrictWarning() { CompilerOptions options = createCompilerOptions(); + CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options); options.setWarningLevel(DiagnosticGroups.ES5_STRICT, CheckLevel.WARNING); test(options, - "/** @suppress{es5Strict} */\n" + - "function f() { var arguments; }", - "function f() {}"); + "/** @suppress{es5Strict} */ function f() { var arguments; }", + ""); } public void testCheckProvidesWarning() { diff --git a/test/com/google/javascript/jscomp/VarCheckTest.java b/test/com/google/javascript/jscomp/VarCheckTest.java index f2daa23460e..712c344623b 100644 --- a/test/com/google/javascript/jscomp/VarCheckTest.java +++ b/test/com/google/javascript/jscomp/VarCheckTest.java @@ -579,7 +579,7 @@ public void testFunctionScopeArguments() { testError("var f = function arguments() {}", VarCheck.VAR_ARGUMENTS_SHADOWED_ERROR); testError("var f = function (arguments) {}", VarCheck.VAR_ARGUMENTS_SHADOWED_ERROR); - testError("function f() {try {} catch(arguments) {}}", VarCheck.VAR_ARGUMENTS_SHADOWED_ERROR); + testSame("function f() {try {} catch(arguments) {}}"); sanityCheck = true; testSame("function f() {var arguments}");