Skip to content

Commit

Permalink
Simplify scope creator's scanVars method.
Browse files Browse the repository at this point in the history 
Instead of trying to track down what nodes are in the
"current lexical scope", explicitly pass the scopes around, and in particular
give scanVars reference up to two scopes:
  * The hoist scope in which vars should be declared
  * The local block scope in which everything else should be declared.

This also allows us to remove both of the boolean parameters from scanVars #codehealth

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=162496322
  • Loading branch information
@blickly
blickly committed Jul 19, 2017
1 parent 6ca0592 commit 12c860d
Showing 1 changed file with 42 additions and 76 deletions.
118 changes: 42 additions & 76 deletions src/com/google/javascript/jscomp/Es6SyntacticScopeCreator.java
View file
Expand Up @@ -22,6 +22,7 @@
import com.google.javascript.rhino.InputId; import com.google.javascript.rhino.InputId;
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
import java.util.Set; import java.util.Set;
import javax.annotation.Nullable;


/** /**
* <p>The syntactic scope creator scans the parse tree to create a Scope object * <p>The syntactic scope creator scans the parse tree to create a Scope object
Expand Down Expand Up @@ -133,70 +134,69 @@ void populate() {
// been declared in the outer scope. // been declared in the outer scope.
String fnName = fnNameNode.getString(); String fnName = fnNameNode.getString();
if (!fnName.isEmpty() && NodeUtil.isFunctionExpression(n)) { if (!fnName.isEmpty() && NodeUtil.isFunctionExpression(n)) {
declareVar(fnNameNode); declareVar(scope, fnNameNode);
} }


// Args: Declare function variables // Args: Declare function variables
checkState(args.isParamList()); checkState(args.isParamList());
declareLHS(args); declareLHS(scope, args);
// Since we create a separate scope for body, stop scanning here // Since we create a separate scope for body, stop scanning here


} else if (n.isClass()) { } else if (n.isClass()) {
final Node classNameNode = n.getFirstChild(); final Node classNameNode = n.getFirstChild();
// Bleed the class name into the scope, if it hasn't // Bleed the class name into the scope, if it hasn't
// been declared in the outer scope. // been declared in the outer scope.
if (!classNameNode.isEmpty()) { if (!classNameNode.isEmpty() && NodeUtil.isClassExpression(n)) {
if (NodeUtil.isClassExpression(n)) { declareVar(scope, classNameNode);
declareVar(classNameNode);
}
} }
} else if (n.isRoot() } else if (n.isRoot()
|| n.isNormalBlock() || n.isNormalBlock()
|| NodeUtil.isAnyFor(n) || NodeUtil.isAnyFor(n)
|| n.isSwitch() || n.isSwitch()
|| n.isModuleBody()) { || n.isModuleBody()) {
boolean scanInnerBlocks = boolean isHoistScope =
n.isRoot() || NodeUtil.isFunctionBlock(n) || n.isModuleBody(); n.isRoot() || NodeUtil.isFunctionBlock(n) || n.isModuleBody();
scanVars(n, scanInnerBlocks, true); Scope hoistScope = isHoistScope ? scope : null;
scanVars(n, hoistScope, scope);
} else { } else {
// n is the global scope // n is the global scope
checkState(scope.isGlobal(), scope); checkState(scope.isGlobal(), scope);
scanVars(n, true, true); scanVars(n, scope, scope);
} }
} }


private void declareLHS(Node n) { private void declareLHS(Scope s, Node n) {
for (Node lhs : NodeUtil.getLhsNodesOfDeclaration(n)) { for (Node lhs : NodeUtil.getLhsNodesOfDeclaration(n)) {
declareVar(lhs); declareVar(s, lhs);
} }
} }


/** /**
* Scans and gather variables declarations under a Node * Scans and gather variables declarations under a Node
* *
* @param n The node * @param n The node
* @param scanInnerBlockScopes Whether the inner block scopes should be scanned for "var"s * @param hoistScope The scope that is the hoist target for vars, if we are scanning for vars.
* @param firstScan Whether it is the first time a scan is performed from the current scope * @param blockScope The scope that is the hoist target for block-level declarations, if we are
* scanning for block level declarations.
*/ */
private void scanVars(Node n, boolean scanInnerBlockScopes, boolean firstScan) { private void scanVars(Node n, @Nullable Scope hoistScope, @Nullable Scope blockScope) {
switch (n.getToken()) { switch (n.getToken()) {
case VAR: case VAR:
if (scope.isHoistScope()) { if (hoistScope != null) {
declareLHS(n); declareLHS(hoistScope, n);
} }
return; return;


case LET: case LET:
case CONST: case CONST:
// Only declare when scope is the current lexical scope // Only declare when scope is the current lexical scope
if (!isNodeAtCurrentLexicalScope(n)) { if (blockScope != null) {
return; declareLHS(blockScope, n);
} }
declareLHS(n);
return; return;


case FUNCTION: case FUNCTION:
if (NodeUtil.isFunctionExpression(n) || !isNodeAtCurrentLexicalScope(n)) { if (NodeUtil.isFunctionExpression(n) || blockScope == null) {
return; return;
} }


Expand All @@ -205,32 +205,32 @@ private void scanVars(Node n, boolean scanInnerBlockScopes, boolean firstScan) {
// This is invalid, but allow it so the checks can catch it. // This is invalid, but allow it so the checks can catch it.
return; return;
} }
declareVar(n.getFirstChild()); declareVar(blockScope, n.getFirstChild());
return; // should not examine function's children return; // should not examine function's children


case CLASS: case CLASS:
if (NodeUtil.isClassExpression(n) || !isNodeAtCurrentLexicalScope(n)) { if (NodeUtil.isClassExpression(n) || blockScope == null) {
return; return;
} }
String className = n.getFirstChild().getString(); String className = n.getFirstChild().getString();
if (className.isEmpty()) { if (className.isEmpty()) {
// This is invalid, but allow it so the checks can catch it. // This is invalid, but allow it so the checks can catch it.
return; return;
} }
declareVar(n.getFirstChild()); declareVar(blockScope, n.getFirstChild());
return; // should not examine class's children return; // should not examine class's children


case CATCH: case CATCH:
checkState(n.hasTwoChildren(), n); checkState(n.hasTwoChildren(), n);
// the first child is the catch var and the second child // the first child is the catch var and the second child
// is the code block // is the code block
if (isNodeAtCurrentLexicalScope(n)) { if (blockScope != null) {
declareLHS(n); declareLHS(blockScope, n);
} }
// A new scope is not created for this BLOCK because there is a scope // A new scope is not created for this BLOCK because there is a scope
// created for the BLOCK above the CATCH // created for the BLOCK above the CATCH
final Node block = n.getSecondChild(); final Node block = n.getSecondChild();
scanVars(block, scanInnerBlockScopes, false); scanVars(block, hoistScope, blockScope);
return; // only one child to scan return; // only one child to scan


case SCRIPT: case SCRIPT:
Expand All @@ -246,7 +246,7 @@ private void scanVars(Node n, boolean scanInnerBlockScopes, boolean firstScan) {


case MODULE_BODY: case MODULE_BODY:
// Module bodies are not part of global scope. // Module bodies are not part of global scope.
if (scope.isGlobal()) { if (hoistScope.isGlobal()) {
return; return;
} }
break; break;
Expand All @@ -255,7 +255,10 @@ private void scanVars(Node n, boolean scanInnerBlockScopes, boolean firstScan) {
break; break;
} }


if (!scanInnerBlockScopes && !firstScan && NodeUtil.createsBlockScope(n)) { boolean isBlockStart = blockScope != null && n == blockScope.getRootNode();
boolean enteringNewBlock = !isBlockStart && NodeUtil.createsBlockScope(n);
if (enteringNewBlock && hoistScope == null) {
// We only enter new blocks when scanning for hoisted vars
return; return;
} }


Expand All @@ -264,7 +267,7 @@ private void scanVars(Node n, boolean scanInnerBlockScopes, boolean firstScan) {
if (NodeUtil.isControlStructure(n) || NodeUtil.isStatementBlock(n)) { if (NodeUtil.isControlStructure(n) || NodeUtil.isStatementBlock(n)) {
for (Node child = n.getFirstChild(); child != null;) { for (Node child = n.getFirstChild(); child != null;) {
Node next = child.getNext(); Node next = child.getNext();
scanVars(child, scanInnerBlockScopes, false); scanVars(child, hoistScope, enteringNewBlock ? null : blockScope);
child = next; child = next;
} }
} }
Expand All @@ -276,7 +279,7 @@ private void scanVars(Node n, boolean scanInnerBlockScopes, boolean firstScan) {
* @param s The scope to declare the variable in. * @param s The scope to declare the variable in.
* @param n The node corresponding to the variable name. * @param n The node corresponding to the variable name.
*/ */
private void declareVar(Node n) { private void declareVar(Scope s, Node n) {
checkState(n.isName() || n.isStringKey(), checkState(n.isName() || n.isStringKey(),
"Invalid node for declareVar: %s", n); "Invalid node for declareVar: %s", n);


Expand All @@ -286,68 +289,31 @@ private void declareVar(Node n) {
// TODO(johnlenz): hash lookups are not free and // TODO(johnlenz): hash lookups are not free and
// building scopes are already expensive // building scopes are already expensive
// restructure the scope building to avoid this check. // restructure the scope building to avoid this check.
Var v = scope.getOwnSlot(name); Var v = s.getOwnSlot(name);
if (v != null && v.getNode() == n) { if (v != null && v.getNode() == n) {
return; return;
} }


CompilerInput input = compiler.getInput(inputId); CompilerInput input = compiler.getInput(inputId);
if (v != null if (v != null
|| isShadowingDisallowed(name) || isShadowingDisallowed(name, s)
|| ((scope.isFunctionScope() || ((s.isFunctionScope()
|| scope.isFunctionBlockScope()) && name.equals(ARGUMENTS))) { || s.isFunctionBlockScope()) && name.equals(ARGUMENTS))) {
redeclarationHandler.onRedeclaration(scope, name, n, input); redeclarationHandler.onRedeclaration(s, name, n, input);
} else { } else {
scope.declare(name, n, input); s.declare(name, n, input);
} }
} }


// Function body declarations are not allowed to shadow // Function body declarations are not allowed to shadow
// function parameters. // function parameters.
private boolean isShadowingDisallowed(String name) { private static boolean isShadowingDisallowed(String name, Scope s) {
if (scope.isFunctionBlockScope()) { if (s.isFunctionBlockScope()) {
Var maybeParam = scope.getParent().getOwnSlot(name); Var maybeParam = s.getParent().getOwnSlot(name);
return maybeParam != null && maybeParam.isParam(); return maybeParam != null && maybeParam.isParam();
} }
return false; return false;
} }

/**
* Determines whether the name should be declared at current lexical scope.
* Assume the parent node is a BLOCK, FOR, FOR_OF, SCRIPT, MODULE_BODY, or LABEL.
*
* @param n The declaration node to be checked
* @return whether the name should be declared at current lexical scope
*/
private boolean isNodeAtCurrentLexicalScope(Node n) {
Node parent = n.getParent();
Node grandparent = parent.getParent();

switch (parent.getToken()) {
case SCRIPT:
return true;
case BLOCK:
if (grandparent.isCase() || grandparent.isDefaultCase() || grandparent.isCatch()) {
return scope.getRootNode() == grandparent.getParent();
}
// Fall through
case FOR:
case FOR_IN:
case FOR_OF:
case MODULE_BODY:
return parent == scope.getRootNode();
case LABEL:
while (parent.isLabel()) {
if (parent.getParent() == scope.getRootNode()) {
return true;
}
parent = parent.getParent();
}
return false;
default:
throw new RuntimeException("Unsupported node parent: " + parent);
}
}
} }


/** /**
Expand Down

0 comments on commit 12c860d

Please sign in to comment.