Move goog.exportSymbol handling from checks to optimizations phase

PiperOrigin-RevId: 716236673
google · Jan 16, 2025 · 04ba2c5 · 04ba2c5
1 parent b3bc455
commit 04ba2c5
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 32 deletions.
diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java
@@ -1445,7 +1445,6 @@ public void process(Node externs, Node root) {
                 final ProcessClosurePrimitives pass = new ProcessClosurePrimitives(compiler);
                 return (Node externs, Node root) -> {
                   pass.process(externs, root);
-                  compiler.addExportedNames(pass.getExportedVariableNames());
                 };
               })
           .build();
@@ -1455,9 +1454,16 @@ public void process(Node externs, Node root) {
       PassFactory.builder()
           .setName("closureProvidesRequires")
           .setInternalFactory(
-              (compiler) ->
-                  new ProcessClosureProvidesAndRequires(
-                      compiler, options.shouldPreservesGoogProvidesAndRequires()))
+              (compiler) -> {
+                preprocessorSymbolTableFactory.maybeInitialize(compiler);
+                final ProcessClosureProvidesAndRequires pass =
+                    new ProcessClosureProvidesAndRequires(
+                        compiler, options.shouldPreservesGoogProvidesAndRequires());
+                return (Node externs, Node root) -> {
+                  pass.process(externs, root);
+                  compiler.addExportedNames(pass.getExportedVariableNames());
+                };
+              })
           .build();
 
   /** Process AngularJS-specific annotations. */

diff --git a/src/com/google/javascript/jscomp/ProcessClosurePrimitives.java b/src/com/google/javascript/jscomp/ProcessClosurePrimitives.java
@@ -129,8 +129,6 @@ class ProcessClosurePrimitives extends AbstractPostOrderCallback implements Comp
 
   private final Set<String> knownClosureSubclasses = new LinkedHashSet<>();
 
-  private final Set<String> exportedVariables = new LinkedHashSet<>();
-
   private final ImmutableMap<String, ModuleMetadata> closureModules;
 
   ProcessClosurePrimitives(AbstractCompiler compiler) {
@@ -140,10 +138,6 @@ class ProcessClosurePrimitives extends AbstractPostOrderCallback implements Comp
             .getModulesByGoogNamespace();
   }
 
