Streamline special handling in Var.Arguments to also handle 'this' an…

…d 'super'.

Adds an AbstractScope.ImplicitVar enum that includes the three special vars that are implicitly created in different scopes.  The enum provides a method to know whether any given scope creates the var, and is integrated into the various Scope.getVar() methods to do the right thing.

This is a step toward supporting ES6 block scopes in LinkedFlowScope, since we need to be more careful exactly which scope to declare qualified names on (there's no longer just the function scope, but the various block scopes in between).  The general approach is to look for the root of the qualified name and this streamlines the case where the root is 'this' or 'super' to not need any special handling.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=189380979

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<S extends AbstractScope<S, V>, V extends AbstractVar<S, V>>
     implements StaticScope, Serializable {
   private final Map<String, V> vars = new LinkedHashMap<>();
+  private final Map<ImplicitVar, V> 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<S, V>) scope).implicitVars.get(var);
+        if (result == null) {
+          ((AbstractScope<S, V>) 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;
+      }
+    }
+  }
 }

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<Token> 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<V> should be assignable to V.

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 {

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

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<TypedVar> getDeclarativelyUnboundVarsWithoutTypes() {
@@ -171,7 +187,6 @@ void setTypeResolver(TypeResolver resolver) {
     this.typeResolver = resolver;
   }
 
-  @SuppressWarnings("unchecked")
   @Override
   public JSType getNamespaceOrTypedefType(String typeName) {
     StaticTypedSlot<JSType> slot = getSlot(typeName);

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) {

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;
-    }
-  }
 }

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<Scope, Var> 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;
-    }
-  }
 }