From 55d8c3f189bf4a885eec1d5e8c06b5b1ce3fc79b Mon Sep 17 00:00:00 2001 From: yitingwang Date: Mon, 1 Apr 2019 15:21:15 -0700 Subject: [PATCH] Move hoistExterns after dependency pruning to allow extern files as entry 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 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 --- .../google/javascript/jscomp/Compiler.java | 26 ++-- .../javascript/jscomp/CompilerTest.java | 135 ++++++++++++++++++ 2 files changed, 151 insertions(+), 10 deletions(-) diff --git a/src/com/google/javascript/jscomp/Compiler.java b/src/com/google/javascript/jscomp/Compiler.java index 400222275cc..15c0cc4fcd8 100644 --- a/src/com/google/javascript/jscomp/Compiler.java +++ b/src/com/google/javascript/jscomp/Compiler.java @@ -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 originalInputs = ImmutableList.copyOf(moduleGraph.getAllInputs()); + if (options.getDependencyOptions().needsManagement()) { for (CompilerInput input : moduleGraph.getAllInputs()) { // Forward-declare all the provided types, so that they @@ -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()); @@ -1813,7 +1819,7 @@ void orderInputs() { */ private void findModulesFromEntryPoints( boolean supportEs6Modules, boolean supportCommonJSModules) { - hoistExterns(); + maybeDoThreadedParsing(); List entryPoints = new ArrayList<>(); Map inputsByProvide = new HashMap<>(); Map inputsByIdentifier = new HashMap<>(); @@ -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 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; } @@ -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; diff --git a/test/com/google/javascript/jscomp/CompilerTest.java b/test/com/google/javascript/jscomp/CompilerTest.java index 64ab86b17b1..13d98ad2e6a 100644 --- a/test/com/google/javascript/jscomp/CompilerTest.java +++ b/test/com/google/javascript/jscomp/CompilerTest.java @@ -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 inputs = + ImmutableList.of( + SourceFile.fromCode( + "/externs.js", "/** @fileoverview @externs */ /** @const {number} */ var bar = 1;"), + SourceFile.fromCode("/foo.js", "console.log(0);")); + + List entryPoints = ImmutableList.of(ModuleIdentifier.forFile("/externs.js")); + + CompilerOptions options = createNewFlagBasedOptions(); + options.setDependencyOptions(DependencyOptions.pruneForEntryPoints(entryPoints)); + + List 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 inputs = + ImmutableList.of( + SourceFile.fromCode( + "/externs.js", "/** @fileoverview @externs */ /** @const {number} */ var bar = 1;"), + SourceFile.fromCode("/foo.js", "console.log(nonexistentExtern);")); + + List entryPoints = ImmutableList.of(ModuleIdentifier.forFile("/externs.js")); + + CompilerOptions options = createNewFlagBasedOptions(); + options.setDependencyOptions(DependencyOptions.pruneForEntryPoints(entryPoints)); + + List 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 inputs = + ImmutableList.of( + SourceFile.fromCode( + "/externs.js", "/** @fileoverview @externs */ /** @const {number} */ var bar = 1;"), + SourceFile.fromCode("/foo.js", "console.log(nonexistentExtern);")); + + List entryPoints = + ImmutableList.of( + ModuleIdentifier.forFile("/externs.js"), ModuleIdentifier.forFile("/foo.js")); + + CompilerOptions options = createNewFlagBasedOptions(); + options.setDependencyOptions(DependencyOptions.pruneForEntryPoints(entryPoints)); + + List 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 inputs = + ImmutableList.of( + SourceFile.fromCode( + "/externs.js", "/** @fileoverview @externs */ /** @const {number} */ var bar = 1;"), + SourceFile.fromCode("/foo.js", "console.log(bar);")); + + List entryPoints = + ImmutableList.of( + ModuleIdentifier.forFile("/externs.js"), ModuleIdentifier.forFile("/foo.js")); + + CompilerOptions options = createNewFlagBasedOptions(); + options.setDependencyOptions(DependencyOptions.pruneForEntryPoints(entryPoints)); + + List 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 inputs = + ImmutableList.of( + SourceFile.fromCode( + "/externs.js", "/** @fileoverview @externs */ /** @const {number} */ var bar = 1;"), + SourceFile.fromCode("/foo.js", "console.log(bar);")); + + List entryPoints = ImmutableList.of(ModuleIdentifier.forFile("/foo.js")); + + CompilerOptions options = createNewFlagBasedOptions(); + options.setDependencyOptions(DependencyOptions.pruneForEntryPoints(entryPoints)); + + List 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();