diff --git a/src/com/google/javascript/jscomp/AbstractScope.java b/src/com/google/javascript/jscomp/AbstractScope.java index 924af2e4008..e32b6e73ce2 100644 --- a/src/com/google/javascript/jscomp/AbstractScope.java +++ b/src/com/google/javascript/jscomp/AbstractScope.java @@ -24,6 +24,7 @@ import com.google.javascript.rhino.StaticScope; import java.io.Serializable; import java.util.Collections; +import java.util.EnumMap; import java.util.LinkedHashMap; import java.util.Map; @@ -53,8 +54,8 @@ abstract class AbstractScope, V extends AbstractVar> implements StaticScope, Serializable { private final Map vars = new LinkedHashMap<>(); + private final Map implicitVars = new EnumMap<>(ImplicitVar.class); private final Node rootNode; - private V arguments; AbstractScope(Node rootNode) { this.rootNode = rootNode; @@ -117,7 +118,7 @@ public final S getParentScope() { return getParent(); } - abstract V makeArgumentsVar(); + abstract V makeImplicitVar(ImplicitVar type); /** * Undeclares a variable, to be used when the compiler optimizes out @@ -157,6 +158,10 @@ public final V getOwnSlot(String name) { */ // Non-final for jsdev tests public V getVar(String name) { + ImplicitVar implicit = name != null ? ImplicitVar.of(name) : null; + if (implicit != null) { + return getImplicitVar(implicit, true); + } S scope = thisScope(); while (scope != null) { V var = scope.getOwnSlot(name); @@ -173,21 +178,30 @@ public V getVar(String name) { } /** - * Get a unique VAR object to represent "arguments" within this scope + * Get a unique Var object to represent "arguments" within this scope */ public final V getArgumentsVar() { - if (isGlobal() || isModuleScope()) { - return null; - } - - if (!isFunctionScope() || rootNode.isArrowFunction()) { - return getParent().getArgumentsVar(); - } + return getImplicitVar(ImplicitVar.ARGUMENTS, false); + } - if (arguments == null) { - arguments = makeArgumentsVar(); + private final V getImplicitVar(ImplicitVar var, boolean allowDeclaredVars) { + S scope = thisScope(); + while (scope != null) { + if (var.isMadeByScope(scope)) { + V result = ((AbstractScope) scope).implicitVars.get(var); + if (result == null) { + ((AbstractScope) scope).implicitVars.put(var, result = scope.makeImplicitVar(var)); + } + return result; + } + V result = allowDeclaredVars ? scope.getOwnSlot(var.name) : null; + if (result != null) { + return result; + } + // Recurse up the parent Scope + scope = scope.getParent(); } - return arguments; + return null; } /** @@ -372,4 +386,44 @@ void checkRootScope() { checkArgument( NodeUtil.createsScope(rootNode) || rootNode.isScript() || rootNode.isRoot(), rootNode); } + + /** + * The three implicit var types, which are defined implicitly (at least) in + * every vanilla function scope without actually being declared. + */ + enum ImplicitVar { + ARGUMENTS("arguments"), + SUPER("super"), + // TODO(sdh): Expand THIS.isMadeByScope to check super.isMadeByScope(scope) || scope.isGlobal() + // Currently this causes a number of problems (see b/74980936), but could eventually lead to + // better type information. We might also want to restrict this so that module-root scopes + // explicitly *don't* have access to the global this, though I think this is more than just + // returning false in isMadeByScope - rather, getVar() needs to stop checking and immediately + // return null. + THIS("this"); + + final String name; + + ImplicitVar(String name) { + this.name = name; + } + + /** Whether this kind of implicit variable is created/owned by the given scope. */ + boolean isMadeByScope(AbstractScope scope) { + return NodeUtil.isVanillaFunction(scope.getRootNode()); + } + + static ImplicitVar of(String name) { + switch (name) { + case "arguments": + return ARGUMENTS; + case "super": + return SUPER; + case "this": + return THIS; + default: + return null; + } + } + } } diff --git a/src/com/google/javascript/jscomp/AbstractVar.java b/src/com/google/javascript/jscomp/AbstractVar.java index 7f0939f2516..b544d82080e 100644 --- a/src/com/google/javascript/jscomp/AbstractVar.java +++ b/src/com/google/javascript/jscomp/AbstractVar.java @@ -16,6 +16,8 @@ package com.google.javascript.jscomp; +import static com.google.common.base.Preconditions.checkState; + import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.javascript.rhino.JSDocInfo; @@ -78,7 +80,7 @@ final CompilerInput getInput() { @Override public StaticSourceFile getSourceFile() { - return nameNode.getStaticSourceFile(); + return (nameNode != null ? nameNode : scope.getRootNode()).getStaticSourceFile(); } @Override @@ -100,7 +102,8 @@ public final Node getParentNode() { * that bleeds into the inner scope). */ public boolean isBleedingFunction() { - return NodeUtil.isFunctionExpression(getParentNode()); + Node parent = getParentNode(); + return parent != null && NodeUtil.isFunctionExpression(parent); } public final S getScope() { @@ -191,8 +194,17 @@ final boolean isImport() { return declarationType() == Token.IMPORT; } - public boolean isArguments() { - return false; + public final boolean isArguments() { + return Var.ARGUMENTS.equals(name) && scope.isFunctionScope(); + } + + public final boolean isThis() { + return "this".equals(name) && scope.isFunctionScope(); + } + + private boolean isImplicit() { + AbstractScope.ImplicitVar var = AbstractScope.ImplicitVar.of(name); + return var != null && var.isMadeByScope(scope); } private static final ImmutableSet DECLARATION_TYPES = Sets.immutableEnumSet( @@ -212,8 +224,10 @@ protected Token declarationType() { return current.getToken(); } } - throw new IllegalStateException("The nameNode for " + this + " must be a descendant" - + " of one of: " + DECLARATION_TYPES); + checkState( + isImplicit(), + "The nameNode for %s must be a descendant of one of: %s", this, DECLARATION_TYPES); + return null; } // This is safe because any concrete subclass of AbstractVar should be assignable to V. diff --git a/src/com/google/javascript/jscomp/Scope.java b/src/com/google/javascript/jscomp/Scope.java index 194c978c687..0b6d0d3b93c 100644 --- a/src/com/google/javascript/jscomp/Scope.java +++ b/src/com/google/javascript/jscomp/Scope.java @@ -66,8 +66,8 @@ Var declare(String name, Node nameNode, CompilerInput input) { } @Override - Var makeArgumentsVar() { - return Var.makeArgumentsVar(this); + Var makeImplicitVar(ImplicitVar var) { + return new Var(var.name, null /* nameNode */, this, -1 /* index */, null /* input */); } private static final class Simple extends Scope { diff --git a/src/com/google/javascript/jscomp/TypeInference.java b/src/com/google/javascript/jscomp/TypeInference.java index e44c5ef2b05..af4a528d035 100644 --- a/src/com/google/javascript/jscomp/TypeInference.java +++ b/src/com/google/javascript/jscomp/TypeInference.java @@ -622,8 +622,11 @@ private void updateScopeForTypeChange( String varName = left.getString(); TypedVar var = syntacticScope.getVar(varName); JSType varType = var == null ? null : var.getType(); - boolean isVarDeclaration = left.hasChildren() - && varType != null && !var.isTypeInferred(); + boolean isVarDeclaration = + left.hasChildren() + && varType != null + && !var.isTypeInferred() + && var.getNameNode() != null; boolean isTypelessConstDecl = isVarDeclaration diff --git a/src/com/google/javascript/jscomp/TypedScope.java b/src/com/google/javascript/jscomp/TypedScope.java index 4f1d851e8bf..c25a6f8e3a2 100644 --- a/src/com/google/javascript/jscomp/TypedScope.java +++ b/src/com/google/javascript/jscomp/TypedScope.java @@ -113,6 +113,8 @@ public TypedScope getParent() { public JSType getTypeOfThis() { if (isGlobal()) { return ObjectType.cast(getRootNode().getJSType()); + } else if (!getRootNode().isFunction()) { + return getClosestNonBlockScope().getTypeOfThis(); } checkState(getRootNode().isFunction()); @@ -138,8 +140,22 @@ TypedVar declare(String name, Node nameNode, } @Override - TypedVar makeArgumentsVar() { - return TypedVar.makeArguments(this); + TypedVar makeImplicitVar(ImplicitVar var) { + return new TypedVar(false, var.name, null, getImplicitVarType(var), this, -1, null); + } + + private JSType getImplicitVarType(ImplicitVar var) { + if (var == ImplicitVar.ARGUMENTS) { + // Look for an extern named "arguments" and use its type if available. + // TODO(sdh): consider looking for "Arguments" ctor rather than "arguments" var: this could + // allow deleting the variable, which doesn't really belong in externs in the first place. + TypedVar globalArgs = getGlobalScope().getVar(Var.ARGUMENTS); + return globalArgs != null && globalArgs.isExtern() + ? globalArgs.getType() + : null; + } + // TODO(sdh): get the superclass for super? + return getTypeOfThis(); } public Iterable getDeclarativelyUnboundVarsWithoutTypes() { @@ -171,7 +187,6 @@ void setTypeResolver(TypeResolver resolver) { this.typeResolver = resolver; } - @SuppressWarnings("unchecked") @Override public JSType getNamespaceOrTypedefType(String typeName) { StaticTypedSlot slot = getSlot(typeName); diff --git a/src/com/google/javascript/jscomp/TypedScopeCreator.java b/src/com/google/javascript/jscomp/TypedScopeCreator.java index 2167d9df030..fb0c517fbaa 100644 --- a/src/com/google/javascript/jscomp/TypedScopeCreator.java +++ b/src/com/google/javascript/jscomp/TypedScopeCreator.java @@ -1012,7 +1012,11 @@ private FunctionType createFunctionTypeFromNodes( ObjectType ownerType = null; if (ownerName != null) { ownerVar = scope.getVar(ownerName); - if (ownerVar != null) { + // TODO(sdh): Remove the !isThis() check. Historically scopes returned + // null for getVar("this"), but they now return a meaningful variable, + // which exposes a lot of type errors that need to be fixed before this + // restriction can be removed: b/74980936. + if (ownerVar != null && !ownerVar.isThis()) { ownerType = ObjectType.cast(ownerVar.getType()); } if (name != null) { diff --git a/src/com/google/javascript/jscomp/TypedVar.java b/src/com/google/javascript/jscomp/TypedVar.java index bae3d4af6aa..ee4bb87692b 100644 --- a/src/com/google/javascript/jscomp/TypedVar.java +++ b/src/com/google/javascript/jscomp/TypedVar.java @@ -18,8 +18,6 @@ import com.google.javascript.rhino.ErrorReporter; import com.google.javascript.rhino.Node; -import com.google.javascript.rhino.StaticSourceFile; -import com.google.javascript.rhino.Token; import com.google.javascript.rhino.jstype.JSType; import com.google.javascript.rhino.jstype.StaticTypedRef; import com.google.javascript.rhino.jstype.StaticTypedSlot; @@ -113,39 +111,4 @@ void markAssignedExactlyOnce() { boolean isMarkedAssignedExactlyOnce() { return markedAssignedExactlyOnce; } - - static TypedVar makeArguments(TypedScope scope) { - // Look for an extern named "arguments" and use its type if available. - // TODO(sdh): consider looking for "Arguments" ctor rather than "arguments" var: this could - // allow deleting the variable, which doesn't really belong in externs in the first place. - TypedVar globalArgs = scope.getGlobalScope().getVar(Var.ARGUMENTS); - JSType type = globalArgs != null && globalArgs.isExtern() ? globalArgs.getType() : null; - return new TypedArguments(type, scope); - } - - private static final class TypedArguments extends TypedVar { - TypedArguments(JSType type, TypedScope scope) { - super(false, Var.ARGUMENTS, null, type, scope, -1, null); - } - - @Override - public boolean isArguments() { - return true; - } - - @Override - public StaticSourceFile getSourceFile() { - return scope.getRootNode().getStaticSourceFile(); - } - - @Override - public boolean isBleedingFunction() { - return false; - } - - @Override - protected Token declarationType() { - return null; - } - } } diff --git a/src/com/google/javascript/jscomp/Var.java b/src/com/google/javascript/jscomp/Var.java index 3f74253fe85..00acd7535c4 100644 --- a/src/com/google/javascript/jscomp/Var.java +++ b/src/com/google/javascript/jscomp/Var.java @@ -19,8 +19,6 @@ import com.google.javascript.rhino.Node; import com.google.javascript.rhino.StaticRef; import com.google.javascript.rhino.StaticSlot; -import com.google.javascript.rhino.StaticSourceFile; -import com.google.javascript.rhino.Token; /** * Used by {@code Scope} to store information about variables. @@ -33,48 +31,8 @@ public class Var extends AbstractVar implements StaticSlot, StaticRe super(name, nameNode, scope, index, input); } - static Var makeArgumentsVar(Scope scope) { - return new Arguments(scope); - } - @Override public String toString() { return "Var " + name + " @ " + nameNode; } - - /** - * A special subclass of Var used to distinguish "arguments" in the current - * scope. - */ - private static class Arguments extends Var { - Arguments(Scope scope) { - super( - ARGUMENTS, // always arguments - null, // no declaration node - scope, - -1, // no variable index - null // input - ); - } - - @Override - public boolean isArguments() { - return true; - } - - @Override - public StaticSourceFile getSourceFile() { - return scope.getRootNode().getStaticSourceFile(); - } - - @Override - public boolean isBleedingFunction() { - return false; - } - - @Override - protected Token declarationType() { - return null; - } - } } diff --git a/test/com/google/javascript/jscomp/Es6SyntacticScopeCreatorTest.java b/test/com/google/javascript/jscomp/Es6SyntacticScopeCreatorTest.java index 6f551dd9325..a2f0faad16a 100644 --- a/test/com/google/javascript/jscomp/Es6SyntacticScopeCreatorTest.java +++ b/test/com/google/javascript/jscomp/Es6SyntacticScopeCreatorTest.java @@ -787,6 +787,58 @@ public void testArgumentsVariableInArrowFunction() { assertThat(innerFunctionScope.getArgumentsVar()).isSameAs(arguments); } + public void testTheThisVariable() { + String js = "function f() { if (true) { function g() {} } }"; + Node root = getRoot(js); + Scope global = scopeCreator.createScope(root, null); + + Node function = root.getFirstChild(); + checkState(function.isFunction(), function); + Scope fScope = scopeCreator.createScope(function, global); + assertFalse(fScope.isDeclared("this", true)); + Var thisVar = fScope.getVar("this"); + assertTrue(thisVar.isThis()); + + Node fBlock = NodeUtil.getFunctionBody(function); + Scope fBlockScope = scopeCreator.createScope(fBlock, fScope); + assertFalse(fBlockScope.isDeclared("this", true)); + assertThat(fBlockScope.getVar("this")).isSameAs(thisVar); + + Node ifBlock = fBlock.getFirstChild().getLastChild(); + Scope blockScope = scopeCreator.createScope(ifBlock, fBlockScope); + assertFalse(blockScope.isDeclared("this", true)); + assertThat(blockScope.getVar("this")).isSameAs(thisVar); + assertThat(blockScope.getVar("this").getScope()).isSameAs(fScope); + + Node gFunction = ifBlock.getFirstChild(); + Scope gScope = scopeCreator.createScope(gFunction, blockScope); + assertFalse(gScope.isDeclared("this", true)); + assertThat(gScope.getVar("this").getScope()).isSameAs(gScope); + } + + public void testTheThisVariableInArrowFunction() { + String js = "function outer() { var inner = () => this.x; }"; + Node root = getRoot(js); + Scope global = scopeCreator.createScope(root, null); + + Node outer = root.getFirstChild(); + checkState(outer.isFunction(), outer); + checkState(!outer.isArrowFunction(), outer); + Scope outerFunctionScope = scopeCreator.createScope(outer, global); + Var thisVar = outerFunctionScope.getVar("this"); + + Node outerBody = NodeUtil.getFunctionBody(outer); + Scope outerBodyScope = scopeCreator.createScope(outerBody, outerFunctionScope); + + Node inner = outerBody.getFirstChild() // VAR + .getFirstChild() // NAME + .getFirstChild(); // FUNCTION + checkState(inner.isFunction(), inner); + checkState(inner.isArrowFunction(), inner); + Scope innerFunctionScope = scopeCreator.createScope(inner, outerBodyScope); + assertThat(innerFunctionScope.getVar("this")).isSameAs(thisVar); + } + public void testIsFunctionBlockScoped() { String js = "if (true) { function f() {}; }"; Node root = getRoot(js);