Skip to content

Commit 04ba2c5

Browse files
lauraharkercopybara-github
authored andcommitted
Move goog.exportSymbol handling from checks to optimizations phase
PiperOrigin-RevId: 716236673
1 parent b3bc455 commit 04ba2c5

7 files changed

+53
-32
lines changed

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

+10-4
Original file line numberDiff line numberDiff line change
@@ -1445,7 +1445,6 @@ public void process(Node externs, Node root) {
14451445
final ProcessClosurePrimitives pass = new ProcessClosurePrimitives(compiler);
14461446
return (Node externs, Node root) -> {
14471447
pass.process(externs, root);
1448-
compiler.addExportedNames(pass.getExportedVariableNames());
14491448
};
14501449
})
14511450
.build();
@@ -1455,9 +1454,16 @@ public void process(Node externs, Node root) {
14551454
PassFactory.builder()
14561455
.setName("closureProvidesRequires")
14571456
.setInternalFactory(
1458-
(compiler) ->
1459-
new ProcessClosureProvidesAndRequires(
1460-
compiler, options.shouldPreservesGoogProvidesAndRequires()))
1457+
(compiler) -> {
1458+
preprocessorSymbolTableFactory.maybeInitialize(compiler);
1459+
final ProcessClosureProvidesAndRequires pass =
1460+
new ProcessClosureProvidesAndRequires(
1461+
compiler, options.shouldPreservesGoogProvidesAndRequires());
1462+
return (Node externs, Node root) -> {
1463+
pass.process(externs, root);
1464+
compiler.addExportedNames(pass.getExportedVariableNames());
1465+
};
1466+
})
14611467
.build();
14621468

14631469
/** Process AngularJS-specific annotations. */

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

-19
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,6 @@ class ProcessClosurePrimitives extends AbstractPostOrderCallback implements Comp
129129

130130
private final Set<String> knownClosureSubclasses = new LinkedHashSet<>();
131131

132-
private final Set<String> exportedVariables = new LinkedHashSet<>();
133-
134132
private final ImmutableMap<String, ModuleMetadata> closureModules;
135133

136134
ProcessClosurePrimitives(AbstractCompiler compiler) {
@@ -140,10 +138,6 @@ class ProcessClosurePrimitives extends AbstractPostOrderCallback implements Comp
140138
.getModulesByGoogNamespace();
141139
}
142140

