Rollforward "Recognize goog.modules with named exports and destructur…

…ing requires in type inference" NEW: ClosureModuleProcessor handles having named exports in both an object literal and regular assignment "exports.x = 0" ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242024407
google · Apr 5, 2019 · a7204e5 · a7204e5
1 parent 1485aa3
commit a7204e5
Show file tree

Hide file tree

Showing 4 changed files with 255 additions and 22 deletions.
diff --git a/src/com/google/javascript/jscomp/TypedScopeCreator.java b/src/com/google/javascript/jscomp/TypedScopeCreator.java
@@ -62,6 +62,9 @@
 import com.google.javascript.jscomp.FunctionTypeBuilder.AstFunctionContents;
 import com.google.javascript.jscomp.NodeTraversal.AbstractScopedCallback;
 import com.google.javascript.jscomp.NodeTraversal.AbstractShallowStatementCallback;
+import com.google.javascript.jscomp.modules.Binding;
+import com.google.javascript.jscomp.modules.Export;
+import com.google.javascript.jscomp.modules.Module;
 import com.google.javascript.jscomp.modules.ModuleMap;
 import com.google.javascript.rhino.ErrorReporter;
 import com.google.javascript.rhino.InputId;
@@ -203,6 +206,12 @@ final class TypedScopeCreator implements ScopeCreator, StaticSymbolTable<TypedVa
 
   private final List<DeferredSetType> deferredSetTypes = new ArrayList<>();
 
