Skip to content

Commit

Permalink
Move hoistExterns after dependency pruning to allow extern files as e…
Browse files Browse the repository at this point in the history 
…ntry points.

When --experimental_fast mode is turned on for js_lib, it pulls in all of the srcs files as entry_points. However, some of them are externs, which originally are hoisted before dependency pruning. This will cause an error during dependency pruning along the lines of "required entry point <extens file> never provided". In order to unblock experimental_fast, this change moves externs hoisting until after dependency pruning so that externs files can be allowed as entry points.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=241411023
  • Loading branch information
@EatingW @blickly
EatingW authored and blickly committed Apr 3, 2019
1 parent 60fc1c7 commit 55d8c3f
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 10 deletions.
26 changes: 16 additions & 10 deletions src/com/google/javascript/jscomp/Compiler.java
View file
Expand Up @@ -1768,9 +1768,14 @@ void orderInputsWithLargeStack() {
}

void orderInputs() {
hoistExterns();
maybeDoThreadedParsing();

// Check if the sources need to be re-ordered.
boolean staleInputs = false;

// Before dependency pruning, save a copy of the original inputs to use for externs hoisting.
ImmutableList<CompilerInput> originalInputs = ImmutableList.copyOf(moduleGraph.getAllInputs());

if (options.getDependencyOptions().needsManagement()) {
for (CompilerInput input : moduleGraph.getAllInputs()) {
// Forward-declare all the provided types, so that they
Expand All @@ -1791,6 +1796,7 @@ void orderInputs() {
MISSING_MODULE_ERROR, e.getMessage()));
}
}
hoistExterns(originalInputs);

