Skip to content

Commit

Permalink
Make TypedScope no longer extend Scope and TypedVar no longer extend …
Browse files Browse the repository at this point in the history 
…Var.

These two changes need to be done simultaneously since TypedScopes want to return TypedVars and vice versa, and if one subclass relationship is broken then the return types of the other are no longer properly covariant.

Making the typed and untyped versions of Scope and Var more distinct allows using generics more cleanly.  It also enables us to remove the redundant overrides in the typed versions that simply called the superclass and casted the result.  Many of those methods in the abstract base class can now be marked 'final'.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=181703776
  • Loading branch information
@shicks @brad4d
shicks authored and brad4d committed Jan 12, 2018
1 parent 6febacc commit dd936ec
Show file tree
Hide file tree
Showing 11 changed files with 181 additions and 253 deletions.
146 changes: 75 additions & 71 deletions src/com/google/javascript/jscomp/AbstractScope.java
View file
Expand Up @@ -37,62 +37,37 @@
*/
abstract class AbstractScope<S extends AbstractScope<S, V>, V extends AbstractVar<S, V>>
implements StaticScope, Serializable {
final Map<String, V> vars = new LinkedHashMap<>();
S parent;
int depth;
final Node rootNode;
private final Map<String, V> vars = new LinkedHashMap<>();
private final Node rootNode;
private V arguments;

/**
* Creates a Scope given the parent Scope and the root node of the current scope.
*
* @param parent The parent Scope. Cannot be null.
* @param rootNode The root node of the current scope. Cannot be null.
*/
AbstractScope(S parent, Node rootNode) {
checkNotNull(parent);
checkArgument(NodeUtil.createsScope(rootNode), rootNode);
checkArgument(
rootNode != parent.rootNode, "rootNode should not be the parent's root node", rootNode);

this.parent = parent;
this.rootNode = rootNode;
this.depth = parent.depth + 1;
}

AbstractScope(Node rootNode) {
// TODO(tbreisacher): Can we tighten this to just NodeUtil.createsScope?
checkArgument(
NodeUtil.createsScope(rootNode) || rootNode.isScript() || rootNode.isRoot(), rootNode);
this.parent = null;
this.rootNode = rootNode;
this.depth = 0;
}

/** The depth of the scope. The global scope has depth 0. */
public abstract int getDepth();

/** Returns the parent scope, or null if this is the global scope. */
public abstract S getParent();

@Override
public String toString() {
public final String toString() {
return "Scope@" + rootNode;
}

/** The depth of the scope. The global scope has depth 0. */
public int getDepth() {
return depth;
}

public Scope untyped() {
checkState(!(this instanceof TypedScope));
return (Scope) this;
throw new IllegalStateException("untyped() called, but not an untyped scope.");
}

public TypedScope typed() {
checkState(this instanceof TypedScope);
return (TypedScope) this;
throw new IllegalStateException("typed() called, but not a typed scope.");
}

/**
* @return True if this scope contains {@code other}, or is the same scope as {@code other}.
*/
boolean contains(S other) {
final boolean contains(S other) {
S s = checkNotNull(other);
while (s != null) {
if (s == this) {
Expand All @@ -107,16 +82,14 @@ boolean contains(S other) {
* Gets the container node of the scope. This is typically the FUNCTION
* node or the global BLOCK/SCRIPT node.
*/
// Non-final for jsdev tests
@Override
public Node getRootNode() {
return rootNode;
}

public S getParent() {
return parent;
}

S getGlobalScope() {
/** Walks up the tree to find the global scope. */
final S getGlobalScope() {
S result = thisScope();
while (result.getParent() != null) {
result = result.getParent();
Expand All @@ -125,8 +98,8 @@ S getGlobalScope() {
}

@Override
public S getParentScope() {
return parent;
public final S getParentScope() {
return getParent();
}

abstract V makeArgumentsVar();
Expand All @@ -135,42 +108,51 @@ public S getParentScope() {
* Undeclares a variable, to be used when the compiler optimizes out
* a variable and removes it from the scope.
*/
void undeclare(V var) {
final void undeclare(V var) {
checkState(var.scope == this);
checkState(vars.get(var.name).equals(var));
undeclareInteral(var);
}

/** Without any safety checks */
void undeclareInteral(V var) {
final void undeclareInteral(V var) {
vars.remove(var.name);
}

final void declareInternal(String name, V var) {
vars.put(name, var);
}

final void clearVarsInternal() {
vars.clear();
}

@Override
public V getSlot(String name) {
public final V getSlot(String name) {
return getVar(name);
}

@Override
public V getOwnSlot(String name) {
public final V getOwnSlot(String name) {
return vars.get(name);
}

/**
* Returns the variable, may be null
*/
// Non-final for jsdev tests
public V getVar(String name) {
S scope = thisScope();
while (scope != null) {
V var = scope.vars.get(name);
V var = scope.getOwnSlot(name);
if (var != null) {
return var;
}
if (Var.ARGUMENTS.equals(name) && NodeUtil.isVanillaFunction(scope.getRootNode())) {
return scope.getArgumentsVar();
}
// Recurse up the parent Scope
scope = scope.parent;
scope = scope.getParent();
}
return null;
}
Expand All @@ -184,7 +166,7 @@ public final V getArgumentsVar() {
}

if (!isFunctionScope() || rootNode.isArrowFunction()) {
return parent.getArgumentsVar();
return getParent().getArgumentsVar();
}

if (arguments == null) {
Expand All @@ -195,28 +177,29 @@ public final V getArgumentsVar() {

/**
* Use only when in a function block scope and want to tell if a name is either at the top of the
* function block scope or the function parameter scope
* function block scope or the function parameter scope. This obviously only applies to ES6 block
* scopes.
*/
public boolean isDeclaredInFunctionBlockOrParameter(String name) {
public final boolean isDeclaredInFunctionBlockOrParameter(String name) {
// In ES6, we create a separate "function parameter scope" above the function block scope to
// handle default parameters. Since nothing in the function block scope is allowed to shadow
// the variables in the function scope, we treat the two scopes as one in this method.
checkState(isFunctionBlockScope());
return isDeclared(name, false) || parent.isDeclared(name, false);
return isDeclared(name, false) || (getParent().isDeclared(name, false));
}

/**
* Returns true if a variable is declared.
*/
public boolean isDeclared(String name, boolean recurse) {
public final boolean isDeclared(String name, boolean recurse) {
S scope = thisScope();
while (true) {
if (scope.vars.containsKey(name)) {
if (scope.getOwnSlot(name) != null) {
return true;
}

if (scope.parent != null && recurse) {
scope = scope.parent;
if (scope.getParent() != null && recurse) {
scope = scope.getParent();
continue;
}
return false;
Expand All @@ -227,7 +210,8 @@ public boolean isDeclared(String name, boolean recurse) {
* Return an iterable over all of the variables declared in this scope
* (except the special 'arguments' variable).
*/
public Iterable<? extends V> getVarIterable() {
// Non-final for jsdev tests
public Iterable<V> getVarIterable() {
return vars.values();
}

Expand All @@ -243,7 +227,7 @@ public Iterable<? extends V> getVarIterable() {
*
* <p>We do not include the special 'arguments' variable.
*/
public Iterable<? extends V> getAllAccessibleVariables() {
public final Iterable<V> getAllAccessibleVariables() {
Map<String, V> accessibleVars = new LinkedHashMap<>();
S s = thisScope();

Expand All @@ -258,48 +242,52 @@ public Iterable<? extends V> getAllAccessibleVariables() {
return accessibleVars.values();
}

public Iterable<? extends V> getAllSymbols() {
// Non-final for jsdev tests
public Iterable<V> getAllSymbols() {
return Collections.unmodifiableCollection(vars.values());
}

/**
* Returns number of variables in this scope (excluding the special 'arguments' variable)
*/
// Non-final for jsdev tests
public int getVarCount() {
return vars.size();
}

/**
* Returns whether this is the global scope.
*/
// Non-final for jsdev tests
public boolean isGlobal() {
return parent == null;
return getParent() == null;
}

/**
* Returns whether this is a local scope (i.e. not the global scope).
*/
// Non-final for jsdev tests
public boolean isLocal() {
return parent != null;
return getParent() != null;
}

public boolean isBlockScope() {
public final boolean isBlockScope() {
return NodeUtil.createsBlockScope(rootNode);
}

public boolean isFunctionBlockScope() {
public final boolean isFunctionBlockScope() {
return NodeUtil.isFunctionBlock(getRootNode());
}

public boolean isFunctionScope() {
public final boolean isFunctionScope() {
return getRootNode().isFunction();
}

public boolean isModuleScope() {
public final boolean isModuleScope() {
return getRootNode().isModuleBody();
}

public boolean isCatchScope() {
public final boolean isCatchScope() {
return getRootNode().isNormalBlock()
&& getRootNode().hasOneChild()
&& getRootNode().getFirstChild().isCatch();
Expand All @@ -313,7 +301,7 @@ && getRootNode().hasOneChild()
* 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() {
final boolean isHoistScope() {
return isFunctionScope() || isFunctionBlockScope() || isGlobal() || isModuleScope();
}

Expand All @@ -324,13 +312,13 @@ boolean isHoistScope() {
* to declare a var inside function parameters, it would make even less sense to say that
* such declarations would be "hoisted" somewhere else.
*/
public S getClosestHoistScope() {
public final S getClosestHoistScope() {
S current = thisScope();
while (current != null) {
if (current.isHoistScope()) {
return current;
}
current = current.parent;
current = current.getParent();
}
return null;
}
Expand All @@ -342,4 +330,20 @@ public S getClosestHoistScope() {
private S thisScope() {
return (S) this;
}

/** Performs simple validity checks on when constructing a child scope. */
void checkChildScope(S parent) {
checkNotNull(parent);
checkArgument(NodeUtil.createsScope(rootNode), rootNode);
checkArgument(
rootNode != parent.getRootNode(),
"rootNode should not be the parent's root node: %s", rootNode);
}

/** Performs simple validity checks on when constructing a root scope. */
void checkRootScope() {
// TODO(tbreisacher): Can we tighten this to just NodeUtil.createsScope?
checkArgument(
NodeUtil.createsScope(rootNode) || rootNode.isScript() || rootNode.isRoot(), rootNode);
}
}
9 changes: 3 additions & 6 deletions src/com/google/javascript/jscomp/AbstractVar.java
View file
Expand Up @@ -77,15 +77,13 @@ public StaticSourceFile getSourceFile() {
return nameNode.getStaticSourceFile();
}

// Temporarily non-final for TypedVar
@Override
public V getSymbol() {
public final V getSymbol() {
return thisVar();
}

// Temporarily non-final for TypedVar
@Override
public V getDeclaration() {
public final V getDeclaration() {
return nameNode == null ? null : thisVar();
}

Expand All @@ -101,8 +99,7 @@ public boolean isBleedingFunction() {
return NodeUtil.isFunctionExpression(getParentNode());
}

// Temporarily non-final for TypedVar
public S getScope() {
public final S getScope() {
return scope;
}

Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/Es6SyntacticScopeCreator.java
View file
Expand Up @@ -81,7 +81,7 @@ private static class DefaultScopeFactory implements ScopeFactory {
public Scope create(Scope parent, Node n) {
return (parent == null)
? Scope.createGlobalScope(n)
: new Scope(parent, n);
: Scope.createChildScope(parent, n);
}
}

Expand Down
8 changes: 7 additions & 1 deletion src/com/google/javascript/jscomp/GlobalVarReferenceMap.java
View file
Expand Up @@ -217,6 +217,8 @@ public void updateReferencesWithGlobalScope(Scope globalScope) {
for (ReferenceCollection collection : refMap.values()) {
List<Reference> newRefs = new ArrayList<>(collection.references.size());
for (Reference ref : collection) {
// This is only ever called with compiler.getTopScope() in production, which is
// a TypedScope. But References are only ever created with
if (ref.getScope() != globalScope) {
newRefs.add(ref.cloneWithNewScope(globalScope));
} else {
Expand Down Expand Up @@ -246,7 +248,11 @@ public GlobalVarRefCleanupPass(AbstractCompiler compiler) {
public void hotSwapScript(Node scriptRoot, Node originalRoot) {
GlobalVarReferenceMap refMap = compiler.getGlobalVarReferences();
if (refMap != null) {
refMap.updateReferencesWithGlobalScope(compiler.getTopScope());
// We don't have a suitable untyped scope to use, but the actual scope probably doesn't
// matter, so long as it's a global scope and doesn't persist references to things that
// need to be cleaned up. So we just generate a new simple root scope.
refMap.updateReferencesWithGlobalScope(
Scope.createGlobalScope(compiler.getTopScope().getRootNode()));
}
}

Expand Down

0 comments on commit dd936ec

Please sign in to comment.