+  // Set of NAME, GETPROP, and STRING_KEY lvalues which should be treated as const declarations when
+  // assigned. Treat simple names in this list as if they were declared `const`. E.g. treat `exports
+  // = class {};` as `const exports = class {};`. Treat GETPROP and STRING_KEY nodes as if they were
+  // annotated @const.
+  private final Set<Node> undeclaredNamesForClosure = new HashSet<>();
+
   /**
    * Defer attachment of types to nodes until all type names
    * have been resolved. Then, we can resolve the type and attach it.
@@ -279,7 +288,7 @@ public Iterable<TypedVar> getAllSymbols() {
   /**
    * Returns a function mapping a scope root node to a {@link TypedScope}.
    *
-   * <p>This method mostly exists in liu of an interface represnting root node -> scope.
+   * <p>This method mostly exists in lieu of an interface representing root node -> scope.
    */
   public Function<Node, TypedScope> getNodeToScopeMapper() {
     return memoized::get;
@@ -352,6 +361,10 @@ private TypedScope createScopeInternal(Node root, TypedScope typedParent) {
     } else {
       newScope = new TypedScope(typedParent, root);
     }
+
+    if (root.isModuleBody()) {
+      initializeModuleScope(root, newScope);
+    }
     if (root.isFunction()) {
       scopeBuilder = new FunctionScopeBuilder(newScope);
     } else if (root.isClass()) {
@@ -374,6 +387,60 @@ private TypedScope createScopeInternal(Node root, TypedScope typedParent) {
     return newScope;
   }
 
+  /** Builds the beginning of a module-scope. This can be an ES module or a goog.module. */
+  private void initializeModuleScope(Node moduleBody, TypedScope moduleScope) {
+    Node scriptNode = moduleBody.getParent();
+
+    if (scriptNode.getBooleanProp(Node.GOOG_MODULE)) {
+      Node googModuleCall = moduleBody.getFirstChild();
+      Node namespace = googModuleCall.getFirstChild().getSecondChild();
+
+      String closureModuleNamespace = namespace.getString();
+      Module module = moduleMap.getClosureModule(closureModuleNamespace);
+
+      declareExportsInModuleScope(module, moduleBody, moduleScope);
+      markGoogModuleExportsAsConst(module);
+    }
+  }
+
+  /**
+   * Ensures that the name `exports` is declared in goog.module scope.
+   *
+   * <p>If a goog.module explicitly assigns to exports, we want to treat that assignment inside the
+   * scope as if it were a declaration: `const exports = ...`. This method only handles cases where
+   * we want to treat exports as implicitly declared.
+   */
+  private void declareExportsInModuleScope(
+      Module googModule, Node moduleBody, TypedScope moduleScope) {
+    if (!googModule.namespace().containsKey(Export.NAMESPACE)) {
+      // The goog.module never assigns `exports = ...`, so declare `exports` as an object literal.
+      moduleScope.declare(
+          "exports",
+          googModule.metadata().rootNode(),
+          typeRegistry.createAnonymousObjectType(null),
+          compiler.getInput(moduleBody.getInputId()),
+          /* inferred= */ false);
+    }
+  }
+
+  /**
+   * Adds nodes representing goog.module exports to a list, to treat them as @const.
+   *
+   * <p>This method handles the following styles of exports:
+   *
+   * <ul>
+   *   <li>{@code exports = class {}} adds the NAME node `exports`
+   *   <li>{@code exports = {Foo};} adds the NAME node `exports` and the STRING_KEY node `Foo`
+   *   <li>{@code exports.Foo = Foo;} adds the GETPROP node `exports.Foo`
+   * </ul>
+   */
+  private void markGoogModuleExportsAsConst(Module googModule) {
+    for (Binding binding : googModule.namespace().values()) {
+      Node exportedNode = binding.originatingExport().exportNode();
+      undeclaredNamesForClosure.add(exportedNode);
+    }
+  }
+
   /**
    * Patches a given global scope by removing variables previously declared in
    * a script and re-traversing a new version of that script.
@@ -1892,15 +1959,6 @@ && shouldUseFunctionLiteralType(
         }
       }
 
-      // Check if this is a goog dependency loading call. If so, find its type.
-      if (moduleImportResolver.isGoogModuleDependencyCall(rValue)) {
-        TypedVar exportedVar = moduleImportResolver.getClosureNamespaceTypeFromCall(rValue);
-        if (exportedVar != null) {
-          return exportedVar.getType();
-        }
-        return null;
-      }
-
       // Check if this is constant, and if it has a known type.
       if (NodeUtil.isConstantDeclaration(compiler.getCodingConvention(), info, lValue)
           || isGoogModuleExports(lValue)) {
@@ -1998,10 +2056,7 @@ private void maybeDeclareAliasType(
     }
 
     boolean isGoogModuleExports(Node lValue) {
-      // TODO(b/124919359): this could also be an ES module scope, filter out that case
-      return currentScope.isModuleScope()
-          && lValue.isName()
-          && lValue.getString().equals("exports");
+      return currentScope.isModuleScope() && undeclaredNamesForClosure.contains(lValue);
     }
 
     /** Returns the AST node associated with the definition, if any. */
@@ -2030,6 +2085,15 @@ private JSType getDeclaredRValueType(@Nullable Node lValue, Node rValue) {
         return rValueInfo.getType().evaluate(currentScope, typeRegistry);
       }
 
+      // Check if this is a goog dependency loading call. If so, find its type.
+      if (moduleImportResolver.isGoogModuleDependencyCall(rValue)) {
+        TypedVar exportedVar = moduleImportResolver.getClosureNamespaceTypeFromCall(rValue);
+        if (exportedVar != null) {
+          return exportedVar.getType();
+        }
+        return null;
+      }
+
       // Check if the type has already been computed during scope-creation.
       // This is mostly useful for literals like BOOLEAN, NUMBER, STRING, and
       // OBJECT_LITERAL
@@ -2359,6 +2423,11 @@ private boolean isQualifiedNameInferred(
         return false;
       }
 
+      // If this is a typed goog.module export, it's not inferred.
+      if (valueType != null && !valueType.isUnknownType() && isGoogModuleExports(n)) {
+        return false;
+      }
+
       // At this point, we're pretty sure it's inferred, since there's neither
       // useful jsdoc info, nor a useful const or doc'd function RHS.  But
       // there's still one case where it may still not be: if the RHS is a
@@ -2622,7 +2691,7 @@ void visitPostorder(NodeTraversal t, Node n, Node parent) {
           Node firstChild = n.getFirstChild();
           if (firstChild.isGetProp() && firstChild.isQualifiedName()) {
             maybeDeclareQualifiedName(t, n.getJSDocInfo(), firstChild, n, firstChild.getNext());
-          } else if (isGoogModuleExports(firstChild)) {
+          } else if (undeclaredNamesForClosure.contains(firstChild)) {
             defineAssignAsIfDeclaration(n);
           }
           break;

diff --git a/src/com/google/javascript/jscomp/modules/ClosureModuleProcessor.java b/src/com/google/javascript/jscomp/modules/ClosureModuleProcessor.java
@@ -283,8 +283,9 @@ private void maybeInitializeExports(Node assignment) {
         if (isNamedExportsLiteral(rhs)) {
           initializeNamedExportsLiteral(rhs);
         } else {
-          markExportsAssignmentInNamespace(lhs);
+          seenExportsAssignment = true;
         }
+        markExportsAssignmentInNamespace(lhs);
       } else if (lhs.isGetProp()
           && lhs.getFirstChild().isName()
           && lhs.getFirstChild().getString().equals("exports")) {
@@ -310,8 +311,6 @@ private void maybeInitializeExportsStub(Node qname) {
      * the Module namespace if there is an explicit' exports = ...' assignment
      */
     private void markExportsAssignmentInNamespace(Node exportsNode) {
-      seenExportsAssignment = true;
-
       namespace.put(
           Export.NAMESPACE,
           Binding.from(

diff --git a/src/com/google/javascript/jscomp/modules/Export.java b/src/com/google/javascript/jscomp/modules/Export.java
@@ -51,7 +51,7 @@ public abstract class Export {
    * The {@link Export#exportName()} of goog.module default exports, e.g. {@code exports = class
    * {};}
    */
-  static final String NAMESPACE = "*exports*";
+  public static final String NAMESPACE = "*exports*";
 
   // Prevent unwanted subclasses.
   Export() {}