Change CheckClosureImports in preparation for enabling it for goog.pr…

…ovide files - fix crash on `arguments` - make hotswapping look at the entire AST to gather the list of Closure namespaces - loosen the restriction on Closure imports in blocks in the global hoist scope; this matches the intended behavior of ProcessClosureProvidesAndRequires ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=252870275
google · Jun 14, 2019 · f45aba0 · f45aba0
1 parent af525de
commit f45aba0
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 6 deletions.
diff --git a/src/com/google/javascript/jscomp/CheckClosureImports.java b/src/com/google/javascript/jscomp/CheckClosureImports.java
@@ -41,7 +41,7 @@
 import javax.annotation.Nullable;
 
 /**
- * Checks all goog.requries, goog.module.gets, goog.forwardDeclares, and goog.requireTypes in all
+ * Checks all goog.requires, goog.module.gets, goog.forwardDeclares, and goog.requireTypes in all
  * files. This pass is a guard to {@link RewriteClosureImports}.
  */
 final class CheckClosureImports implements HotSwapCompilerPass {
@@ -211,7 +211,7 @@ public void hotSwapScript(Node scriptRoot, Node originalRoot) {
       return;
     }
 
-    NodeTraversal.traverse(compiler, scriptRoot, checker);
+    NodeTraversal.traverse(compiler, scriptRoot.getParent(), checker);
   }
 
   @Override
@@ -287,7 +287,7 @@ private void checkValidImportCodeReference(NodeTraversal t, Node nameNode) {
       }
 
       Node declarationNameNode = var.getNameNode();
-      if (!NodeUtil.isDeclarationLValue(declarationNameNode)) {
+      if (declarationNameNode == null || !NodeUtil.isDeclarationLValue(declarationNameNode)) {
         return;
       }
 
@@ -405,7 +405,7 @@ private void checkImport(
         Node parent,
         ModuleMetadata currentModule,
         ClosureImport importType) {
-      boolean atTopLevelScope = t.inGlobalScope() || t.inModuleScope();
+      boolean atTopLevelScope = t.inGlobalHoistScope() || t.inModuleScope();
       boolean isNonModule = currentModule.isGoogProvide() || currentModule.isScript();
       boolean validAssignment = !isNonModule || parent.isExprResult();
 

diff --git a/test/com/google/javascript/jscomp/CheckClosureImportsTest.java b/test/com/google/javascript/jscomp/CheckClosureImportsTest.java
@@ -228,8 +228,26 @@ public void importInEsModuleScopeIsOk() {
   }
 
   @Test
-  public void importInBlockScopeIsError() {
-    testCommonCase("{ <import>('symbol'); }", INVALID_CLOSURE_CALL_SCOPE_ERROR);
+  public void importInBlockScopeInGlobalHoistScopeIsOk() {
+    testCommonCase("{ <import>('symbol'); }");
+  }
+
+  @Test
+  public void importInBlockScopeInModuleHoistScopeIsError() {
+    moduleType = ModuleType.GOOG_MODULE;
+    testCommonCase(
+        lines(
+            "goog.module('test');", //
+            "{ <import>('symbol'); }"),
+        INVALID_CLOSURE_CALL_SCOPE_ERROR);
+
+    moduleType = ModuleType.ES6_MODULE;
+
+    testCommonCase(
+        lines(
+            "{ <import>('symbol'); }", //
+            "export {};"),
+        INVALID_CLOSURE_CALL_SCOPE_ERROR);
   }
 
   @Test
@@ -694,4 +712,18 @@ public void googRequireBetweenEsModulesIsWarning() {
                     "export {};"))),
         warning(Es6RewriteModules.SHOULD_IMPORT_ES6_MODULE));
   }
+
+  @Test
+  public void testIgnoreArguments() {
+    moduleType = ModuleType.GOOG_MODULE;
+    test(
+        srcs(
+            PROVIDES_SYMBOL_SRC,
+            makeTestFile(
+                lines(
+                    "goog.module('test');", //
+                    "function fn() {",
+                    "  return arguments;",
+                    "}"))));
+  }
 }