From 27039b0c2c9905982c54b657a8664992e9e67800 Mon Sep 17 00:00:00 2001 From: tbreisacher Date: Wed, 3 May 2017 19:29:20 -0700 Subject: [PATCH] Add isHoistScope for improved readability when dealing with scopes ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=155037058 --- .../jscomp/Es6SyntacticScopeCreator.java | 2 +- .../javascript/jscomp/HoistVarsOutOfBlocks.java | 2 +- src/com/google/javascript/jscomp/Scope.java | 17 +++++++++++++---- .../jscomp/Es6SyntacticScopeCreatorTest.java | 2 ++ 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/com/google/javascript/jscomp/Es6SyntacticScopeCreator.java b/src/com/google/javascript/jscomp/Es6SyntacticScopeCreator.java index b89dc68bacf..57b3e996699 100644 --- a/src/com/google/javascript/jscomp/Es6SyntacticScopeCreator.java +++ b/src/com/google/javascript/jscomp/Es6SyntacticScopeCreator.java @@ -142,7 +142,7 @@ private void declareLHS(Node n) { private void scanVars(Node n, boolean scanInnerBlockScopes, boolean firstScan) { switch (n.getToken()) { case VAR: - if (scope.getClosestHoistScope() == scope) { + if (scope.isHoistScope()) { declareLHS(n); } return; diff --git a/src/com/google/javascript/jscomp/HoistVarsOutOfBlocks.java b/src/com/google/javascript/jscomp/HoistVarsOutOfBlocks.java index bff0fd19715..33884ec148c 100644 --- a/src/com/google/javascript/jscomp/HoistVarsOutOfBlocks.java +++ b/src/com/google/javascript/jscomp/HoistVarsOutOfBlocks.java @@ -75,7 +75,7 @@ public void process(Node externs, Node root) { public void afterExitScope(NodeTraversal t, ReferenceMap refMap) { // TODO(tbreisacher): Avoid calling t.getScope() here, so that we aren't creating scopes we // don't need to. - if (t.getScope() != t.getClosestHoistScope()) { + if (!t.getScope().isHoistScope()) { return; } this.refMap = refMap; diff --git a/src/com/google/javascript/jscomp/Scope.java b/src/com/google/javascript/jscomp/Scope.java index d11aa2ef5f1..e92922310ca 100644 --- a/src/com/google/javascript/jscomp/Scope.java +++ b/src/com/google/javascript/jscomp/Scope.java @@ -256,6 +256,18 @@ && getRootNode().hasOneChild() && getRootNode().getFirstChild().isCatch(); } + /** + * If a var were declared in this scope, would it belong to this scope (as opposed to some + * enclosing scope)? + * + * We consider function scopes to be hoist scopes. Even though it's impossible to declare a var + * inside function parameters, it would make less sense to say that if you did declare one in + * the function parameters, it would be hoisted somewhere else. + */ + boolean isHoistScope() { + return isFunctionScope() || isFunctionBlockScope() || isGlobal() || isModuleScope(); + } + /** * If a var were declared in this scope, return the scope it would be hoisted to. * @@ -266,10 +278,7 @@ && getRootNode().hasOneChild() public Scope getClosestHoistScope() { Scope current = this; while (current != null) { - if (current.isFunctionScope() - || current.isFunctionBlockScope() - || current.isGlobal() - || current.isModuleScope()) { + if (current.isHoistScope()) { return current; } current = current.parent; diff --git a/test/com/google/javascript/jscomp/Es6SyntacticScopeCreatorTest.java b/test/com/google/javascript/jscomp/Es6SyntacticScopeCreatorTest.java index 70b1df846a0..994f5eb12c0 100644 --- a/test/com/google/javascript/jscomp/Es6SyntacticScopeCreatorTest.java +++ b/test/com/google/javascript/jscomp/Es6SyntacticScopeCreatorTest.java @@ -430,6 +430,7 @@ public void testScopeRootNode() { assertEquals(root, globalScope.getRootNode()); assertFalse(globalScope.isBlockScope()); assertEquals(globalScope, globalScope.getClosestHoistScope()); + assertTrue(globalScope.isHoistScope()); Node function = root.getFirstChild(); checkState(function.isFunction(), function); @@ -440,6 +441,7 @@ public void testScopeRootNode() { assertEquals(fooBlockNode, fooScope.getRootNode()); assertTrue(fooScope.isBlockScope()); assertEquals(fooScope, fooScope.getClosestHoistScope()); + assertTrue(fooScope.isHoistScope()); assertTrue(fooScope.isDeclared("x", false)); }