From 6ac58c94af9f95f6f826dda463864a5a960489ad Mon Sep 17 00:00:00 2001 From: johnplaisted Date: Fri, 16 Nov 2018 14:17:49 -0800 Subject: [PATCH] If pruning is enabled then prune any files that are weakly reachable from entry points. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=221851333 --- .../google/javascript/jscomp/Compiler.java | 62 +- .../javascript/jscomp/JSModuleGraph.java | 111 +++- .../jscomp/deps/Es6SortedDependencies.java | 47 +- .../jscomp/deps/SortedDependencies.java | 37 +- .../javascript/jscomp/CompilerTest.java | 191 ++++++ .../javascript/jscomp/JSModuleGraphTest.java | 612 ++++++++++++++---- .../deps/Es6SortedDependenciesTest.java | 61 +- 7 files changed, 912 insertions(+), 209 deletions(-) diff --git a/src/com/google/javascript/jscomp/Compiler.java b/src/com/google/javascript/jscomp/Compiler.java index 2b2cd4e092e..56d9b8fc7db 100644 --- a/src/com/google/javascript/jscomp/Compiler.java +++ b/src/com/google/javascript/jscomp/Compiler.java @@ -483,8 +483,6 @@ public void initModules( initOptions(options); checkFirstModule(modules); - modules = moveWeakSources(modules); - fillEmptyModules(modules); this.externs = makeExternInputs(externs); @@ -499,6 +497,9 @@ public void initModules( return; } + // Creating the module graph can move weak source around, and end up with empty modules. + fillEmptyModules(getModules()); + this.commentsPerFile = new ConcurrentHashMap<>(moduleGraph.getInputCount()); initBasedOnOptions(); @@ -567,52 +568,6 @@ private void checkFirstModule(List modules) { } } - /** Moves all weak sources into a separate weak module that depends on every other module. */ - private List moveWeakSources(List modules) { - // Collect weak sources. - List weakInputs = new ArrayList<>(); - for (JSModule module : modules) { - if (module.getName().equals(JSModule.WEAK_MODULE_NAME)) { - // Skip an already existing weak module - see below. - continue; - } - for (int i = 0; i < module.getInputCount(); ) { - CompilerInput input = module.getInput(i); - if (input.getSourceFile().isWeak()) { - module.remove(input); - weakInputs.add(input); - } else { - i++; - } - } - } - - // If a weak module already exists (e.g. in a stage 2 compilation), make sure it contains all - // weak sources, but leave the module graph otherwise untouched. - if (moduleGraph != null - && moduleGraph.getModuleByName(JSModule.WEAK_MODULE_NAME) != null - && !weakInputs.isEmpty()) { - throw new RuntimeException( - "A weak module already exists but weak sources were found in other modules."); - } - - // Create the weak module and make it depend on every other module. - JSModule weakModule = new JSModule(JSModule.WEAK_MODULE_NAME); - for (JSModule module : modules) { - weakModule.addDependency(module); - } - - // Move the weak sources. - for (CompilerInput input : weakInputs) { - weakModule.add(input); - } - - // Make a copy in case the original list is immutable. - modules = ImmutableList.builder().addAll(modules).add(weakModule).build(); - - return modules; - } - /** * Empty modules get an empty "fill" file, so that we can move code into * an empty module. @@ -823,6 +778,15 @@ public void stage2Passes() { checkState(moduleGraph != null, "No inputs. Did you call init() or initModules()?"); checkState(!hasErrors()); checkState(!options.getInstrumentForCoverageOnly()); + JSModule weakModule = moduleGraph.getModuleByName(JSModule.WEAK_MODULE_NAME); + if (weakModule != null) { + for (CompilerInput i : moduleGraph.getAllInputs()) { + if (i.getSourceFile().isWeak()) { + checkState( + i.getModule() == weakModule, "Expected all weak files to be in the weak module."); + } + } + } runInCompilerThread( () -> { if (options.shouldOptimize()) { @@ -1840,6 +1804,8 @@ void orderInputs() { } } + // Manage dependencies may move weak sources around, and end up with empty modules. + fillEmptyModules(getModules()); hoistNoCompileFiles(); if (staleInputs) { diff --git a/src/com/google/javascript/jscomp/JSModuleGraph.java b/src/com/google/javascript/jscomp/JSModuleGraph.java index 81fd74ce821..bacad15c900 100644 --- a/src/com/google/javascript/jscomp/JSModuleGraph.java +++ b/src/com/google/javascript/jscomp/JSModuleGraph.java @@ -35,6 +35,7 @@ import com.google.javascript.jscomp.deps.SortedDependencies.MissingProvideException; import com.google.javascript.jscomp.graph.LinkedDirectedGraph; import com.google.javascript.jscomp.parsing.parser.util.format.SimpleFormat; +import com.google.javascript.rhino.StaticSourceFile.SourceKind; import java.io.Serializable; import java.util.ArrayList; import java.util.Arrays; @@ -96,6 +97,7 @@ public JSModuleGraph(JSModule[] modulesInDepOrder) { /** Creates a module graph from a list of modules in dependency order. */ public JSModuleGraph(List modulesInDepOrder) { Preconditions.checkState(!modulesInDepOrder.isEmpty()); + modulesInDepOrder = makeWeakModule(modulesInDepOrder); modules = new JSModule[modulesInDepOrder.size()]; // n = number of modules @@ -118,6 +120,9 @@ public JSModuleGraph(List modulesInDepOrder) { // O(n*m) subtreeSize = initSubtreeSize(); + + // Move all sources marked as weak by outside sources (e.g. flags) into the weak module. + moveMarkedWeakSources(getModuleByName(JSModule.WEAK_MODULE_NAME), getAllInputs()); } private List> initModulesByDepth() { @@ -146,6 +151,52 @@ private List> initModulesByDepth() { return tmpModulesByDepth; } + /** + * If a weak module doesn't already exist, creates a weak module depending on every other module. + * + *

Does not move any sources into the weak module. + * + * @return a new list of modules that includes the weak module, if it was newly created, or the + * same list if the weak module already existed + * @throws IllegalStateException if a weak module already exists but doesn't fulfill the above + * conditions + */ + private List makeWeakModule(List modulesInDepOrder) { + boolean hasWeakModule = false; + for (JSModule module : modulesInDepOrder) { + if (module.getName().equals(JSModule.WEAK_MODULE_NAME)) { + hasWeakModule = true; + Set allOtherModules = new HashSet<>(modulesInDepOrder); + allOtherModules.remove(module); + checkState( + module.getAllDependencies().containsAll(allOtherModules), + "The weak module must depend on all other modules."); + checkState( + module.getAllDependencies().size() == allOtherModules.size(), + "The weak module cannot have extra dependencies."); + break; + } + } + if (hasWeakModule) { + // All weak files (and only weak files) should be in the weak module. + for (JSModule module : modulesInDepOrder) { + for (CompilerInput input : module.getInputs()) { + checkState( + input.getSourceFile().isWeak() == module.getName().equals(JSModule.WEAK_MODULE_NAME), + "Weak sources must be in the weak module."); + } + } + } else { + JSModule weakModule = new JSModule(JSModule.WEAK_MODULE_NAME); + for (JSModule module : modulesInDepOrder) { + weakModule.addDependency(module); + } + modulesInDepOrder = new ArrayList<>(modulesInDepOrder); + modulesInDepOrder.add(weakModule); + } + return modulesInDepOrder; + } + private BitSet[] initTransitiveDepsBitSets() { BitSet[] array = new BitSet[modules.length]; for (int moduleIndex = 0; moduleIndex < modules.length; ++moduleIndex) { @@ -426,6 +477,27 @@ private Set getTransitiveDeps(JSModule m) { return deps; } + /** + * Moves all sources that have {@link SourceKind#WEAK} into the weak module so that they may be + * pruned later. + */ + private static void moveMarkedWeakSources(JSModule weakModule, Iterable inputs) { + checkNotNull(weakModule); + ImmutableList allInputs = ImmutableList.copyOf(inputs); + for (CompilerInput i : allInputs) { + if (i.getSourceFile().isWeak()) { + JSModule existingModule = i.getModule(); + if (existingModule == weakModule) { + continue; + } + if (existingModule != null) { + existingModule.remove(i); + } + weakModule.add(i); + } + } + } + /** * Apply the dependency options to the list of sources, returning a new source list re-ordering * and dropping files as necessary. This module graph will be updated to reflect the new list. @@ -467,7 +539,7 @@ public ImmutableList manageDependencies(DependencyOptions depende // The order of inputs, sorted independently of modules. List absoluteOrder = - sorter.getDependenciesOf(originalInputs, dependencyOptions.shouldSort()); + sorter.getStrongDependenciesOf(originalInputs, dependencyOptions.shouldSort()); // Figure out which sources *must* be in each module. ListMultimap entryPointInputsPerModule = @@ -478,8 +550,7 @@ public ImmutableList manageDependencies(DependencyOptions depende entryPointInputsPerModule.put(module, input); } - // Clear the modules of their inputs. This also nulls out - // the input's reference to its module. + // Clear the modules of their inputs. This also nulls out the input's reference to its module. for (JSModule module : getAllModules()) { module.removeAll(); } @@ -488,6 +559,7 @@ public ImmutableList manageDependencies(DependencyOptions depende // of that module's dependencies. List orderedInputs = new ArrayList<>(); Set reachedInputs = new HashSet<>(); + for (JSModule module : entryPointInputsPerModule.keySet()) { List transitiveClosure; // Prefer a depth first ordering of dependencies from entry points. @@ -511,10 +583,14 @@ public ImmutableList manageDependencies(DependencyOptions depende // Simply order inputs so that any required namespace comes before it's usage. // Ordered result varies based on the original order of inputs. transitiveClosure = - sorter.getDependenciesOf( + sorter.getStrongDependenciesOf( entryPointInputsPerModule.get(module), dependencyOptions.shouldSort()); } for (CompilerInput input : transitiveClosure) { + if (dependencyOptions.shouldPrune() && input.getSourceFile().isWeak()) { + throw new IllegalStateException( + "A file that is reachable via an entry point cannot be marked as weak."); + } JSModule oldModule = input.getModule(); if (oldModule == null) { input.setModule(module); @@ -530,11 +606,31 @@ public ImmutableList manageDependencies(DependencyOptions depende orderedInputs = absoluteOrder; } + JSModule weakModule = getModuleByName(JSModule.WEAK_MODULE_NAME); + checkNotNull(weakModule); + // Mark all sources that are detected as weak. + if (dependencyOptions.shouldPrune()) { + List weakInputs = sorter.getSortedWeakDependenciesOf(orderedInputs); + for (CompilerInput i : weakInputs) { + // Have a separate loop here to add all these dependencies rather than rely on + // moveMarkedWeakSources below. moveMarkedWeakSources will be in user order, while this + // loop is in dependency order. For sources detected as weak we want dependency order, + // for pre-marked as weak we want user order. + checkState(i.getModule() == null); + i.getSourceFile().setKind(SourceKind.WEAK); + i.setModule(weakModule); + weakModule.add(i); + } + } + + // Move all sources marked as weak by outside sources (e.g. flags) into the weak module. + moveMarkedWeakSources(weakModule, originalInputs); + // All the inputs are pointing to the modules that own them. Yeah! // Update the modules to reflect this. for (CompilerInput input : orderedInputs) { JSModule module = input.getModule(); - if (module != null) { + if (module != null && !module.getInputs().contains(input)) { module.add(input); } } @@ -549,7 +645,7 @@ public ImmutableList manageDependencies(DependencyOptions depende } /** - * Given an input and set of unprocessed inputs, return the input and it's dependencies by + * Given an input and set of unprocessed inputs, return the input and it's strong dependencies by * performing a recursive, depth-first traversal. */ private List getDepthFirstDependenciesOf( @@ -561,8 +657,7 @@ private List getDepthFirstDependenciesOf( return orderedInputs; } - for (String importedNamespace : - Iterables.concat(rootInput.getRequiredSymbols(), rootInput.getTypeRequires())) { + for (String importedNamespace : rootInput.getRequiredSymbols()) { CompilerInput dependency = null; if (inputsByProvide.containsKey(importedNamespace) && unreachedInputs.contains(inputsByProvide.get(importedNamespace))) { diff --git a/src/com/google/javascript/jscomp/deps/Es6SortedDependencies.java b/src/com/google/javascript/jscomp/deps/Es6SortedDependencies.java index 7e52e7ac387..d24e0529452 100644 --- a/src/com/google/javascript/jscomp/deps/Es6SortedDependencies.java +++ b/src/com/google/javascript/jscomp/deps/Es6SortedDependencies.java @@ -68,7 +68,7 @@ public Es6SortedDependencies(List userOrderedInputs) { } @Override - public ImmutableList getDependenciesOf(List rootInputs, boolean sorted) { + public ImmutableList getStrongDependenciesOf(List rootInputs, boolean sorted) { checkArgument(userOrderedInputs.containsAll(rootInputs)); Set includedInputs = new HashSet<>(); @@ -76,6 +76,7 @@ public ImmutableList getDependenciesOf(List rootInputs, boolean so while (!worklist.isEmpty()) { INPUT input = worklist.pop(); if (includedInputs.add(input)) { + for (String symbolName : input.getRequiredSymbols()) { INPUT importedSymbolName = exportingInputBySymbolName.get(symbolName); if (importedSymbolName != null) { @@ -110,8 +111,44 @@ public ImmutableList getInputsWithoutProvides() { } @Override - public ImmutableList getSortedDependenciesOf(List roots) { - return getDependenciesOf(roots, true); + public ImmutableList getSortedStrongDependenciesOf(List roots) { + return getStrongDependenciesOf(roots, true); + } + + @Override + public List getSortedWeakDependenciesOf(List rootInputs) { + Set strongInputs = new HashSet<>(getSortedStrongDependenciesOf(rootInputs)); + Set weakInputs = new HashSet<>(); + Deque worklist = new ArrayDeque<>(strongInputs); + while (!worklist.isEmpty()) { + INPUT input = worklist.pop(); + boolean isStrong = strongInputs.contains(input); + + Iterable edges = + isStrong + ? input.getTypeRequires() + : Iterables.concat(input.getRequiredSymbols(), input.getTypeRequires()); + + if (!isStrong) { + weakInputs.add(input); + } + + for (String symbolName : edges) { + INPUT importedSymbolName = exportingInputBySymbolName.get(symbolName); + if (importedSymbolName != null) { + worklist.add(importedSymbolName); + } + } + } + + ImmutableList.Builder builder = ImmutableList.builder(); + for (INPUT input : importOrderedInputs) { + if (weakInputs.contains(input)) { + builder.add(input); + } + } + + return builder.build(); } @Override @@ -163,7 +200,9 @@ private void processInputs() { } } for (INPUT userOrderedInput : userOrderedInputs) { - for (String symbolName : userOrderedInput.getRequiredSymbols()) { + for (String symbolName : + Iterables.concat( + userOrderedInput.getRequiredSymbols(), userOrderedInput.getTypeRequires())) { INPUT importedInput = exportingInputBySymbolName.get(symbolName); if (importedInput != null) { importedInputByImportingInput.put(userOrderedInput, importedInput); diff --git a/src/com/google/javascript/jscomp/deps/SortedDependencies.java b/src/com/google/javascript/jscomp/deps/SortedDependencies.java index 75ba50ebc8c..fa3c68389c6 100644 --- a/src/com/google/javascript/jscomp/deps/SortedDependencies.java +++ b/src/com/google/javascript/jscomp/deps/SortedDependencies.java @@ -47,23 +47,36 @@ public interface SortedDependencies { public List getSortedList(); /** - * Gets all the dependencies of the given roots. The inputs must be returned - * in a stable order. In other words, if A comes before B, and A does not - * transitively depend on B, then A must also come before B in the returned - * list. + * Gets all the strong dependencies of the given roots. The inputs must be returned in a stable + * order. In other words, if A comes before B, and A does not transitively depend on B, then A + * must also come before B in the returned list. */ - public List getSortedDependenciesOf(List roots); + public List getSortedStrongDependenciesOf(List roots); /** - * Gets all the dependencies of the given roots. The inputs must be returned - * in a stable order. In other words, if A comes before B, and A does not - * transitively depend on B, then A must also come before B in the returned - * list. + * Gets all the weak dependencies of the given roots. The inputs must be returned in stable order. + * In other words, if A comes before B, and A does not * transitively depend on B, then A must + * also come before B in the returned * list. * - * @param sorted If true, get them in topologically sorted order. If false, - * get them in the original order they were passed to the compiler. + *

The weak dependencies are those that are only reachable via type requires from the roots. + * Note that if a root weakly requires another input, then all of its transitive dependencies + * (strong or weak) that are not strongly reachable from the roots will be included. e.g. if A + * weakly requires B, and B strongly requires C, and A is the sole root, then this will return B + * and C. However, if we add D as a root, and D strongly requires C, then this will only return B. + * + *

Root inputs will never be in the returned list as they are all considered strong. + */ + public List getSortedWeakDependenciesOf(List roots); + + /** + * Gets all the strong dependencies of the given roots. The inputs must be returned in a stable + * order. In other words, if A comes before B, and A does not transitively depend on B, then A + * must also come before B in the returned list. + * + * @param sorted If true, get them in topologically sorted order. If false, get them in the + * original order they were passed to the compiler. */ - public List getDependenciesOf(List roots, boolean sorted); + public List getStrongDependenciesOf(List roots, boolean sorted); public List getInputsWithoutProvides(); diff --git a/test/com/google/javascript/jscomp/CompilerTest.java b/test/com/google/javascript/jscomp/CompilerTest.java index 97054c33f85..0c7b1343f07 100644 --- a/test/com/google/javascript/jscomp/CompilerTest.java +++ b/test/com/google/javascript/jscomp/CompilerTest.java @@ -19,6 +19,7 @@ import static com.google.common.truth.Truth.assertWithMessage; import static com.google.javascript.jscomp.CompilerTestCase.lines; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.fail; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; @@ -2077,4 +2078,194 @@ public void testWeakSourcesEntryPoint() throws Exception { assertThat(compiler.toSource()) .isEqualTo("var module$exports$strong={};function module$contents$strong_f(x){alert(x)};"); } + + @Test + public void testImplicitWeakSourcesWithEntryPoint() throws Exception { + SourceFile extern = SourceFile.fromCode("extern.js", "/** @externs */ function alert(x) {}"); + SourceFile strong = + SourceFile.fromCode( + "strong.js", + lines( + "goog.module('strong');", + "const T = goog.requireType('weak');", + "/** @param {!T} x */ function f(x) { alert(x); }")); + SourceFile weak = + SourceFile.fromCode( + "type.js", + lines( + "goog.module('weak');", + "/** @typedef {number|string} */ exports.T;", + "sideeffect();")); + + CompilerOptions options = new CompilerOptions(); + options.setEmitUseStrict(false); + options.setClosurePass(true); + options.setDependencyOptions( + DependencyOptions.pruneForEntryPoints( + ImmutableList.of(ModuleIdentifier.forClosure("strong")))); + + Compiler compiler = new Compiler(); + + compiler.init(ImmutableList.of(extern), ImmutableList.of(strong, weak), options); + + compiler.parse(); + compiler.check(); + compiler.performOptimizations(); + + assertThat(compiler.toSource()) + .isEqualTo("var module$exports$strong={};function module$contents$strong_f(x){alert(x)};"); + } + + @Test + public void testImplicitWeakSourcesWithEntryPointLegacyPrune() throws Exception { + SourceFile extern = SourceFile.fromCode("extern.js", "/** @externs */ function alert(x) {}"); + SourceFile strong = + SourceFile.fromCode( + "moocher.js", + lines( + "goog.requireType('weak');", + "/** @param {!weak.T} x */ function f(x) { alert(x); }")); + SourceFile weak = + SourceFile.fromCode( + "type.js", + lines( + "goog.module('weak');", + "/** @typedef {number|string} */ exports.T;", + "sideeffect();")); + + CompilerOptions options = new CompilerOptions(); + options.setEmitUseStrict(false); + options.setClosurePass(true); + options.setDependencyOptions(DependencyOptions.pruneLegacyForEntryPoints(ImmutableList.of())); + + Compiler compiler = new Compiler(); + + compiler.init(ImmutableList.of(extern), ImmutableList.of(strong, weak), options); + + compiler.parse(); + compiler.check(); + compiler.performOptimizations(); + + assertThat(compiler.toSource()).isEqualTo("function f(x){alert(x)};"); + } + + @Test + public void testTransitiveImplicitWeakSourcesWithEntryPoint() throws Exception { + SourceFile extern = SourceFile.fromCode("extern.js", "/** @externs */ function alert(x) {}"); + SourceFile strong = + SourceFile.fromCode( + "strong.js", + lines( + "goog.module('strong');", + "const T = goog.requireType('weakEntry');", + "/** @param {!T} x */ function f(x) { alert(x); }")); + SourceFile weakEntry = + SourceFile.fromCode( + "weakEntry.js", + lines( + "goog.module('weakEntry');", + "const w = goog.require('weakByAssociation');", + "exports = w;")); + SourceFile weakByAssociation = + SourceFile.fromCode( + "weakByAssociation.js", + lines( + "goog.module('weakByAssociation');", + "/** @typedef {number|string} */ exports.T;", + "sideEffect();")); + + CompilerOptions options = new CompilerOptions(); + options.setEmitUseStrict(false); + options.setClosurePass(true); + options.setDependencyOptions( + DependencyOptions.pruneForEntryPoints( + ImmutableList.of(ModuleIdentifier.forClosure("strong")))); + + Compiler compiler = new Compiler(); + + compiler.init( + ImmutableList.of(extern), ImmutableList.of(strong, weakEntry, weakByAssociation), options); + + compiler.parse(); + compiler.check(); + compiler.performOptimizations(); + + assertThat(compiler.getWarnings()).isEmpty(); + assertThat(compiler.getErrors()).isEmpty(); + + assertThat(compiler.toSource()) + .isEqualTo("var module$exports$strong={};function module$contents$strong_f(x){alert(x)};"); + } + + @Test + public void testWeakAsEntryPointIsError() throws Exception { + SourceFile extern = SourceFile.fromCode("extern.js", ""); + SourceFile weakEntry = + SourceFile.fromCode( + "weakEntry.js", + lines( + "goog.module('weakEntry');", + "/** @typedef {number|string} */ exports.T;", + "sideEffect();"), + SourceKind.WEAK); + + CompilerOptions options = new CompilerOptions(); + options.setDependencyOptions( + DependencyOptions.pruneForEntryPoints( + ImmutableList.of(ModuleIdentifier.forClosure("weakEntry")))); + + Compiler compiler = new Compiler(); + + compiler.init(ImmutableList.of(extern), ImmutableList.of(weakEntry), options); + + try { + compiler.parse(); + fail(); + } catch (IllegalStateException e) { + // expected + assertThat(e) + .hasMessageThat() + .isEqualTo("A file that is reachable via an entry point cannot be marked as weak."); + } + } + + @Test + public void testWeakStronglyReachableIsError() throws Exception { + SourceFile extern = SourceFile.fromCode("extern.js", "/** @externs */ function alert(x) {}"); + SourceFile strong = + SourceFile.fromCode( + "strong.js", + lines( + "goog.module('strong');", + "const T = goog.require('weakEntry');", + "/** @param {!T} x */ function f(x) { alert(x); }"), + SourceKind.STRONG); + SourceFile weakEntry = + SourceFile.fromCode( + "weakEntry.js", + lines( + "goog.module('weakEntry');", + "/** @typedef {number|string} */ exports.T;", + "sideEffect();"), + SourceKind.WEAK); + + CompilerOptions options = new CompilerOptions(); + options.setDependencyOptions( + DependencyOptions.pruneForEntryPoints( + ImmutableList.of(ModuleIdentifier.forClosure("strong")))); + + Compiler compiler = new Compiler(); + + compiler.init(ImmutableList.of(extern), ImmutableList.of(strong, weakEntry), options); + + try { + compiler.parse(); + fail(); + } catch (IllegalStateException e) { + // expected + assertThat(e) + .hasMessageThat() + .isEqualTo("A file that is reachable via an entry point cannot be marked as weak."); + } + } } diff --git a/test/com/google/javascript/jscomp/JSModuleGraphTest.java b/test/com/google/javascript/jscomp/JSModuleGraphTest.java index ddbfc68fb2e..15535c5c886 100644 --- a/test/com/google/javascript/jscomp/JSModuleGraphTest.java +++ b/test/com/google/javascript/jscomp/JSModuleGraphTest.java @@ -19,17 +19,20 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static java.util.Collections.shuffle; +import static org.junit.Assert.fail; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.gson.JsonArray; import com.google.gson.JsonObject; import com.google.javascript.jscomp.deps.DependencyInfo.Require; +import com.google.javascript.rhino.StaticSourceFile.SourceKind; import java.util.ArrayList; import java.util.Arrays; import java.util.BitSet; import java.util.HashMap; import java.util.List; +import java.util.stream.Collectors; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -42,13 +45,12 @@ @RunWith(JUnit4.class) public final class JSModuleGraphTest { - // NOTE: These are not static. It would probably be clearer to initialize them in setUp() - private final JSModule A = new JSModule("A"); - private final JSModule B = new JSModule("B"); - private final JSModule C = new JSModule("C"); - private final JSModule D = new JSModule("D"); - private final JSModule E = new JSModule("E"); - private final JSModule F = new JSModule("F"); + private JSModule moduleA; + private JSModule moduleB; + private JSModule moduleC; + private JSModule moduleD; + private JSModule moduleE; + private JSModule moduleF; private JSModuleGraph graph = null; // For resolving dependencies only. @@ -56,18 +58,134 @@ public final class JSModuleGraphTest { @Before public void setUp() throws Exception { - B.addDependency(A); // __A__ - C.addDependency(A); // / | \ - D.addDependency(B); // B C | - E.addDependency(B); // / \ /| | - E.addDependency(C); // D E | / - F.addDependency(A); // \|/ - F.addDependency(C); // F - F.addDependency(E); - graph = new JSModuleGraph(new JSModule[] {A, B, C, D, E, F}); compiler = new Compiler(); } + private void makeDeps() { + moduleA = new JSModule("moduleA"); + moduleB = new JSModule("moduleB"); + moduleC = new JSModule("moduleC"); + moduleD = new JSModule("moduleD"); + moduleE = new JSModule("moduleE"); + moduleF = new JSModule("moduleF"); + moduleB.addDependency(moduleA); // __A__ + moduleC.addDependency(moduleA); // / | \ + moduleD.addDependency(moduleB); // B C | + moduleE.addDependency(moduleB); // / \ /| | + moduleE.addDependency(moduleC); // D E | / + moduleF.addDependency(moduleA); // \|/ + moduleF.addDependency(moduleC); // F + moduleF.addDependency(moduleE); + } + + private void makeGraph() { + graph = + new JSModuleGraph(new JSModule[] {moduleA, moduleB, moduleC, moduleD, moduleE, moduleF}); + } + + private JSModule getWeakModule() { + return graph.getModuleByName(JSModule.WEAK_MODULE_NAME); + } + + @Test + public void testMakesWeakModuleIfNotPassed() { + makeDeps(); + makeGraph(); + assertThat(graph.getModuleCount()).isEqualTo(7); + assertThat(graph.getModulesByName()).containsKey(JSModule.WEAK_MODULE_NAME); + assertThat(getWeakModule().getAllDependencies()) + .containsExactly(moduleA, moduleB, moduleC, moduleD, moduleE, moduleF); + } + + @Test + public void testAcceptExistingWeakModule() { + makeDeps(); + JSModule weakModule = new JSModule(JSModule.WEAK_MODULE_NAME); + + weakModule.addDependency(moduleA); + weakModule.addDependency(moduleB); + weakModule.addDependency(moduleC); + weakModule.addDependency(moduleD); + weakModule.addDependency(moduleE); + weakModule.addDependency(moduleF); + + weakModule.add(SourceFile.fromCode("weak", "", SourceKind.WEAK)); + + JSModuleGraph graph = + new JSModuleGraph( + new JSModule[] {moduleA, moduleB, moduleC, moduleD, moduleE, moduleF, weakModule}); + + assertThat(graph.getModuleCount()).isEqualTo(7); + assertThat(graph.getModuleByName(JSModule.WEAK_MODULE_NAME)).isSameAs(weakModule); + } + + @Test + public void testExistingWeakModuleMustHaveDependenciesOnAllOtherModules() { + makeDeps(); + JSModule weakModule = new JSModule(JSModule.WEAK_MODULE_NAME); + + weakModule.addDependency(moduleA); + weakModule.addDependency(moduleB); + weakModule.addDependency(moduleC); + weakModule.addDependency(moduleD); + weakModule.addDependency(moduleE); + // Missing F + + try { + new JSModuleGraph( + new JSModule[] {moduleA, moduleB, moduleC, moduleD, moduleE, moduleF, weakModule}); + fail(); + } catch (IllegalStateException e) { + assertThat(e).hasMessageThat().isEqualTo("The weak module must depend on all other modules."); + } + } + + @Test + public void testWeakFileCannotExistOutsideWeakModule() { + makeDeps(); + JSModule weakModule = new JSModule(JSModule.WEAK_MODULE_NAME); + + weakModule.addDependency(moduleA); + weakModule.addDependency(moduleB); + weakModule.addDependency(moduleC); + weakModule.addDependency(moduleD); + weakModule.addDependency(moduleE); + weakModule.addDependency(moduleF); + + moduleA.add(SourceFile.fromCode("a", "", SourceKind.WEAK)); + + try { + new JSModuleGraph( + new JSModule[] {moduleA, moduleB, moduleC, moduleD, moduleE, moduleF, weakModule}); + fail(); + } catch (IllegalStateException e) { + assertThat(e).hasMessageThat().isEqualTo("Weak sources must be in the weak module."); + } + } + + @Test + public void testStrongFileCannotExistInWeakModule() { + makeDeps(); + JSModule weakModule = new JSModule(JSModule.WEAK_MODULE_NAME); + + weakModule.addDependency(moduleA); + weakModule.addDependency(moduleB); + weakModule.addDependency(moduleC); + weakModule.addDependency(moduleD); + weakModule.addDependency(moduleE); + weakModule.addDependency(moduleF); + + weakModule.add(SourceFile.fromCode("a", "", SourceKind.STRONG)); + + try { + new JSModuleGraph( + new JSModule[] {moduleA, moduleB, moduleC, moduleD, moduleE, moduleF, weakModule}); + fail(); + } catch (IllegalStateException e) { + assertThat(e).hasMessageThat().isEqualTo("Weak sources must be in the weak module."); + } + } + @Test public void testSmallerTreeBeatsDeeperTree() { final JSModule a = new JSModule("a"); @@ -109,109 +227,121 @@ public void testSmallerTreeBeatsDeeperTree() { @Test public void testModuleDepth() { - assertWithMessage("A should have depth 0").that(A.getDepth()).isEqualTo(0); - assertWithMessage("B should have depth 1").that(B.getDepth()).isEqualTo(1); - assertWithMessage("C should have depth 1").that(C.getDepth()).isEqualTo(1); - assertWithMessage("D should have depth 2").that(D.getDepth()).isEqualTo(2); - assertWithMessage("E should have depth 2").that(E.getDepth()).isEqualTo(2); - assertWithMessage("F should have depth 3").that(F.getDepth()).isEqualTo(3); + makeDeps(); + makeGraph(); + assertWithMessage("moduleA should have depth 0").that(moduleA.getDepth()).isEqualTo(0); + assertWithMessage("moduleB should have depth 1").that(moduleB.getDepth()).isEqualTo(1); + assertWithMessage("moduleC should have depth 1").that(moduleC.getDepth()).isEqualTo(1); + assertWithMessage("moduleD should have depth 2").that(moduleD.getDepth()).isEqualTo(2); + assertWithMessage("moduleE should have depth 2").that(moduleE.getDepth()).isEqualTo(2); + assertWithMessage("moduleF should have depth 3").that(moduleF.getDepth()).isEqualTo(3); } @Test public void testDeepestCommonDep() { - assertDeepestCommonDep(null, A, A); - assertDeepestCommonDep(null, A, B); - assertDeepestCommonDep(null, A, C); - assertDeepestCommonDep(null, A, D); - assertDeepestCommonDep(null, A, E); - assertDeepestCommonDep(null, A, F); - assertDeepestCommonDep(A, B, B); - assertDeepestCommonDep(A, B, C); - assertDeepestCommonDep(A, B, D); - assertDeepestCommonDep(A, B, E); - assertDeepestCommonDep(A, B, F); - assertDeepestCommonDep(A, C, C); - assertDeepestCommonDep(A, C, D); - assertDeepestCommonDep(A, C, E); - assertDeepestCommonDep(A, C, F); - assertDeepestCommonDep(B, D, D); - assertDeepestCommonDep(B, D, E); - assertDeepestCommonDep(B, D, F); - assertDeepestCommonDep(C, E, E); - assertDeepestCommonDep(C, E, F); - assertDeepestCommonDep(E, F, F); + makeDeps(); + makeGraph(); + assertDeepestCommonDep(null, moduleA, moduleA); + assertDeepestCommonDep(null, moduleA, moduleB); + assertDeepestCommonDep(null, moduleA, moduleC); + assertDeepestCommonDep(null, moduleA, moduleD); + assertDeepestCommonDep(null, moduleA, moduleE); + assertDeepestCommonDep(null, moduleA, moduleF); + assertDeepestCommonDep(moduleA, moduleB, moduleB); + assertDeepestCommonDep(moduleA, moduleB, moduleC); + assertDeepestCommonDep(moduleA, moduleB, moduleD); + assertDeepestCommonDep(moduleA, moduleB, moduleE); + assertDeepestCommonDep(moduleA, moduleB, moduleF); + assertDeepestCommonDep(moduleA, moduleC, moduleC); + assertDeepestCommonDep(moduleA, moduleC, moduleD); + assertDeepestCommonDep(moduleA, moduleC, moduleE); + assertDeepestCommonDep(moduleA, moduleC, moduleF); + assertDeepestCommonDep(moduleB, moduleD, moduleD); + assertDeepestCommonDep(moduleB, moduleD, moduleE); + assertDeepestCommonDep(moduleB, moduleD, moduleF); + assertDeepestCommonDep(moduleC, moduleE, moduleE); + assertDeepestCommonDep(moduleC, moduleE, moduleF); + assertDeepestCommonDep(moduleE, moduleF, moduleF); } @Test public void testDeepestCommonDepInclusive() { - assertDeepestCommonDepInclusive(A, A, A); - assertDeepestCommonDepInclusive(A, A, B); - assertDeepestCommonDepInclusive(A, A, C); - assertDeepestCommonDepInclusive(A, A, D); - assertDeepestCommonDepInclusive(A, A, E); - assertDeepestCommonDepInclusive(A, A, F); - assertDeepestCommonDepInclusive(B, B, B); - assertDeepestCommonDepInclusive(A, B, C); - assertDeepestCommonDepInclusive(B, B, D); - assertDeepestCommonDepInclusive(B, B, E); - assertDeepestCommonDepInclusive(B, B, F); - assertDeepestCommonDepInclusive(C, C, C); - assertDeepestCommonDepInclusive(A, C, D); - assertDeepestCommonDepInclusive(C, C, E); - assertDeepestCommonDepInclusive(C, C, F); - assertDeepestCommonDepInclusive(D, D, D); - assertDeepestCommonDepInclusive(B, D, E); - assertDeepestCommonDepInclusive(B, D, F); - assertDeepestCommonDepInclusive(E, E, E); - assertDeepestCommonDepInclusive(E, E, F); - assertDeepestCommonDepInclusive(F, F, F); + makeDeps(); + makeGraph(); + assertDeepestCommonDepInclusive(moduleA, moduleA, moduleA); + assertDeepestCommonDepInclusive(moduleA, moduleA, moduleB); + assertDeepestCommonDepInclusive(moduleA, moduleA, moduleC); + assertDeepestCommonDepInclusive(moduleA, moduleA, moduleD); + assertDeepestCommonDepInclusive(moduleA, moduleA, moduleE); + assertDeepestCommonDepInclusive(moduleA, moduleA, moduleF); + assertDeepestCommonDepInclusive(moduleB, moduleB, moduleB); + assertDeepestCommonDepInclusive(moduleA, moduleB, moduleC); + assertDeepestCommonDepInclusive(moduleB, moduleB, moduleD); + assertDeepestCommonDepInclusive(moduleB, moduleB, moduleE); + assertDeepestCommonDepInclusive(moduleB, moduleB, moduleF); + assertDeepestCommonDepInclusive(moduleC, moduleC, moduleC); + assertDeepestCommonDepInclusive(moduleA, moduleC, moduleD); + assertDeepestCommonDepInclusive(moduleC, moduleC, moduleE); + assertDeepestCommonDepInclusive(moduleC, moduleC, moduleF); + assertDeepestCommonDepInclusive(moduleD, moduleD, moduleD); + assertDeepestCommonDepInclusive(moduleB, moduleD, moduleE); + assertDeepestCommonDepInclusive(moduleB, moduleD, moduleF); + assertDeepestCommonDepInclusive(moduleE, moduleE, moduleE); + assertDeepestCommonDepInclusive(moduleE, moduleE, moduleF); + assertDeepestCommonDepInclusive(moduleF, moduleF, moduleF); } @Test public void testSmallestCoveringSubtree() { - assertSmallestCoveringSubtree(A, A, A, A); - assertSmallestCoveringSubtree(A, A, A, B); - assertSmallestCoveringSubtree(A, A, A, C); - assertSmallestCoveringSubtree(A, A, A, D); - assertSmallestCoveringSubtree(A, A, A, E); - assertSmallestCoveringSubtree(A, A, A, F); - assertSmallestCoveringSubtree(B, A, B, B); - assertSmallestCoveringSubtree(A, A, B, C); - assertSmallestCoveringSubtree(B, A, B, D); - assertSmallestCoveringSubtree(B, A, B, E); - assertSmallestCoveringSubtree(B, A, B, F); - assertSmallestCoveringSubtree(C, A, C, C); - assertSmallestCoveringSubtree(A, A, C, D); - assertSmallestCoveringSubtree(C, A, C, E); - assertSmallestCoveringSubtree(C, A, C, F); - assertSmallestCoveringSubtree(D, A, D, D); - assertSmallestCoveringSubtree(B, A, D, E); - assertSmallestCoveringSubtree(B, A, D, F); - assertSmallestCoveringSubtree(E, A, E, E); - assertSmallestCoveringSubtree(E, A, E, F); - assertSmallestCoveringSubtree(F, A, F, F); + makeDeps(); + makeGraph(); + assertSmallestCoveringSubtree(moduleA, moduleA, moduleA, moduleA); + assertSmallestCoveringSubtree(moduleA, moduleA, moduleA, moduleB); + assertSmallestCoveringSubtree(moduleA, moduleA, moduleA, moduleC); + assertSmallestCoveringSubtree(moduleA, moduleA, moduleA, moduleD); + assertSmallestCoveringSubtree(moduleA, moduleA, moduleA, moduleE); + assertSmallestCoveringSubtree(moduleA, moduleA, moduleA, moduleF); + assertSmallestCoveringSubtree(moduleB, moduleA, moduleB, moduleB); + assertSmallestCoveringSubtree(moduleA, moduleA, moduleB, moduleC); + assertSmallestCoveringSubtree(moduleB, moduleA, moduleB, moduleD); + assertSmallestCoveringSubtree(moduleB, moduleA, moduleB, moduleE); + assertSmallestCoveringSubtree(moduleB, moduleA, moduleB, moduleF); + assertSmallestCoveringSubtree(moduleC, moduleA, moduleC, moduleC); + assertSmallestCoveringSubtree(moduleA, moduleA, moduleC, moduleD); + assertSmallestCoveringSubtree(moduleC, moduleA, moduleC, moduleE); + assertSmallestCoveringSubtree(moduleC, moduleA, moduleC, moduleF); + assertSmallestCoveringSubtree(moduleD, moduleA, moduleD, moduleD); + assertSmallestCoveringSubtree(moduleB, moduleA, moduleD, moduleE); + assertSmallestCoveringSubtree(moduleB, moduleA, moduleD, moduleF); + assertSmallestCoveringSubtree(moduleE, moduleA, moduleE, moduleE); + assertSmallestCoveringSubtree(moduleE, moduleA, moduleE, moduleF); + assertSmallestCoveringSubtree(moduleF, moduleA, moduleF, moduleF); } @Test public void testGetTransitiveDepsDeepestFirst() { - assertTransitiveDepsDeepestFirst(A); - assertTransitiveDepsDeepestFirst(B, A); - assertTransitiveDepsDeepestFirst(C, A); - assertTransitiveDepsDeepestFirst(D, B, A); - assertTransitiveDepsDeepestFirst(E, C, B, A); - assertTransitiveDepsDeepestFirst(F, E, C, B, A); + makeDeps(); + makeGraph(); + assertTransitiveDepsDeepestFirst(moduleA); + assertTransitiveDepsDeepestFirst(moduleB, moduleA); + assertTransitiveDepsDeepestFirst(moduleC, moduleA); + assertTransitiveDepsDeepestFirst(moduleD, moduleB, moduleA); + assertTransitiveDepsDeepestFirst(moduleE, moduleC, moduleB, moduleA); + assertTransitiveDepsDeepestFirst(moduleF, moduleE, moduleC, moduleB, moduleA); } @Test public void testManageDependenciesLooseWithoutEntryPoint() throws Exception { + makeDeps(); + makeGraph(); setUpManageDependenciesTest(); DependencyOptions depOptions = DependencyOptions.pruneLegacyForEntryPoints(ImmutableList.of()); List results = graph.manageDependencies(depOptions); - assertInputs(A, "a1", "a3"); - assertInputs(B, "a2", "b2"); - assertInputs(C); // no inputs - assertInputs(E, "c1", "e1", "e2"); + assertInputs(moduleA, "a1", "a3"); + assertInputs(moduleB, "a2", "b2"); + assertInputs(moduleC); // no inputs + assertInputs(moduleE, "c1", "e1", "e2"); assertThat(sourceNames(results)) .isEqualTo(ImmutableList.of("a1", "a3", "a2", "b2", "c1", "e1", "e2")); @@ -219,16 +349,18 @@ public void testManageDependenciesLooseWithoutEntryPoint() throws Exception { @Test public void testManageDependenciesLooseWithEntryPoint() throws Exception { + makeDeps(); + makeGraph(); setUpManageDependenciesTest(); DependencyOptions depOptions = DependencyOptions.pruneLegacyForEntryPoints( ImmutableList.of(ModuleIdentifier.forClosure("c2"))); List results = graph.manageDependencies(depOptions); - assertInputs(A, "a1", "a3"); - assertInputs(B, "a2", "b2"); - assertInputs(C, "c1", "c2"); - assertInputs(E, "e1", "e2"); + assertInputs(moduleA, "a1", "a3"); + assertInputs(moduleB, "a2", "b2"); + assertInputs(moduleC, "c1", "c2"); + assertInputs(moduleE, "e1", "e2"); assertThat(sourceNames(results)) .isEqualTo(ImmutableList.of("a1", "a3", "a2", "b2", "c1", "c2", "e1", "e2")); @@ -236,6 +368,8 @@ public void testManageDependenciesLooseWithEntryPoint() throws Exception { @Test public void testManageDependenciesStrictWithEntryPoint() throws Exception { + makeDeps(); + makeGraph(); setUpManageDependenciesTest(); DependencyOptions depOptions = DependencyOptions.pruneForEntryPoints(ImmutableList.of(ModuleIdentifier.forClosure("c2"))); @@ -243,23 +377,25 @@ public void testManageDependenciesStrictWithEntryPoint() throws Exception { // Everything gets pushed up into module c, because that's // the only one that has entry points. - assertInputs(A); - assertInputs(B); - assertInputs(C, "a1", "c1", "c2"); - assertInputs(E); + assertInputs(moduleA); + assertInputs(moduleB); + assertInputs(moduleC, "a1", "c1", "c2"); + assertInputs(moduleE); assertThat(sourceNames(results)).containsExactly("a1", "c1", "c2").inOrder(); } @Test public void testManageDependenciesSortOnly() throws Exception { + makeDeps(); + makeGraph(); setUpManageDependenciesTest(); List results = graph.manageDependencies(DependencyOptions.sortOnly()); - assertInputs(A, "a1", "a2", "a3"); - assertInputs(B, "b1", "b2"); - assertInputs(C, "c1", "c2"); - assertInputs(E, "e1", "e2"); + assertInputs(moduleA, "a1", "a2", "a3"); + assertInputs(moduleB, "b1", "b2"); + assertInputs(moduleC, "c1", "c2"); + assertInputs(moduleE, "e1", "e2"); assertThat(sourceNames(results)) .isEqualTo(ImmutableList.of("a1", "a2", "a3", "b1", "b2", "c1", "c2", "e1", "e2")); @@ -271,31 +407,37 @@ public void testManageDependenciesSortOnly() throws Exception { @Test public void testManageDependenciesSortOnlyImpl() throws Exception { - A.add(code("a2", provides("a2"), requires("a1"))); - A.add(code("a1", provides("a1"), requires())); - A.add(code("base.js", BASEJS, provides(), requires())); + makeDeps(); + makeGraph(); + moduleA.add(code("a2", provides("a2"), requires("a1"))); + moduleA.add(code("a1", provides("a1"), requires())); + moduleA.add(code("base.js", BASEJS, provides(), requires())); - for (CompilerInput input : A.getInputs()) { + for (CompilerInput input : moduleA.getInputs()) { input.setCompiler(compiler); } List results = graph.manageDependencies(DependencyOptions.sortOnly()); - assertInputs(A, "base.js", "a1", "a2"); + assertInputs(moduleA, "base.js", "a1", "a2"); assertThat(sourceNames(results)).containsExactly("base.js", "a1", "a2").inOrder(); } @Test public void testNoFiles() throws Exception { + makeDeps(); + makeGraph(); List results = graph.manageDependencies(DependencyOptions.sortOnly()); assertThat(results).isEmpty(); } @Test public void testToJson() { + makeDeps(); + makeGraph(); JsonArray modules = graph.toJson(); - assertThat(modules.size()).isEqualTo(6); + assertThat(modules.size()).isEqualTo(7); for (int i = 0; i < modules.size(); i++) { JsonObject m = modules.get(i).getAsJsonObject(); assertThat(m.get("name")).isNotNull(); @@ -304,8 +446,8 @@ public void testToJson() { assertThat(m.get("inputs")).isNotNull(); } JsonObject m = modules.get(3).getAsJsonObject(); - assertThat(m.get("name").getAsString()).isEqualTo("D"); - assertThat(m.get("dependencies").getAsJsonArray().toString()).isEqualTo("[\"B\"]"); + assertThat(m.get("name").getAsString()).isEqualTo("moduleD"); + assertThat(m.get("dependencies").getAsJsonArray().toString()).isEqualTo("[\"moduleB\"]"); assertThat(m.get("transitive-dependencies").getAsJsonArray().size()).isEqualTo(2); assertThat(m.get("inputs").getAsJsonArray().toString()).isEqualTo("[]"); } @@ -313,23 +455,23 @@ public void testToJson() { private List setUpManageDependenciesTest() { List inputs = new ArrayList<>(); - A.add(code("a1", provides("a1"), requires())); - A.add(code("a2", provides("a2"), requires("a1"))); - A.add(code("a3", provides(), requires("a1"))); + moduleA.add(code("a1", provides("a1"), requires())); + moduleA.add(code("a2", provides("a2"), requires("a1"))); + moduleA.add(code("a3", provides(), requires("a1"))); - B.add(code("b1", provides("b1"), requires("a2"))); - B.add(code("b2", provides(), requires("a1", "a2"))); + moduleB.add(code("b1", provides("b1"), requires("a2"))); + moduleB.add(code("b2", provides(), requires("a1", "a2"))); - C.add(code("c1", provides("c1"), requires("a1"))); - C.add(code("c2", provides("c2"), requires("c1"))); + moduleC.add(code("c1", provides("c1"), requires("a1"))); + moduleC.add(code("c2", provides("c2"), requires("c1"))); - E.add(code("e1", provides(), requires("c1"))); - E.add(code("e2", provides(), requires("c1"))); + moduleE.add(code("e1", provides(), requires("c1"))); + moduleE.add(code("e2", provides(), requires("c1"))); - inputs.addAll(A.getInputs()); - inputs.addAll(B.getInputs()); - inputs.addAll(C.getInputs()); - inputs.addAll(E.getInputs()); + inputs.addAll(moduleA.getInputs()); + inputs.addAll(moduleB.getInputs()); + inputs.addAll(moduleC.getInputs()); + inputs.addAll(moduleE.getInputs()); for (CompilerInput input : inputs) { input.setCompiler(compiler); @@ -339,6 +481,8 @@ private List setUpManageDependenciesTest() { @Test public void testGoogBaseOrderedCorrectly() throws Exception { + makeDeps(); + makeGraph(); List sourceFiles = new ArrayList<>(); sourceFiles.add(code("a9", provides("a9"), requires())); sourceFiles.add(code("a8", provides("a8"), requires())); @@ -356,18 +500,18 @@ public void testGoogBaseOrderedCorrectly() throws Exception { DependencyOptions.pruneForEntryPoints(ImmutableList.of(ModuleIdentifier.forClosure("a1"))); for (int i = 0; i < 10; i++) { shuffle(sourceFiles); - A.removeAll(); + moduleA.removeAll(); for (SourceFile sourceFile : sourceFiles) { - A.add(sourceFile); + moduleA.add(sourceFile); } - for (CompilerInput input : A.getInputs()) { + for (CompilerInput input : moduleA.getInputs()) { input.setCompiler(compiler); } List results = graph.manageDependencies(depOptions); - assertInputs(A, "base.js", "a2", "a3", "a4", "a5", "a6", "a7", "a8", "a9", "a1"); + assertInputs(moduleA, "base.js", "a2", "a3", "a4", "a5", "a6", "a7", "a8", "a9", "a1"); assertThat(sourceNames(results)) .containsExactly("base.js", "a2", "a3", "a4", "a5", "a6", "a7", "a8", "a9", "a1") @@ -377,6 +521,8 @@ public void testGoogBaseOrderedCorrectly() throws Exception { @Test public void testProperEs6ModuleOrdering() throws Exception { + makeDeps(); + makeGraph(); List sourceFiles = new ArrayList<>(); sourceFiles.add(code("/entry.js", provides(), requires())); sourceFiles.add(code("/a/a.js", provides(), requires())); @@ -408,12 +554,12 @@ public void testProperEs6ModuleOrdering() throws Exception { ImmutableList.of(ModuleIdentifier.forFile("/entry.js"))); for (int iterationCount = 0; iterationCount < 10; iterationCount++) { shuffle(sourceFiles); - A.removeAll(); + moduleA.removeAll(); for (SourceFile sourceFile : sourceFiles) { - A.add(sourceFile); + moduleA.add(sourceFile); } - for (CompilerInput input : A.getInputs()) { + for (CompilerInput input : moduleA.getInputs()) { input.setCompiler(compiler); for (String require : orderedRequires.get(input.getSourceFile().getName())) { input.addOrderedRequire(Require.compilerModule(require)); @@ -424,7 +570,14 @@ public void testProperEs6ModuleOrdering() throws Exception { List results = graph.manageDependencies(depOptions); assertInputs( - A, "/b/c.js", "/b/b.js", "/b/a.js", "/important.js", "/a/b.js", "/a/a.js", "/entry.js"); + moduleA, + "/b/c.js", + "/b/b.js", + "/b/a.js", + "/important.js", + "/a/b.js", + "/a/a.js", + "/entry.js"); assertThat(sourceNames(results)) .containsExactly( @@ -433,6 +586,193 @@ public void testProperEs6ModuleOrdering() throws Exception { } } + @Test + public void testMoveMarkedWeakSources() throws Exception { + makeDeps(); + + SourceFile weak1 = SourceFile.fromCode("weak1", "", SourceKind.WEAK); + SourceFile weak2 = SourceFile.fromCode("weak2", "", SourceKind.WEAK); + SourceFile strong1 = SourceFile.fromCode("strong1", "", SourceKind.STRONG); + SourceFile strong2 = SourceFile.fromCode("strong2", "", SourceKind.STRONG); + + moduleA.add(weak1); + moduleA.add(strong1); + moduleA.add(weak2); + moduleA.add(strong2); + + for (CompilerInput input : moduleA.getInputs()) { + input.setCompiler(compiler); + } + + makeGraph(); + + assertThat( + getWeakModule().getInputs().stream() + .map(i -> i.getSourceFile()) + .collect(Collectors.toList())) + .containsExactly(weak1, weak2); + assertThat( + moduleA.getInputs().stream().map(i -> i.getSourceFile()).collect(Collectors.toList())) + .containsExactly(strong1, strong2); + } + + @Test + public void testMoveMarkedWeakSourcesDuringManageDepsSortOnly() throws Exception { + makeDeps(); + + SourceFile weak1 = SourceFile.fromCode("weak1", "", SourceKind.WEAK); + SourceFile weak2 = SourceFile.fromCode("weak2", "", SourceKind.WEAK); + SourceFile strong1 = SourceFile.fromCode("strong1", "", SourceKind.STRONG); + SourceFile strong2 = SourceFile.fromCode("strong2", "", SourceKind.STRONG); + + moduleA.add(weak1); + moduleA.add(strong1); + moduleA.add(weak2); + moduleA.add(strong2); + + for (CompilerInput input : moduleA.getInputs()) { + input.setCompiler(compiler); + } + + makeGraph(); + graph.manageDependencies(DependencyOptions.sortOnly()); + + assertThat( + getWeakModule().getInputs().stream() + .map(i -> i.getSourceFile()) + .collect(Collectors.toList())) + .containsExactly(weak1, weak2); + assertThat( + moduleA.getInputs().stream().map(i -> i.getSourceFile()).collect(Collectors.toList())) + .containsExactly(strong1, strong2); + } + + @Test + public void testMoveMarkedWeakSourcesDuringManageDepsPrune() throws Exception { + makeDeps(); + + SourceFile weak1 = SourceFile.fromCode("weak1", "", SourceKind.WEAK); + SourceFile weak2 = SourceFile.fromCode("weak2", "", SourceKind.WEAK); + SourceFile strong1 = SourceFile.fromCode("strong1", "", SourceKind.STRONG); + SourceFile strong2 = SourceFile.fromCode("strong2", "", SourceKind.STRONG); + + moduleA.add(weak1); + moduleA.add(strong1); + moduleA.add(weak2); + moduleA.add(strong2); + + for (CompilerInput input : moduleA.getInputs()) { + input.setCompiler(compiler); + } + + makeGraph(); + graph.manageDependencies( + DependencyOptions.pruneForEntryPoints( + ImmutableList.of( + ModuleIdentifier.forFile("strong1"), ModuleIdentifier.forFile("strong2")))); + + assertThat( + getWeakModule().getInputs().stream() + .map(i -> i.getSourceFile()) + .collect(Collectors.toList())) + .containsExactly(weak1, weak2); + assertThat( + moduleA.getInputs().stream().map(i -> i.getSourceFile()).collect(Collectors.toList())) + .containsExactly(strong1, strong2); + } + + @Test + public void testMoveImplicitWeakSourcesFromMoocherDuringManageDepsLegacyPrune() throws Exception { + makeDeps(); + + SourceFile weak = SourceFile.fromCode("weak", "goog.provide('weak');"); + SourceFile strong = SourceFile.fromCode("strong", ""); + SourceFile moocher = SourceFile.fromCode("moocher", "goog.requireType('weak');"); + + moduleA.add(weak); + moduleA.add(strong); + moduleA.add(moocher); + + for (CompilerInput input : moduleA.getInputs()) { + input.setCompiler(compiler); + } + + makeGraph(); + graph.manageDependencies( + DependencyOptions.pruneLegacyForEntryPoints( + ImmutableList.of(ModuleIdentifier.forFile("strong")))); + + assertThat( + getWeakModule().getInputs().stream() + .map(i -> i.getSourceFile()) + .collect(Collectors.toList())) + .containsExactly(weak); + assertThat( + moduleA.getInputs().stream().map(i -> i.getSourceFile()).collect(Collectors.toList())) + .containsExactly(strong, moocher); + } + + @Test + public void testImplicitWeakSourcesNotMovedDuringManageDepsSortOnly() throws Exception { + makeDeps(); + + SourceFile weak1 = SourceFile.fromCode("weak1", "goog.provide('weak1');"); + SourceFile weak2 = SourceFile.fromCode("weak2", "goog.provide('weak2');"); + SourceFile strong1 = SourceFile.fromCode("strong1", "goog.requireType('weak1');"); + SourceFile strong2 = SourceFile.fromCode("strong2", "goog.requireType('weak2');"); + + moduleA.add(weak1); + moduleA.add(strong1); + moduleA.add(weak2); + moduleA.add(strong2); + + for (CompilerInput input : moduleA.getInputs()) { + input.setCompiler(compiler); + } + + makeGraph(); + graph.manageDependencies(DependencyOptions.sortOnly()); + + assertThat(getWeakModule().getInputs()).isEmpty(); + assertThat( + moduleA.getInputs().stream().map(i -> i.getSourceFile()).collect(Collectors.toList())) + .containsExactly(weak1, strong1, weak2, strong2); + } + + @Test + public void testImplicitWeakSourcesMovedDuringManageDepsPrune() throws Exception { + makeDeps(); + + SourceFile weak1 = SourceFile.fromCode("weak1", "goog.provide('weak1');"); + SourceFile weak2 = SourceFile.fromCode("weak2", "goog.provide('weak2');"); + SourceFile strong1 = SourceFile.fromCode("strong1", "goog.requireType('weak1');"); + SourceFile strong2 = SourceFile.fromCode("strong2", "goog.requireType('weak2');"); + + moduleA.add(weak1); + moduleA.add(strong1); + moduleA.add(weak2); + moduleA.add(strong2); + + for (CompilerInput input : moduleA.getInputs()) { + input.setCompiler(compiler); + } + + makeGraph(); + graph.manageDependencies( + DependencyOptions.pruneForEntryPoints( + ImmutableList.of( + ModuleIdentifier.forFile("strong1"), ModuleIdentifier.forFile("strong2")))); + + assertThat( + getWeakModule().getInputs().stream() + .map(i -> i.getSourceFile()) + .collect(Collectors.toList())) + .containsExactly(weak1, weak2); + assertThat( + moduleA.getInputs().stream().map(i -> i.getSourceFile()).collect(Collectors.toList())) + .containsExactly(strong1, strong2); + } + private void assertInputs(JSModule module, String... sourceNames) { assertThat(sourceNames(module.getInputs())).isEqualTo(ImmutableList.copyOf(sourceNames)); } diff --git a/test/com/google/javascript/jscomp/deps/Es6SortedDependenciesTest.java b/test/com/google/javascript/jscomp/deps/Es6SortedDependenciesTest.java index b11c941ab02..2eb890f4911 100644 --- a/test/com/google/javascript/jscomp/deps/Es6SortedDependenciesTest.java +++ b/test/com/google/javascript/jscomp/deps/Es6SortedDependenciesTest.java @@ -344,6 +344,57 @@ public void testSort10() { assertThat(sorted.getSortedList()).containsExactly(c, a, b).inOrder(); } + @Test + public void testWeakSort() throws Exception { + SimpleDependencyInfo a = + SimpleDependencyInfo.builder("a", "a").setProvides("a").setTypeRequires("b").build(); + SimpleDependencyInfo b = + SimpleDependencyInfo.builder("b", "b").setProvides("b").setTypeRequires("c").build(); + SimpleDependencyInfo c = SimpleDependencyInfo.builder("c", "c").setProvides("c").build(); + + assertSortedWeakDeps(ImmutableList.of(c, b), ImmutableList.of(a, b, c), ImmutableList.of(a)); + + assertSortedWeakDeps(ImmutableList.of(c, b), ImmutableList.of(a, c, b), ImmutableList.of(a)); + + assertSortedWeakDeps(ImmutableList.of(c, b), ImmutableList.of(b, c, a), ImmutableList.of(a)); + + assertSortedWeakDeps(ImmutableList.of(c, b), ImmutableList.of(c, b, a), ImmutableList.of(a)); + } + + @Test + public void testWeakSortIncludesStrongEdgesFromWeakSources() throws Exception { + SimpleDependencyInfo a = + SimpleDependencyInfo.builder("a", "a").setProvides("a").setTypeRequires("b").build(); + SimpleDependencyInfo b = + SimpleDependencyInfo.builder("b", "b") + .setProvides("b") + .setRequires(Require.googRequireSymbol("c")) + .build(); + SimpleDependencyInfo c = SimpleDependencyInfo.builder("c", "c").setProvides("c").build(); + + assertSortedWeakDeps(ImmutableList.of(c, b), ImmutableList.of(a, c, b), ImmutableList.of(a)); + } + + @Test + public void testWeakAndStrongIsStrong() throws Exception { + SimpleDependencyInfo a = + SimpleDependencyInfo.builder("a", "a") + .setProvides("a") + .setRequires(Require.googRequireSymbol("c")) + .build(); + SimpleDependencyInfo b = + SimpleDependencyInfo.builder("b", "b").setProvides("b").setTypeRequires("c").build(); + SimpleDependencyInfo c = SimpleDependencyInfo.builder("c", "c").setProvides("c").build(); + + assertSortedWeakDeps(ImmutableList.of(), ImmutableList.of(a, b, c), ImmutableList.of(a, b)); + + assertSortedWeakDeps(ImmutableList.of(), ImmutableList.of(a, b, c), ImmutableList.of(b, a)); + + assertSortedWeakDeps(ImmutableList.of(), ImmutableList.of(c, b, a), ImmutableList.of(a, b)); + + assertSortedWeakDeps(ImmutableList.of(), ImmutableList.of(c, a, b), ImmutableList.of(b, a)); + } + private static void assertSortedInputs( List expected, List shuffled) { SortedDependencies sorted = createSortedDependencies(shuffled); @@ -356,7 +407,15 @@ private static void assertSortedDeps( List roots) throws Exception { SortedDependencies sorted = createSortedDependencies(shuffled); - assertThat(sorted.getSortedDependenciesOf(roots)).isEqualTo(expected); + assertThat(sorted.getSortedStrongDependenciesOf(roots)).isEqualTo(expected); + } + + private static void assertSortedWeakDeps( + List expected, + List shuffled, + List roots) { + SortedDependencies sorted = createSortedDependencies(shuffled); + assertThat(sorted.getSortedWeakDependenciesOf(roots)).isEqualTo(expected); } private static void assertOrder(