Run the checks from ClosureCheckModule inside bundled goog.modules.

In addition, create a new callback type for traversing module scopes,
including both file-level modules and bundled goog.modules.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=125824641

src/com/google/javascript/jscomp/ClosureCheckModule.java

@@ -16,7 +16,7 @@
 package com.google.javascript.jscomp;
 
 import com.google.common.base.Preconditions;
-import com.google.javascript.jscomp.NodeTraversal.Callback;
+import com.google.javascript.jscomp.NodeTraversal.AbstractModuleCallback;
 import com.google.javascript.rhino.JSDocInfo;
 import com.google.javascript.rhino.Node;
 import com.google.javascript.rhino.Token;
@@ -27,10 +27,11 @@
 /**
  * Checks that goog.module() is used correctly.
  *
- * Note that this file only does checks that can be done per-file. Whole program
- * checks happen during goog.module rewriting, in {@link ClosureRewriteModule}.
+ * <p>Note that this file only does checks that can be done per-file. Whole program checks happen
+ * during goog.module rewriting, in {@link ClosureRewriteModule}.
  */
-public final class ClosureCheckModule implements Callback, HotSwapCompilerPass {
+public final class ClosureCheckModule extends AbstractModuleCallback
+    implements HotSwapCompilerPass {
   static final DiagnosticType AT_EXPORT_IN_GOOG_MODULE =
       DiagnosticType.error(
           "JSC_AT_EXPORT_IN_GOOG_MODULE",
@@ -122,28 +123,36 @@ public void hotSwapScript(Node scriptRoot, Node originalRoot) {
   }
 
   @Override
-  public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
-    if (n.isScript()) {
-      if (NodeUtil.isGoogModuleFile(n)) {
-        n.putBooleanProp(Node.GOOG_MODULE, true);
-        return true;
+  public void enterModule(NodeTraversal t, Node scopeRoot) {
+    Node firstStatement = scopeRoot.getFirstChild();
+    if (NodeUtil.isExprCall(firstStatement)) {
+      Node call = firstStatement.getFirstChild();
+      Node callee = call.getFirstChild();
+      if (callee.matchesQualifiedName("goog.module")) {
+        Preconditions.checkState(currentModuleName == null);
+        currentModuleName = extractFirstArgumentName(call);
       }
-      return false;
     }
-    return true;
+  }
+
+  @Override
+  public void exitModule(NodeTraversal t, Node scopeRoot) {
+    currentModuleName = null;
+    shortRequiredNamespaces.clear();
+    defaultExportNode = null;
   }
 
   @Override
   public void visit(NodeTraversal t, Node n, Node parent) {
+    if (currentModuleName == null) {
+      return;
+    }
     switch (n.getType()) {
       case CALL:
         Node callee = n.getFirstChild();
-        if (callee.matchesQualifiedName("goog.module")) {
-          if (currentModuleName == null) {
-            currentModuleName = extractFirstArgumentName(n);
-          } else {
-            t.report(n, MULTIPLE_MODULES_IN_FILE);
-          }
+        if (callee.matchesQualifiedName("goog.module")
+            && !currentModuleName.equals(extractFirstArgumentName(n))) {
+          t.report(n, MULTIPLE_MODULES_IN_FILE);
         } else if (callee.matchesQualifiedName("goog.provide")) {
           t.report(n, MODULE_AND_PROVIDES);
         } else if (callee.matchesQualifiedName("goog.require")) {
@@ -198,11 +207,6 @@ public void visit(NodeTraversal t, Node n, Node parent) {
           }
         }
         break;
-      case SCRIPT:
-        currentModuleName = null;
-        shortRequiredNamespaces.clear();
-        defaultExportNode = null;
-        break;
     }
   }
 

src/com/google/javascript/jscomp/ClosureRewriteModule.java

@@ -251,6 +251,7 @@ private class ScriptRecorder extends AbstractPreOrderCallback {
     @Override
     public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
       if (NodeUtil.isGoogModuleFile(n)) {
+        n.putBooleanProp(Node.GOOG_MODULE, true);
         inlineModuleIntoGlobal(n);
         compiler.reportCodeChange();
         checkAndSetStrictModeDirective(t, n);

src/com/google/javascript/jscomp/NodeTraversal.java

@@ -150,14 +150,10 @@ public abstract static class AbstractPreOrderCallback implements Callback {
     public void visit(NodeTraversal t, Node n, Node parent) {}
   }
 
-  /**
-   * Abstract scoped callback to visit all nodes in postorder.
-   */
-  public abstract static class AbstractScopedCallback
-      implements ScopedCallback {
+  /** Abstract scoped callback to visit all nodes in postorder. */
+  public abstract static class AbstractScopedCallback implements ScopedCallback {
     @Override
-    public final boolean shouldTraverse(NodeTraversal nodeTraversal, Node n,
-        Node parent) {
+    public final boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent) {
       return true;
     }
 
@@ -174,26 +170,61 @@ public void exitScope(NodeTraversal t) {}
    */
   public abstract static class AbstractShallowCallback implements Callback {
     @Override
-    public final boolean shouldTraverse(NodeTraversal nodeTraversal, Node n,
-        Node parent) {
+    public final boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent) {
       // We do want to traverse the name of a named function, but we don't
       // want to traverse the arguments or body.
-      return parent == null || !parent.isFunction() ||
-          n == parent.getFirstChild();
+      return parent == null || !parent.isFunction() || n == parent.getFirstChild();
     }
   }
 
   /**
-   * Abstract callback to visit all structure and statement nodes but doesn't
-   * traverse into functions or expressions.
+   * Abstract callback to visit all structure and statement nodes but doesn't traverse into
+   * functions or expressions.
    */
-  public abstract static class AbstractShallowStatementCallback
-      implements Callback {
+  public abstract static class AbstractShallowStatementCallback implements Callback {
     @Override
-    public final boolean shouldTraverse(NodeTraversal nodeTraversal, Node n,
-        Node parent) {
-      return parent == null || NodeUtil.isControlStructure(parent)
-         || NodeUtil.isStatementBlock(parent);
+    public final boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent) {
+      return parent == null
+          || NodeUtil.isControlStructure(parent)
+          || NodeUtil.isStatementBlock(parent);
+    }
+  }
+
+  /**
+   * Abstract callback that knows when goog.modules (and in the future ES6 modules) are entered
+   * and exited. This includes both whole file modules and bundled modules.
+   */
+  public abstract static class AbstractModuleCallback implements ScopedCallback {
+
+    /**
+     * Called immediately after entering a module.
+     */
+    public abstract void enterModule(NodeTraversal t, Node scopeRoot);
+
+    /**
+     * Called immediately before exiting a module.
+     */
+    public abstract void exitModule(NodeTraversal t, Node scopeRoot);
+
+    @Override
+    public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
+      return true;
+    }
+
+    @Override
+    public final void enterScope(NodeTraversal t) {
+      Node scopeRoot = t.getScopeRoot();
+      if (NodeUtil.isModuleScopeRoot(scopeRoot)) {
+        enterModule(t, scopeRoot);
+      }
+    }
+
+    @Override
+    public final void exitScope(NodeTraversal t) {
+      Node scopeRoot = t.getScopeRoot();
+      if (NodeUtil.isModuleScopeRoot(scopeRoot)) {
+        exitModule(t, scopeRoot);
+      }
     }
   }
 
@@ -863,14 +894,19 @@ public boolean inGlobalHoistScope() {
    * in a global block scope.
    */
   public boolean inModuleScope() {
-    return getScopeRoot().isModuleBody();
+    return NodeUtil.isModuleScopeRoot(getScopeRoot());
   }
 
   /**
    * Determines whether the hoist scope of the current traversal is global.
    */
   public boolean inModuleHoistScope() {
-    return getCfgRoot().isModuleBody();
+    Node moduleRoot = getCfgRoot();
+    if (moduleRoot.isFunction()) {
+      // For wrapped modules, the function block is the module scope root.
+      moduleRoot = moduleRoot.getLastChild();
+    }
+    return NodeUtil.isModuleScopeRoot(moduleRoot);
   }
 
   int getScopeDepth() {

src/com/google/javascript/jscomp/NodeUtil.java

@@ -4491,6 +4491,30 @@ private static boolean isGoogModuleCall(Node n) {
     return false;
   }
 
+  static boolean isModuleScopeRoot(Node n) {
+    return n.isModuleBody() || isBundledGoogModuleScopeRoot(n);
+  }
+
+  private static boolean isBundledGoogModuleScopeRoot(Node n) {
+    if (!n.isBlock() || !n.hasChildren() || !isGoogModuleCall(n.getFirstChild())) {
+      return false;
+    }
+    Node function = n.getParent();
+    if (function == null
+        || !function.isFunction()
+        || getFunctionParameters(function).getChildCount() != 1
+        || !getFunctionParameters(function).getFirstChild().matchesQualifiedName("exports")) {
+      return false;
+    }
+    Node call = function.getParent();
+    if (!call.isCall()
+        || call.getChildCount() != 2
+        || !call.getFirstChild().matchesQualifiedName("goog.loadModule")) {
+      return false;
+    }
+    return call.getParent().isExprResult() && call.getGrandparent().isScript();
+  }
+
   private static boolean isGoogModuleDeclareLegacyNamespaceCall(Node n) {
     if (isExprCall(n)) {
       Node target = n.getFirstFirstChild();

test/com/google/javascript/jscomp/ClosureCheckModuleTest.java

@@ -97,6 +97,55 @@ public void testMultipleGoogModules() {
         MULTIPLE_MODULES_IN_FILE);
   }
 
+  public void testBundledGoogModules() {
+    testError(
+        LINE_JOINER.join(
+            "goog.loadModule(function(exports){",
+            "  'use strict';",
+            "  goog.module('xyz');",
+            "  foo.call(this, 1, 2, 3);",
+            "  return exports;",
+            "});"),
+        GOOG_MODULE_REFERENCES_THIS);
+
+    testError(
+        LINE_JOINER.join(
+            "goog.loadModule(function(exports){",
+            "  'use strict';",
+            "  goog.module('foo.example.ClassName');",
+            "  /** @constructor @export */ function ClassName() {}",
+            "  exports = ClassName;",
+            "  return exports;",
+            "});"),
+        ClosureCheckModule.AT_EXPORT_IN_GOOG_MODULE);
+
+    testSameEs6(
+        LINE_JOINER.join(
+            "goog.loadModule(function(exports){",
+            "  'use strict';",
+            "  goog.module('xyz');",
+            "  exports = class {}",
+            "  return exports;",
+            "});",
+            "goog.loadModule(function(exports){",
+            "  goog.module('abc');",
+            "  var Foo = goog.require('xyz');",
+            "  var x = new Foo;",
+            "  return exports;",
+            "});"));
+
+    testError(
+        LINE_JOINER.join(
+            "goog.loadModule(function(exports){",
+            "  'use strict';",
+            "  goog.module('xyz');",
+            "  goog.module('abc');",
+            "  var x = goog.require('other.x');",
+            "  return exports;",
+            "});"),
+        MULTIPLE_MODULES_IN_FILE);
+  }
+
   public void testGoogModuleReferencesGlobalName() {
     testError("goog.module('x.y.z');\nx.y.z = function() {};", REFERENCE_TO_MODULE_GLOBAL_NAME);
   }