diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java index b9bb38a9856..806788e8203 100644 --- 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 index c77b2fb1f34..d431085e4f8 100644 --- 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 knownClosureSubclasses = new LinkedHashSet<>(); - private final Set exportedVariables = new LinkedHashSet<>(); - private final ImmutableMap closureModules; ProcessClosurePrimitives(AbstractCompiler compiler) { @@ -140,10 +138,6 @@ class ProcessClosurePrimitives extends AbstractPostOrderCallback implements Comp .getModulesByGoogNamespace(); } - Set 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 index fbbecb43ff8..f9c78f0aa64 100644 --- 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 providedNames = new LinkedHashMap<>(); + private final Set 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 requiresToBeRemoved = new ArrayList<>(); @@ -85,6 +87,10 @@ public void process(Node externs, Node root) { rewriteProvidesAndRequires(externs, root); } + Set getExportedVariableNames() { + return exportedVariables; + } + /** Collects all `goog.provide`s in the given namespace and warns on invalid code */ Map 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 index 381f6e45639..b4c10b1af08 100644 --- 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 index 6bbc022f3c7..a1ab133cae1 100644 --- 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 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 index 4d51e853e8b..ed24d287261 100644 --- 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 index fec846ff8dc..417fa8e7c91 100644 --- 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