// Manage dependencies may move weak sources around, and end up with empty modules.
fillEmptyModules(getModules());
Expand All @@ -1813,7 +1819,7 @@ void orderInputs() {
*/
private void findModulesFromEntryPoints(
boolean supportEs6Modules, boolean supportCommonJSModules) {
hoistExterns();
maybeDoThreadedParsing();
List<CompilerInput> entryPoints = new ArrayList<>();
Map<String, CompilerInput> inputsByProvide = new HashMap<>();
Map<String, CompilerInput> inputsByIdentifier = new HashMap<>();
Expand Down Expand Up @@ -1909,14 +1915,11 @@ private void findModulesFromInput(
}
}

/**
* Hoists inputs with the @externs annotation into the externs list.
*/
void hoistExterns() {
/** Hoists inputs with the @externs annotation into the externs list. */
void hoistExterns(ImmutableList<CompilerInput> originalInputs) {
boolean staleInputs = false;
maybeDoThreadedParsing();
// Iterate a copy because hoisting modifies what we're iterating over.
for (CompilerInput input : ImmutableList.copyOf(moduleGraph.getAllInputs())) {

for (CompilerInput input : originalInputs) {
if (hoistIfExtern(input)) {
staleInputs = true;
}
Expand All @@ -1937,7 +1940,10 @@ private boolean hoistIfExtern(CompilerInput input) {
externsRoot.addChildToBack(input.getAstRoot(this));
input.setIsExtern();

input.getModule().remove(input);
JSModule module = input.getModule();
if (module != null) {
module.remove(input);
}

externs.add(input);
return true;
Expand Down
135 changes: 135 additions & 0 deletions test/com/google/javascript/jscomp/CompilerTest.java
View file
Expand Up @@ -1459,6 +1459,141 @@ public void testEs6ModulePathWithOddCharacters() throws Exception {
assertThat(result.errors).isEmpty();
}

@Test
public void testExternsFileAsEntryPoint() throws Exception {
// Test that you can specify externs as entry points.
// This is needed for turning on experimental_fast for js_lib,
// which adds all sources as entry points.
List<SourceFile> inputs =
ImmutableList.of(
SourceFile.fromCode(
"/externs.js", "/** @fileoverview @externs */ /** @const {number} */ var bar = 1;"),
SourceFile.fromCode("/foo.js", "console.log(0);"));

List<ModuleIdentifier> entryPoints = ImmutableList.of(ModuleIdentifier.forFile("/externs.js"));

CompilerOptions options = createNewFlagBasedOptions();
options.setDependencyOptions(DependencyOptions.pruneForEntryPoints(entryPoints));

List<SourceFile> externs =
AbstractCommandLineRunner.getBuiltinExterns(options.getEnvironment());

Compiler compiler = new Compiler();
compiler.compile(externs, inputs, options);

Result result = compiler.getResult();
assertThat(result.errors).isEmpty();
assertThat(compiler.toSource()).isEqualTo(""); // Empty since srcs are pruned.
}

@Test
public void testExternsFileAsEntryPoint2() throws Exception {
// Test code reference to an extern that doesn't exist,
// but the extern is still the sole entry point.
List<SourceFile> inputs =
ImmutableList.of(
SourceFile.fromCode(
"/externs.js", "/** @fileoverview @externs */ /** @const {number} */ var bar = 1;"),
SourceFile.fromCode("/foo.js", "console.log(nonexistentExtern);"));

List<ModuleIdentifier> entryPoints = ImmutableList.of(ModuleIdentifier.forFile("/externs.js"));

CompilerOptions options = createNewFlagBasedOptions();
options.setDependencyOptions(DependencyOptions.pruneForEntryPoints(entryPoints));

List<SourceFile> externs =
AbstractCommandLineRunner.getBuiltinExterns(options.getEnvironment());

Compiler compiler = new Compiler();
compiler.compile(externs, inputs, options);

Result result = compiler.getResult();
assertThat(result.errors).isEmpty();
assertThat(compiler.toSource()).isEqualTo("");
}

@Test
public void testExternsFileAsEntryPoint3() throws Exception {
// Test code reference to an extern that doesn't exist,
// but the extern and source files are both entry points
List<SourceFile> inputs =
ImmutableList.of(
SourceFile.fromCode(
"/externs.js", "/** @fileoverview @externs */ /** @const {number} */ var bar = 1;"),
SourceFile.fromCode("/foo.js", "console.log(nonexistentExtern);"));

List<ModuleIdentifier> entryPoints =
ImmutableList.of(
ModuleIdentifier.forFile("/externs.js"), ModuleIdentifier.forFile("/foo.js"));

CompilerOptions options = createNewFlagBasedOptions();
options.setDependencyOptions(DependencyOptions.pruneForEntryPoints(entryPoints));

List<SourceFile> externs =
AbstractCommandLineRunner.getBuiltinExterns(options.getEnvironment());

Compiler compiler = new Compiler();
compiler.compile(externs, inputs, options);

Result result = compiler.getResult();
assertThat(result.errors).hasSize(1);
assertThat(result.errors.get(0).getType()).isEqualTo(VarCheck.UNDEFINED_VAR_ERROR);
}

@Test
public void testExternsFileAsEntryPoint4() throws Exception {
// Test that has a code reference to an extern that does exist,
// and the extern and source files are both entry points
List<SourceFile> inputs =
ImmutableList.of(
SourceFile.fromCode(
"/externs.js", "/** @fileoverview @externs */ /** @const {number} */ var bar = 1;"),
SourceFile.fromCode("/foo.js", "console.log(bar);"));

List<ModuleIdentifier> entryPoints =
ImmutableList.of(
ModuleIdentifier.forFile("/externs.js"), ModuleIdentifier.forFile("/foo.js"));

CompilerOptions options = createNewFlagBasedOptions();
options.setDependencyOptions(DependencyOptions.pruneForEntryPoints(entryPoints));

List<SourceFile> externs =
AbstractCommandLineRunner.getBuiltinExterns(options.getEnvironment());

Compiler compiler = new Compiler();
compiler.compile(externs, inputs, options);

Result result = compiler.getResult();
assertThat(result.errors).isEmpty();
assertThat(compiler.toSource()).isEqualTo("console.log(bar);");
}

@Test
public void testExternsFileAsEntryPoint5() throws Exception {
// Test that has a code reference to an extern that does exist,
// and only the source source file is an entry point
List<SourceFile> inputs =
ImmutableList.of(
SourceFile.fromCode(
"/externs.js", "/** @fileoverview @externs */ /** @const {number} */ var bar = 1;"),
SourceFile.fromCode("/foo.js", "console.log(bar);"));

List<ModuleIdentifier> entryPoints = ImmutableList.of(ModuleIdentifier.forFile("/foo.js"));

CompilerOptions options = createNewFlagBasedOptions();
options.setDependencyOptions(DependencyOptions.pruneForEntryPoints(entryPoints));

List<SourceFile> externs =
AbstractCommandLineRunner.getBuiltinExterns(options.getEnvironment());

Compiler compiler = new Compiler();
compiler.compile(externs, inputs, options);

Result result = compiler.getResult();
assertThat(result.errors).isEmpty();
assertThat(compiler.toSource()).isEqualTo("console.log(bar);");
}

@Test
public void testGetEmptyResult() {
Result result = new Compiler().getResult();
Expand Down

0 comments on commit 55d8c3f

Please sign in to comment.