143-
Set<String> getExportedVariableNames() {
144-
return exportedVariables;
145-
}
146-
147141
@Override
148142
public void process(Node externs, Node root) {
149143
// Replace and validate other Closure primitives
@@ -244,24 +238,11 @@ private void checkGoogFunctions(NodeTraversal t, Node call) {
244238
// when we see a provides/requires, and don't worry about
245239
// reporting the change when we actually do the replacement.
246240
String methodName = callee.getString();
247-
Node arg = callee.getNext();
248241
switch (methodName) {
249242
case "inherits":
250243
// Note: inherits is allowed in local scope
251244
processInheritsCall(call);
252245
break;
253-
case "exportSymbol":
254-
// Note: exportSymbol is allowed in local scope
255-
if (arg.isStringLit()) {
256-
String argString = arg.getString();
257-
int dot = argString.indexOf('.');
258-
if (dot == -1) {
259-
exportedVariables.add(argString);
260-
} else {
261-
exportedVariables.add(argString.substring(0, dot));
262-
}
263-
}
264-
break;
265246
case "addDependency":
266247
if (validateUnaliasablePrimitiveCall(t, call, methodName)) {
267248
processAddDependency(call);

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

+19
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ class ProcessClosureProvidesAndRequires implements CompilerPass {
6262
// Use a LinkedHashMap because the goog.provides must be processed in a deterministic order.
6363
private final Map<String, ProvidedName> providedNames = new LinkedHashMap<>();
6464

65+
private final Set<String> exportedVariables = new LinkedHashSet<>();
66+
6567
// If this is true, rewriting will not remove any goog.provide or goog.require calls
6668
private final boolean preserveGoogProvidesAndRequires;
6769
private final List<Node> requiresToBeRemoved = new ArrayList<>();
@@ -85,6 +87,10 @@ public void process(Node externs, Node root) {
8587
rewriteProvidesAndRequires(externs, root);
8688
}
8789

90+
Set<String> getExportedVariableNames() {
91+
return exportedVariables;
92+
}
93+
8894
/** Collects all `goog.provide`s in the given namespace and warns on invalid code */
8995
Map<String, ProvidedName> collectProvidedNames(Node externs, Node root) {
9096
if (this.providedNames.isEmpty()) {
@@ -152,6 +158,19 @@ public void visit(NodeTraversal t, Node n, Node parent) {
152158
// when we see a provides/requires, and don't worry about
153159
// reporting the change when we actually do the replacement.
154160
switch (left.getString()) {
161+
case "exportSymbol":
162+
// Note: exportSymbol is allowed in local scope
163+
Node arg = n.getSecondChild();
164+
if (arg.isStringLit()) {
165+
String argString = arg.getString();
166+
int dot = argString.indexOf('.');
167+
if (dot == -1) {
168+
exportedVariables.add(argString);
169+
} else {
170+
exportedVariables.add(argString.substring(0, dot));
171+
}
172+
}
173+
break;
155174
case "require":
156175
case "requireType":
157176
if (isValidPrimitiveCall(t, n)) {

src/com/google/javascript/jscomp/testing/TestExternsBuilder.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ private static final String lines(String... lines) {
6565
" * @template T",
6666
" */",
6767
"goog.define = function(name, defaultValue) {};",
68-
"goog.exportSymbol = function() {};",
68+
"goog.exportSymbol = function(publicName, symbol) {};",
6969
"goog.inherits = function(childCtor, parentCtor) {",
7070
" childCtor.superClass_ = parentCtor.prototype;",
7171
"};",

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

+18-2
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public ProcessClosureProvidesAndRequiresTest() {
3939
}
4040

4141
private boolean preserveGoogProvidesAndRequires;
42+
private ProcessClosureProvidesAndRequires lastProcessor;
4243

4344
private ProcessClosureProvidesAndRequires createClosureProcessor(Compiler compiler) {
4445
return new ProcessClosureProvidesAndRequires(compiler, preserveGoogProvidesAndRequires);
@@ -53,14 +54,15 @@ public void setUp() throws Exception {
5354
enableTypeCheck();
5455
enableCreateModuleMap(); // necessary for the typechecker
5556
replaceTypesWithColors();
57+
lastProcessor = null;
5658
}
5759

5860
@Override
5961
protected CompilerPass getProcessor(Compiler compiler) {
6062
return (Node externs, Node root) -> {
6163
verifyCollectProvidedNamesDoesntChangeAst(externs, root, compiler);
62-
ProcessClosureProvidesAndRequires processor = createClosureProcessor(compiler);
63-
processor.rewriteProvidesAndRequires(externs, root);
64+
lastProcessor = createClosureProcessor(compiler);
65+
lastProcessor.rewriteProvidesAndRequires(externs, root);
6466
};
6567
}
6668

@@ -938,6 +940,20 @@ public void testEsModule_ignored() {
938940
assertThat(providedNameMap.keySet()).containsExactly("goog");
939941
}
940942

943+
@Test
944+
public void testGoogExportSymbolRecording() {
945+
testSame(
946+
"""
947+
goog.exportSymbol('a', 0);
948+
goog.exportSymbol('foo.bar.baz', 1);
949+
goog.exportSymbol(notAStringLiteral, 2);
950+
function f() {
951+
goog.exportSymbol('b', 3);
952+
}
953+
""");
954+
assertThat(lastProcessor.getExportedVariableNames()).containsExactly("a", "foo", "b");
955+
}
956+
941957
private Map<String, ProvidedName> getProvidedNameCollection(String js) {
942958
Compiler compiler = createCompiler();
943959
ProcessClosureProvidesAndRequires processor = createClosureProcessor(compiler);

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -1186,7 +1186,8 @@ public void process(Node externs, Node root) {
11861186
new GatherModuleMetadata(
11871187
compiler, /* processCommonJsModules= */ false, ResolutionMode.BROWSER)
11881188
.process(externs, root);
1189-
ProcessClosurePrimitives closurePass = new ProcessClosurePrimitives(compiler);
1189+
ProcessClosureProvidesAndRequires closurePass =
1190+
new ProcessClosureProvidesAndRequires(compiler, true);
11901191
closurePass.process(externs, root);
11911192
renameVars =
11921193
new RenameVars(

test/com/google/javascript/jscomp/integration/TypedAstIntegrationTest.java

+3-5
Original file line numberDiff line numberDiff line change
@@ -382,13 +382,11 @@ public void exportSymbol_preventsVariableRenamingCollision() throws IOException
382382
Compiler compiler = compileTypedAstShards(options);
383383

384384
assertCompiledCodeEquals(
385-
// TODO - b/389980981: don't reuse the name 'a' in variable renaming 'a' because it's also
386-
// used in the goog.exportSymbol call.
387385
compiler,
388386
lines(
389-
"var a;", //
390-
"var b;",
391-
"a.exportSymbol('a', b);"));
387+
"var b;", //
388+
"var c;",
389+
"b.exportSymbol('a', c);"));
392390
}
393391

394392
@Test

0 commit comments

Comments
 (0)