Add checks for more invalid use of goog.module

These include: * There should be only one export of a given name per module. * Export statement (`exports = value`) should be at the module top-level ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123441641
google · May 27, 2016 · 85879c9 · 85879c9
1 parent 620d39c
commit 85879c9
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 10 deletions.
diff --git a/src/com/google/javascript/jscomp/ClosureCheckModule.java b/src/com/google/javascript/jscomp/ClosureCheckModule.java
@@ -70,6 +70,16 @@ public final class ClosureCheckModule implements Callback, HotSwapCompilerPass {
           "JSC_ONE_REQUIRE_PER_DECLARATION",
           "There may only be one goog.require() per var/let/const declaration.");
 
+  static final DiagnosticType EXPORT_NOT_A_MODULE_LEVEL_STATEMENT =
+      DiagnosticType.error(
+          "JSC_EXPORT_NOT_A_MODULE_LEVEL_STATEMENT",
+          "Exports must be a statement at the top-level of a module");
+
+  static final DiagnosticType EXPORT_REPEATED_ERROR =
+      DiagnosticType.error(
+          "JSC_EXPORT_REPEATED_ERROR",
+          "Name cannot be exported multiple times. Previous export on line {0}.");
+
   static final DiagnosticType REFERENCE_TO_MODULE_GLOBAL_NAME =
       DiagnosticType.error(
           "JSC_REFERENCE_TO_MODULE_GLOBAL_NAME",
@@ -95,6 +105,7 @@ public final class ClosureCheckModule implements Callback, HotSwapCompilerPass {
 
   private String currentModuleName = null;
   private Map<String, String> shortRequiredNamespaces = new HashMap<>();
+  private Node defaultExportNode = null;
 
   public ClosureCheckModule(AbstractCompiler compiler) {
     this.compiler = compiler;
@@ -141,16 +152,9 @@ public void visit(NodeTraversal t, Node n, Node parent) {
         break;
       case Token.ASSIGN: {
         Node lhs = n.getFirstChild();
-        if (lhs.isQualifiedName()) {
-          Node root = NodeUtil.getRootOfQualifiedName(lhs);
-          if (root.matchesQualifiedName("exports")
-              && (lhs.isName() || !root.getNext().getString().equals("prototype"))
-              && !NodeUtil.isLegacyGoogModuleFile(NodeUtil.getEnclosingScript(n))) {
-            JSDocInfo jsDoc = n.getJSDocInfo();
-            if (jsDoc != null && jsDoc.isExport()) {
-              t.report(n, AT_EXPORT_IN_NON_LEGACY_GOOG_MODULE);
-            }
-          }
+        if (lhs.isQualifiedName()
+            && NodeUtil.getRootOfQualifiedName(lhs).matchesQualifiedName("exports")) {
+          checkModuleExport(t, n, parent);
         }
         break;
       }
@@ -197,10 +201,35 @@ public void visit(NodeTraversal t, Node n, Node parent) {
       case Token.SCRIPT:
         currentModuleName = null;
         shortRequiredNamespaces.clear();
+        defaultExportNode = null;
         break;
     }
   }
 
+  private void checkModuleExport(NodeTraversal t, Node n, Node parent) {
+    Preconditions.checkArgument(n.isAssign());
+    Node lhs = n.getFirstChild();
+    Preconditions.checkState(lhs.isQualifiedName());
+    Preconditions.checkState(NodeUtil.getRootOfQualifiedName(lhs).matchesQualifiedName("exports"));
+    if (lhs.isName()) {
+      if  (defaultExportNode != null) {
+        // Multiple exports
+        t.report(n, EXPORT_REPEATED_ERROR, String.valueOf(defaultExportNode.getLineno()));
+      } else if (!t.inGlobalScope() || !parent.isExprResult()) {
+        // Invalid export location.
+        t.report(n, EXPORT_NOT_A_MODULE_LEVEL_STATEMENT);
+      }
+      defaultExportNode = lhs;
+    }
+    if ((lhs.isName() || !NodeUtil.isPrototypeProperty(lhs))
+        && !NodeUtil.isLegacyGoogModuleFile(NodeUtil.getEnclosingScript(n))) {
+      JSDocInfo jsDoc = n.getJSDocInfo();
+      if (jsDoc != null && jsDoc.isExport()) {
+        t.report(n, AT_EXPORT_IN_NON_LEGACY_GOOG_MODULE);
+      }
+    }
+  }
+
   private String extractFirstArgumentName(Node callNode) {
     Node firstArg = callNode.getSecondChild();
     if (firstArg != null && firstArg.isString()) {

diff --git a/test/com/google/javascript/jscomp/ClosureCheckModuleTest.java b/test/com/google/javascript/jscomp/ClosureCheckModuleTest.java
@@ -15,6 +15,8 @@
  */
 package com.google.javascript.jscomp;
 
+import static com.google.javascript.jscomp.ClosureCheckModule.EXPORT_NOT_A_MODULE_LEVEL_STATEMENT;
+import static com.google.javascript.jscomp.ClosureCheckModule.EXPORT_REPEATED_ERROR;
 import static com.google.javascript.jscomp.ClosureCheckModule.GOOG_MODULE_REFERENCES_THIS;
 import static com.google.javascript.jscomp.ClosureCheckModule.GOOG_MODULE_USES_THROW;
 import static com.google.javascript.jscomp.ClosureCheckModule.LET_GOOG_REQUIRE;
@@ -309,4 +311,28 @@ public void testLegalGoogRequires() {
             "",
             "const {assert, fail} = goog.require('goog.asserts');"));
   }
+
+  public void testIllegalExports() {
+    testError(
+        LINE_JOINER.join(
+            "goog.module('xyz');",
+            "",
+            "if (window.exportMe) { exports = 5; }"),
+            EXPORT_NOT_A_MODULE_LEVEL_STATEMENT);
+
+    testError(
+        LINE_JOINER.join(
+            "goog.module('xyz');",
+            "",
+            "window.exportMe && (exports = 5);"),
+            EXPORT_NOT_A_MODULE_LEVEL_STATEMENT);
+
+    testError(
+        LINE_JOINER.join(
+            "goog.module('xyz');",
+            "",
+            "exports = 5;",
+            "exports = 'str';"),
+            EXPORT_REPEATED_ERROR);
+  }
 }