-  Set<String> getExportedVariableNames() {
-    return exportedVariables;
-  }
-
   @Override
   public void process(Node externs, Node root) {
     // Replace and validate other Closure primitives
@@ -244,24 +238,11 @@ private void checkGoogFunctions(NodeTraversal t, Node call) {
     // when we see a provides/requires, and don't worry about
     // reporting the change when we actually do the replacement.
     String methodName = callee.getString();
-    Node arg = callee.getNext();
     switch (methodName) {
       case "inherits":
         // Note: inherits is allowed in local scope
         processInheritsCall(call);
         break;
-      case "exportSymbol":
-        // Note: exportSymbol is allowed in local scope
-        if (arg.isStringLit()) {
-          String argString = arg.getString();
-          int dot = argString.indexOf('.');
-          if (dot == -1) {
-            exportedVariables.add(argString);
-          } else {
-            exportedVariables.add(argString.substring(0, dot));
-          }
-        }
-        break;
       case "addDependency":
         if (validateUnaliasablePrimitiveCall(t, call, methodName)) {
           processAddDependency(call);

diff --git a/src/com/google/javascript/jscomp/ProcessClosureProvidesAndRequires.java b/src/com/google/javascript/jscomp/ProcessClosureProvidesAndRequires.java
@@ -62,6 +62,8 @@ class ProcessClosureProvidesAndRequires implements CompilerPass {
   // Use a LinkedHashMap because the goog.provides must be processed in a deterministic order.
   private final Map<String, ProvidedName> providedNames = new LinkedHashMap<>();
 
+  private final Set<String> exportedVariables = new LinkedHashSet<>();
+
   // If this is true, rewriting will not remove any goog.provide or goog.require calls
   private final boolean preserveGoogProvidesAndRequires;
   private final List<Node> requiresToBeRemoved = new ArrayList<>();
@@ -85,6 +87,10 @@ public void process(Node externs, Node root) {
     rewriteProvidesAndRequires(externs, root);
   }
 
+  Set<String> getExportedVariableNames() {
+    return exportedVariables;
+  }
+
   /** Collects all `goog.provide`s in the given namespace and warns on invalid code */
   Map<String, ProvidedName> collectProvidedNames(Node externs, Node root) {
     if (this.providedNames.isEmpty()) {
@@ -152,6 +158,19 @@ public void visit(NodeTraversal t, Node n, Node parent) {
               // when we see a provides/requires, and don't worry about
               // reporting the change when we actually do the replacement.
               switch (left.getString()) {
+                case "exportSymbol":
+                  // Note: exportSymbol is allowed in local scope
+                  Node arg = n.getSecondChild();
+                  if (arg.isStringLit()) {
+                    String argString = arg.getString();
+                    int dot = argString.indexOf('.');
+                    if (dot == -1) {
+                      exportedVariables.add(argString);
+                    } else {
+                      exportedVariables.add(argString.substring(0, dot));
+                    }
+                  }
+                  break;
                 case "require":
                 case "requireType":
                   if (isValidPrimitiveCall(t, n)) {

diff --git a/src/com/google/javascript/jscomp/testing/TestExternsBuilder.java b/src/com/google/javascript/jscomp/testing/TestExternsBuilder.java
@@ -65,7 +65,7 @@ private static final String lines(String... lines) {
           " * @template T",
           " */",
           "goog.define = function(name, defaultValue) {};",
-          "goog.exportSymbol = function() {};",
+          "goog.exportSymbol = function(publicName, symbol) {};",
           "goog.inherits = function(childCtor, parentCtor) {",
           "  childCtor.superClass_ = parentCtor.prototype;",
           "};",

diff --git a/test/com/google/javascript/jscomp/ProcessClosureProvidesAndRequiresTest.java b/test/com/google/javascript/jscomp/ProcessClosureProvidesAndRequiresTest.java
@@ -39,6 +39,7 @@ public ProcessClosureProvidesAndRequiresTest() {
   }
 
   private boolean preserveGoogProvidesAndRequires;
+  private ProcessClosureProvidesAndRequires lastProcessor;
 
   private ProcessClosureProvidesAndRequires createClosureProcessor(Compiler compiler) {
     return new ProcessClosureProvidesAndRequires(compiler, preserveGoogProvidesAndRequires);
@@ -53,14 +54,15 @@ public void setUp() throws Exception {
     enableTypeCheck();
     enableCreateModuleMap(); // necessary for the typechecker
     replaceTypesWithColors();
+    lastProcessor = null;
   }
 
   @Override
   protected CompilerPass getProcessor(Compiler compiler) {
     return (Node externs, Node root) -> {
       verifyCollectProvidedNamesDoesntChangeAst(externs, root, compiler);
-      ProcessClosureProvidesAndRequires processor = createClosureProcessor(compiler);
-      processor.rewriteProvidesAndRequires(externs, root);
+      lastProcessor = createClosureProcessor(compiler);
+      lastProcessor.rewriteProvidesAndRequires(externs, root);
     };
   }
 
@@ -938,6 +940,20 @@ public void testEsModule_ignored() {
     assertThat(providedNameMap.keySet()).containsExactly("goog");
   }
 
+  @Test
+  public void testGoogExportSymbolRecording() {
+    testSame(
+        """
+            goog.exportSymbol('a', 0);
+            goog.exportSymbol('foo.bar.baz', 1);
+            goog.exportSymbol(notAStringLiteral, 2);
+            function f() {
+              goog.exportSymbol('b', 3);
+            }
+        """);
+    assertThat(lastProcessor.getExportedVariableNames()).containsExactly("a", "foo", "b");
+  }
+
   private Map<String, ProvidedName> getProvidedNameCollection(String js) {
     Compiler compiler = createCompiler();
     ProcessClosureProvidesAndRequires processor = createClosureProcessor(compiler);

diff --git a/test/com/google/javascript/jscomp/RenameVarsTest.java b/test/com/google/javascript/jscomp/RenameVarsTest.java
@@ -1186,7 +1186,8 @@ public void process(Node externs, Node root) {
       new GatherModuleMetadata(
               compiler, /* processCommonJsModules= */ false, ResolutionMode.BROWSER)
           .process(externs, root);
-      ProcessClosurePrimitives closurePass = new ProcessClosurePrimitives(compiler);
+      ProcessClosureProvidesAndRequires closurePass =
+          new ProcessClosureProvidesAndRequires(compiler, true);
       closurePass.process(externs, root);
       renameVars =
           new RenameVars(

diff --git a/test/com/google/javascript/jscomp/integration/TypedAstIntegrationTest.java b/test/com/google/javascript/jscomp/integration/TypedAstIntegrationTest.java
@@ -382,13 +382,11 @@ public void exportSymbol_preventsVariableRenamingCollision() throws IOException
     Compiler compiler = compileTypedAstShards(options);
 
     assertCompiledCodeEquals(
-        // TODO - b/389980981: don't reuse the name 'a' in variable renaming 'a' because it's also
-        // used in the goog.exportSymbol call.
         compiler,
         lines(
-            "var a;", //
-            "var b;",
-            "a.exportSymbol('a', b);"));
+            "var b;", //
+            "var c;",
+            "b.exportSymbol('a', c);"));
   }
 
   @Test