diff --git a/src/com/google/javascript/jscomp/AbstractScope.java b/src/com/google/javascript/jscomp/AbstractScope.java index 2cb0961c851..fa48ce7a8c5 100644 --- a/src/com/google/javascript/jscomp/AbstractScope.java +++ b/src/com/google/javascript/jscomp/AbstractScope.java @@ -28,12 +28,27 @@ import java.util.Map; /** - * Scope contains information about a variable scope in JavaScript. - * Scopes can be nested, a scope points back to its parent scope. - * A Scope contains information about variables defined in that scope. + * Scope contains information about a variable scope in JavaScript. Scopes can be nested, a scope + * points back to its parent scope. A Scope contains information about variables defined in that + * scope. * - * @see NodeTraversal + *

ES6 introduces new scoping rules, which adds some complexity to this class. In particular, + * scopes fall into four mutually exclusive categories based on their root node: block, function, + * module, or global. Function, module, and global scopes are collectively referred to as "non-block + * scopes". We also define a scope as a "hoist scope" if it is a non-block scope *or* it is the + * outermost block scope within a function (i.e. a "function block scope"). Hoist scopes are + * important because "var" declarations are hoisted to the closest hoist scope, as opposed to ES6 + * "let" and "const" which are not hoisted, but instead added directly to whatever scope they're + * declared in. + * + *

Finally, a caution about function scopes and function block scopes: the language does not + * permit any shadowing to occur between them (with the exception of bleeding function names), so in + * many situations these scopes are treated as a single scope. Under block scoping, only function + * parameters (and optionally, bleeding function names) are declared in the function scope. It is + * kept as a separate scope so that default parameter initializers may be evaluated in a separate + * scope from the function body. * + * @see NodeTraversal */ abstract class AbstractScope, V extends AbstractVar> implements StaticScope, Serializable { @@ -323,6 +338,19 @@ public final S getClosestHoistScope() { return null; } + /** + * Returns the closest non-block scope. This is equivalent to what the current scope would have + * been for non-block-scope creators, and is thus useful for migrating code to use block scopes. + */ + public final S getClosestNonBlockScope() { + S scope = getClosestHoistScope(); + if (scope.isBlockScope()) { + scope = scope.getParent(); + checkState(!scope.isBlockScope()); + } + return scope; + } + // This is safe because any concrete subclass of AbstractScope should be assignable to S. // While it's theoretically possible to do otherwise, such a class would be very awkward to // implement, and is therefore not worth worrying about. diff --git a/src/com/google/javascript/jscomp/FunctionTypeBuilder.java b/src/com/google/javascript/jscomp/FunctionTypeBuilder.java index 9cafaaf96a3..0afb9451f00 100644 --- a/src/com/google/javascript/jscomp/FunctionTypeBuilder.java +++ b/src/com/google/javascript/jscomp/FunctionTypeBuilder.java @@ -25,12 +25,8 @@ import static com.google.javascript.rhino.jstype.JSTypeNative.VOID_TYPE; import com.google.common.base.Predicate; -import com.google.common.collect.HashMultiset; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableMultiset; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Multiset; import com.google.javascript.rhino.IR; import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.JSTypeExpression; @@ -919,15 +915,6 @@ static interface FunctionContents { /** Returns if this consists of a single throw. */ boolean mayHaveSingleThrow(); - - /** Gets a list of variables in this scope that are escaped. */ - Iterable getEscapedVarNames(); - - /** Gets a list of variables whose properties are escaped. */ - Set getEscapedQualifiedNames(); - - /** Gets the number of times each variable has been assigned. */ - Multiset getAssignedNameCounts(); } static class UnknownFunctionContents implements FunctionContents { @@ -956,29 +943,11 @@ public boolean mayHaveNonEmptyReturns() { public boolean mayHaveSingleThrow() { return true; } - - @Override - public Iterable getEscapedVarNames() { - return ImmutableList.of(); - } - - @Override - public Set getEscapedQualifiedNames() { - return ImmutableSet.of(); - } - - @Override - public Multiset getAssignedNameCounts() { - return ImmutableMultiset.of(); - } } static class AstFunctionContents implements FunctionContents { private final Node n; private boolean hasNonEmptyReturns = false; - private Set escapedVarNames; - private Set escapedQualifiedNames; - private final Multiset assignedVarNames = HashMultiset.create(); AstFunctionContents(Node n) { this.n = n; @@ -1008,40 +977,5 @@ public boolean mayHaveSingleThrow() { Node block = n.getLastChild(); return block.hasOneChild() && block.getFirstChild().isThrow(); } - - @Override - public Iterable getEscapedVarNames() { - return escapedVarNames == null - ? ImmutableList.of() : escapedVarNames; - } - - void recordEscapedVarName(String name) { - if (escapedVarNames == null) { - escapedVarNames = new HashSet<>(); - } - escapedVarNames.add(name); - } - - @Override - public Set getEscapedQualifiedNames() { - return escapedQualifiedNames == null - ? ImmutableSet.of() : escapedQualifiedNames; - } - - void recordEscapedQualifiedName(String name) { - if (escapedQualifiedNames == null) { - escapedQualifiedNames = new HashSet<>(); - } - escapedQualifiedNames.add(name); - } - - @Override - public Multiset getAssignedNameCounts() { - return assignedVarNames; - } - - void recordAssignedName(String name) { - assignedVarNames.add(name); - } } } diff --git a/src/com/google/javascript/jscomp/NodeTraversal.java b/src/com/google/javascript/jscomp/NodeTraversal.java index b685f48ff28..e4e1d67c9aa 100644 --- a/src/com/google/javascript/jscomp/NodeTraversal.java +++ b/src/com/google/javascript/jscomp/NodeTraversal.java @@ -967,6 +967,18 @@ public Node getClosestHoistScopeRoot() { return scopes.peek().getClosestHoistScope().getRootNode(); } + public Node getClosestNonBlockScopeRoot() { + int roots = scopeRoots.size(); + for (int i = roots; i > 0; i--) { + Node rootNode = scopeRoots.get(i - 1); + if (!NodeUtil.createsBlockScope(rootNode)) { + return rootNode; + } + } + + return scopes.peek().getClosestNonBlockScope().getRootNode(); + } + public AbstractScope getClosestHoistScope() { for (int i = scopeRoots.size(); i > 0; i--) { if (isHoistScopeRootNode(scopeRoots.get(i - 1))) { @@ -1050,8 +1062,7 @@ public boolean inGlobalScope() { /** Determines whether the traversal is currently in the scope of the block of a function. */ public boolean inFunctionBlockScope() { - Node scopeRoot = getScopeRoot(); - return scopeRoot.isNormalBlock() && scopeRoot.getParent().isFunction(); + return NodeUtil.isFunctionBlock(getScopeRoot()); } /** diff --git a/src/com/google/javascript/jscomp/TypedScopeCreator.java b/src/com/google/javascript/jscomp/TypedScopeCreator.java index fc2cfb73311..8af916b59c9 100644 --- a/src/com/google/javascript/jscomp/TypedScopeCreator.java +++ b/src/com/google/javascript/jscomp/TypedScopeCreator.java @@ -51,8 +51,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import com.google.common.collect.HashMultiset; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Multiset; @@ -85,9 +85,12 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.function.Predicate; import javax.annotation.Nullable; /** @@ -166,9 +169,12 @@ final class TypedScopeCreator implements ScopeCreator, StaticSymbolTable memoized = new LinkedHashMap<>(); private final boolean runsAfterNTI; - // Simple properties inferred about functions. - private final Map functionAnalysisResults = - new LinkedHashMap<>(); + // Set of functions with non-empty returns, for passing to FunctionTypeBuilder. + private final Set functionsWithNonEmptyReturns = new HashSet<>(); + // Includes both simple and qualified names. + private final Set escapedVarNames = new HashSet<>(); + // Count of how many times each variable is assigned. + private final Multiset assignedVarNames = HashMultiset.create(); // For convenience private final ObjectType unknownType; @@ -246,11 +252,7 @@ Collection getAllMemoizedScopes() { * @param scriptName the name of the script file to remove nodes for. */ void removeScopesForScript(String scriptName) { - for (Node scopeRoot : ImmutableSet.copyOf(memoized.keySet())) { - if (scriptName.equals(scopeRoot.getSourceFileName())) { - memoized.remove(scopeRoot); - } - } + memoized.keySet().removeIf(n -> scriptName.equals(NodeUtil.getSourceName(n))); } /** Create a scope, looking up in the map for the parent scope. */ @@ -303,8 +305,7 @@ private TypedScope createScopeInternal(Node root, TypedScope typedParent) { root.getLastChild().setJSType(globalThis); // Run a first-order analysis over the syntax tree. - (new FirstOrderFunctionAnalyzer(compiler, functionAnalysisResults)) - .process(root.getFirstChild(), root.getLastChild()); + new FirstOrderFunctionAnalyzer().process(root.getFirstChild(), root.getLastChild()); // Find all the classes in the global scope. newScope = createInitialScope(root); @@ -347,14 +348,14 @@ void patchGlobalScope(TypedScope globalScope, Node scriptRoot) { String scriptName = NodeUtil.getSourceName(scriptRoot); checkNotNull(scriptName); - for (Node node : ImmutableList.copyOf(functionAnalysisResults.keySet())) { - if (scriptName.equals(NodeUtil.getSourceName(node))) { - functionAnalysisResults.remove(node); - } - } - (new FirstOrderFunctionAnalyzer( - compiler, functionAnalysisResults)).process(null, scriptRoot); + Predicate inScript = + var -> scriptName.equals(NodeUtil.getSourceName(var.getScopeRoot())); + escapedVarNames.removeIf(inScript); + assignedVarNames.elementSet().removeIf(inScript); + functionsWithNonEmptyReturns.removeIf(n -> scriptName.equals(NodeUtil.getSourceName(n))); + + new FirstOrderFunctionAnalyzer().process(null, scriptRoot); // TODO(bashir): Variable declaration is not the only side effect of last // global scope generation but here we only wipe that part off. @@ -1028,13 +1029,16 @@ private FunctionType createFunctionTypeFromNodes( ownerType, propName, prototypeOwnerTypeMap); } + AstFunctionContents contents = fnRoot != null ? new AstFunctionContents(fnRoot) : null; + if (functionsWithNonEmptyReturns.contains(fnRoot)) { + contents.recordNonEmptyReturn(); + } FunctionTypeBuilder builder = - new FunctionTypeBuilder(name, compiler, errorRoot, - scope) - .setContents(getFunctionAnalysisResults(fnRoot)) - .inferFromOverriddenFunction(overriddenType, parametersNode) - .inferTemplateTypeName(info, prototypeOwner) - .inferInheritance(info); + new FunctionTypeBuilder(name, compiler, errorRoot, scope) + .setContents(contents) + .inferFromOverriddenFunction(overriddenType, parametersNode) + .inferTemplateTypeName(info, prototypeOwner) + .inferInheritance(info); if (info == null || !info.hasReturnType()) { /** @@ -1268,8 +1272,7 @@ void defineSlot(Node n, Node parent, String variableName, setDeferredType(n, type); } - newVar = - scopeToDeclareIn.declare(variableName, n, type, input, inferred); + newVar = declare(scopeToDeclareIn, variableName, n, type, input, inferred); if (type instanceof EnumType) { Node initialValue = newVar.getInitialValue(); @@ -1322,6 +1325,26 @@ void defineSlot(Node n, Node parent, String variableName, } } + /** + * Declares a variable with the given {@code name} and {@code type} on the given {@code scope}, + * returning the newly-declared {@link TypedVar}. Additionally checks the {@link + * #escapedVarNames} and {@link #assignedVarNames} maps (which were populated during the {@link + * FirstOrderFunctionAnalyzer} and marks the result as escaped or assigned exactly once if + * appropriate. + */ + private TypedVar declare( + TypedScope scope, String name, Node n, JSType type, CompilerInput input, boolean inferred) { + TypedVar var = scope.declare(name, n, type, input, inferred); + ScopedName scopedName = ScopedName.of(name, scope.getRootNode()); + if (escapedVarNames.contains(scopedName)) { + var.markEscaped(); + } + if (assignedVarNames.count(scopedName) == 1) { + var.markAssignedExactlyOnce(); + } + return var; + } + private void finishConstructorDefinition( Node n, String variableName, FunctionType fnType, TypedScope scopeToDeclareIn, CompilerInput input, TypedVar newVar) { @@ -1827,9 +1850,7 @@ private boolean isQualifiedNameInferred( // Check if this is assigned in an inner scope. // Functions assigned in inner scopes are inferred. - AstFunctionContents contents = - getFunctionAnalysisResults(scope.getRootNode()); - if (contents == null || !contents.getEscapedQualifiedNames().contains(qName)) { + if (!escapedVarNames.contains(ScopedName.of(qName, scope.getRootNode()))) { return false; } } @@ -2010,33 +2031,6 @@ private LocalScopeBuilder(TypedScope scope) { thisTypeForProperties = getThisTypeForCollectingProperties(); } - /** - * Traverse the scope root and build it. - */ - @Override - void build() { - super.build(); - - AstFunctionContents contents = - getFunctionAnalysisResults(scope.getRootNode()); - if (contents != null) { - for (String varName : contents.getEscapedVarNames()) { - TypedVar v = scope.getVar(varName); - checkState(v.getScope() == scope); - v.markEscaped(); - } - - for (Multiset.Entry entry : - contents.getAssignedNameCounts().entrySet()) { - TypedVar v = scope.getVar(entry.getElement()); - checkState(v.getScope() == scope); - if (entry.getCount() == 1) { - v.markAssignedExactlyOnce(); - } - } - } - } - /** * Visit a node in a local scope, and add any local variables or catch * parameters into the local symbol table. @@ -2180,32 +2174,16 @@ private void declareArguments(Node functionNode) { } // end LocalScopeBuilder /** - * Does a first-order function analysis that just looks at simple things - * like what variables are escaped, and whether 'this' is used. + * Does a first-order function analysis that just looks at simple things like what variables are + * escaped, and whether 'this' is used. */ - private static class FirstOrderFunctionAnalyzer - extends AbstractScopedCallback implements CompilerPass { - private final AbstractCompiler compiler; - private final Map data; - - FirstOrderFunctionAnalyzer( - AbstractCompiler compiler, Map outParam) { - this.compiler = compiler; - this.data = outParam; - } + private class FirstOrderFunctionAnalyzer extends AbstractScopedCallback { - @Override public void process(Node externs, Node root) { + void process(Node externs, Node root) { if (externs == null) { - NodeTraversal.traverseTyped(compiler, root, this); + NodeTraversal.traverseEs6(compiler, root, this); } else { - NodeTraversal.traverseRootsTyped(compiler, this, externs, root); - } - } - - @Override public void enterScope(NodeTraversal t) { - if (!t.inGlobalScope()) { - Node n = t.getScopeRoot(); - data.put(n, new AstFunctionContents(n)); + NodeTraversal.traverseRootsEs6(compiler, this, externs, root); } } @@ -2222,52 +2200,43 @@ private static class FirstOrderFunctionAnalyzer return; } + Node nonBlockScopeRoot = t.getClosestNonBlockScopeRoot(); + if (n.isReturn() && n.getFirstChild() != null) { - data.get(t.getScopeRoot()).recordNonEmptyReturn(); + functionsWithNonEmptyReturns.add(nonBlockScopeRoot); } - if (n.isName() - && NodeUtil.isLValue(n) - // Be careful of bleeding functions, which create variables - // in the inner scope, not the scope where the name appears. - && !NodeUtil.isBleedingFunctionName(n)) { + // Be careful of bleeding functions, which create variables + // in the inner scope, not the scope where the name appears. + if (n.isName() && NodeUtil.isLValue(n) && !NodeUtil.isBleedingFunctionName(n)) { String name = n.getString(); - TypedScope scope = t.getTypedScope(); - TypedVar var = scope.getVar(name); + Scope scope = t.getScope(); + Var var = scope.getVar(name); if (var != null) { - TypedScope ownerScope = var.getScope(); + Scope ownerScope = var.getScope().getClosestNonBlockScope(); if (ownerScope.isLocal()) { - data.get(ownerScope.getRootNode()).recordAssignedName(name); - } - - if (scope != ownerScope && ownerScope.isLocal()) { - data.get(ownerScope.getRootNode()).recordEscapedVarName(name); + Node ownerScopeRoot = ownerScope.getRootNode(); + assignedVarNames.add(ScopedName.of(name, ownerScopeRoot)); + if (nonBlockScopeRoot != ownerScopeRoot) { + escapedVarNames.add(ScopedName.of(name, ownerScopeRoot)); + } } } } else if (n.isGetProp() && n.isUnscopedQualifiedName() && NodeUtil.isLValue(n)) { String name = NodeUtil.getRootOfQualifiedName(n).getString(); - TypedScope scope = t.getTypedScope(); - TypedVar var = scope.getVar(name); + Scope scope = t.getScope(); + Var var = scope.getVar(name); if (var != null) { - TypedScope ownerScope = var.getScope(); - if (scope != ownerScope && ownerScope.isLocal()) { - data.get(ownerScope.getRootNode()) - .recordEscapedQualifiedName(n.getQualifiedName()); + Scope ownerScope = var.getScope().getClosestNonBlockScope(); + Node ownerScopeRoot = ownerScope.getRootNode(); + if (ownerScope.isLocal() && nonBlockScopeRoot != ownerScopeRoot) { + escapedVarNames.add(ScopedName.of(n.getQualifiedName(), ownerScopeRoot)); } } } } } - private AstFunctionContents getFunctionAnalysisResults(@Nullable Node n) { - if (n == null) { - return null; - } - - // Sometimes this will return null in things that build partial scopes. - return functionAnalysisResults.get(n); - } - @Override public boolean hasBlockScope() { return false;