Skip to content

Commit 212c105

Browse files
lauraharkercopybara-github
authored andcommitted
Ban use_precompiled_libraries + any dependency management flags
This functionality is currently semi-broken and will sometimes incorrectly prune or order files. The root problem is that our compilations that generate a TypedAST also run module rewriting passes, which breaks the ability of JSCompiler to order files. PiperOrigin-RevId: 506760290
1 parent f1e67c3 commit 212c105

File tree

2 files changed

+47
-2
lines changed

2 files changed

+47
-2
lines changed

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,15 @@ public int size() {
424424
// If pruning many unused inputs, it's possible that it's faster to regex-parse everything and
425425
// AST-parse only the non-pruned inputs, rather than just AST-parse everything once.
426426
this.preferRegexParser = this.preferRegexParser || options.getDependencyOptions().shouldPrune();
427+
428+
if (options.getMergedPrecompiledLibraries()
429+
&& options.getDependencyOptions().needsManagement()) {
430+
throw new IllegalArgumentException(
431+
"Using precompiled libraries (i.e. TypedAST) is incompatible with flags that "
432+
+ "automatically order/prune dependencies: "
433+
+ options.getDependencyOptions()
434+
+ "");
435+
}
427436
}
428437

429438
public void printConfig() {
@@ -548,6 +557,8 @@ public final void initWithTypedAstFilesystem(
548557
ImmutableSet<SourceFile> files =
549558
ImmutableSet.<SourceFile>builder().addAll(externs).addAll(sources).build();
550559

560+
options.setMergedPrecompiledLibraries(true);
561+
551562
this.initOptions(options);
552563
this.init(externs, sources, options);
553564
this.initTypedAstFilesystem(files, typedAstListStream, options);
@@ -573,6 +584,8 @@ public void initModulesWithTypedAstFilesystem(
573584
}
574585
ImmutableSet<SourceFile> files = filesBuilder.build();
575586

587+
options.setMergedPrecompiledLibraries(true);
588+
576589
this.initOptions(options);
577590
this.initModules(externs, modules, options);
578591
this.initTypedAstFilesystem(files, typedAstListStream, options);
@@ -586,8 +599,6 @@ private void initTypedAstFilesystem(
586599
checkState(this.typedAstFilesystem == null);
587600
maybeSetTracker();
588601

589-
options.setMergedPrecompiledLibraries(true);
590-
591602
this.setLifeCycleStage(LifeCycleStage.COLORS_AND_SIMPLIFIED_JSDOC);
592603
// To speed up builds that don't run type-based optimizations, skip type deserialization
593604
boolean deserializeTypes = options.requiresTypesForOptimization();

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static com.google.common.truth.Truth.assertThat;
2020
import static com.google.javascript.jscomp.base.JSCompStrings.lines;
2121
import static com.google.javascript.rhino.testing.NodeSubject.assertNode;
22+
import static org.junit.Assert.assertThrows;
2223

2324
import com.google.common.collect.ImmutableList;
2425
import com.google.common.collect.ImmutableMap;
@@ -28,8 +29,10 @@
2829
import com.google.javascript.jscomp.CompilerOptions.IncrementalCheckMode;
2930
import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
3031
import com.google.javascript.jscomp.CrossChunkMethodMotion;
32+
import com.google.javascript.jscomp.DependencyOptions;
3133
import com.google.javascript.jscomp.DiagnosticGroups;
3234
import com.google.javascript.jscomp.JSChunk;
35+
import com.google.javascript.jscomp.ModuleIdentifier;
3336
import com.google.javascript.jscomp.SourceFile;
3437
import com.google.javascript.jscomp.VariableRenamingPolicy;
3538
import com.google.javascript.jscomp.WarningLevel;
@@ -73,6 +76,7 @@ public void simpleAlertCall() throws IOException {
7376

7477
CompilerOptions options = new CompilerOptions();
7578
CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
79+
options.setDependencyOptions(DependencyOptions.none());
7680

7781
Compiler compiler = compileTypedAstShards(options);
7882

@@ -93,6 +97,7 @@ public void alertCallWithCrossLibraryVarReference() throws IOException {
9397

9498
CompilerOptions options = new CompilerOptions();
9599
CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
100+
options.setDependencyOptions(DependencyOptions.none());
96101

97102
Compiler compiler = compileTypedAstShards(options);
98103

@@ -139,6 +144,7 @@ public void disambiguatesAndDeletesMethodsAcrossLibraries() throws IOException {
139144

140145
CompilerOptions options = new CompilerOptions();
141146
CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
147+
options.setDependencyOptions(DependencyOptions.none());
142148
options.setDisambiguateProperties(true);
143149

144150
Compiler compiler = compileTypedAstShards(options);
@@ -163,6 +169,7 @@ public void disambiguatesAndDeletesMethodsAcrossLibraries_withTranspilation() th
163169

164170
CompilerOptions options = new CompilerOptions();
165171
CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
172+
options.setDependencyOptions(DependencyOptions.none());
166173
options.setDisambiguateProperties(true);
167174
options.setLanguageOut(LanguageMode.ECMASCRIPT5);
168175

@@ -256,6 +263,7 @@ public void externJSDocPropertiesNotRenamed() throws IOException {
256263

257264
CompilerOptions options = new CompilerOptions();
258265
CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
266+
options.setDependencyOptions(DependencyOptions.none());
259267

260268
Compiler compiler = compileTypedAstShards(options);
261269

@@ -281,6 +289,7 @@ public void gatherExternProperties() throws IOException {
281289

282290
CompilerOptions options = new CompilerOptions();
283291
CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
292+
options.setDependencyOptions(DependencyOptions.none());
284293

285294
Compiler compiler = compileTypedAstShards(options);
286295

@@ -313,6 +322,7 @@ public void protectsHiddenSideEffects() throws IOException {
313322

314323
CompilerOptions options = new CompilerOptions();
315324
CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
325+
options.setDependencyOptions(DependencyOptions.none());
316326

317327
Compiler compiler = compileTypedAstShards(options);
318328

@@ -329,6 +339,7 @@ public void removesRegExpCallsIfSafe() throws IOException {
329339

330340
CompilerOptions options = new CompilerOptions();
331341
CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
342+
options.setDependencyOptions(DependencyOptions.none());
332343

333344
Compiler compiler = compileTypedAstShards(options);
334345

@@ -348,6 +359,7 @@ public void removesRegExpCallsIfUnsafelyReferenced() throws IOException {
348359

349360
CompilerOptions options = new CompilerOptions();
350361
CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
362+
options.setDependencyOptions(DependencyOptions.none());
351363

352364
Compiler compiler = compileTypedAstShards(options);
353365

@@ -374,6 +386,7 @@ public void runsJ2clOptimizations() throws IOException {
374386

375387
CompilerOptions options = new CompilerOptions();
376388
CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
389+
options.setDependencyOptions(DependencyOptions.none());
377390

378391
Compiler compiler = compileTypedAstShards(options);
379392

@@ -460,6 +473,27 @@ public void testCrossChunkMethodMotion() throws IOException {
460473
.isEqualTo(expectedRoot);
461474
}
462475

476+
@Test
477+
public void dependencyModePruningForGoogModules_banned() throws IOException {
478+
precompileLibrary(
479+
extern(new TestExternsBuilder().addClosureExterns().build()),
480+
code("goog.module('keepMe'); const x = 1;"), // input_1
481+
code("goog.module('entryPoint'); goog.require('keepMe');"), // input_2
482+
code("goog.module('dropMe'); const x = 3;")); // input_3
483+
484+
CompilerOptions options = new CompilerOptions();
485+
options.setDependencyOptions(
486+
DependencyOptions.pruneForEntryPoints(
487+
ImmutableList.of(ModuleIdentifier.forClosure("entryPoint"))));
488+
489+
// TODO(b/219588952): if we decide to support this, verify that JSCompiler no longer incorrectly
490+
// prunes the 'keepMe' module.
491+
// This might be fixable by just removing module rewriting from the 'checks' phase.
492+
Exception ex =
493+
assertThrows(IllegalArgumentException.class, () -> compileTypedAstShards(options));
494+
assertThat(ex).hasMessageThat().contains("mode=PRUNE");
495+
}
496+
463497
// use over 'compileTypedAstShards' if you want to validate reported errors or warnings in your
464498
// @Test case.
465499
private Compiler compileTypedAstShardsWithoutErrorChecks(CompilerOptions options)

0 commit comments

Comments
 (0)