diff --git a/src/com/google/javascript/jscomp/CrossModuleCodeMotion.java b/src/com/google/javascript/jscomp/CrossModuleCodeMotion.java index 58079477b09..fc7f8595028 100644 --- a/src/com/google/javascript/jscomp/CrossModuleCodeMotion.java +++ b/src/com/google/javascript/jscomp/CrossModuleCodeMotion.java @@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; +import com.google.common.base.Objects; import com.google.javascript.jscomp.CrossModuleReferenceCollector.TopLevelStatement; import com.google.javascript.rhino.IR; import com.google.javascript.rhino.Node; @@ -27,24 +28,39 @@ import java.util.BitSet; import java.util.Deque; import java.util.HashMap; -import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.Map; -import java.util.Set; +import java.util.logging.Logger; /** - * A compiler pass for moving global variable declarations and assignments to their properties to a - * deeper module if possible. + * A compiler pass for moving code to a deeper module if possible. + * - currently it only moves functions + variables + * */ class CrossModuleCodeMotion implements CompilerPass { + private static final Logger logger = + Logger.getLogger(CrossModuleCodeMotion.class.getName()); + private final AbstractCompiler compiler; private final JSModuleGraph graph; /** - * Map from module to the node in that module that should parent variable declarations that have - * to be moved into that module + * Map from module to the node in that module that should parent any string + * variable declarations that have to be moved into that module + */ + private final Map moduleVarParentMap = + new HashMap<>(); + + /* + * NOTE - I made this a LinkedHashMap to make testing easier. With a regular + * HashMap, the variables may not output in a consistent order */ - private final Map moduleInsertionPointMap = new HashMap<>(); + private final Map namedInfo = + new LinkedHashMap<>(); + + private final Map instanceofNodes = + new LinkedHashMap<>(); private final boolean parentModuleCanSeeSymbolsDeclaredInChildren; @@ -59,503 +75,215 @@ class CrossModuleCodeMotion implements CompilerPass { boolean parentModuleCanSeeSymbolsDeclaredInChildren) { this.compiler = compiler; this.graph = graph; - this.parentModuleCanSeeSymbolsDeclaredInChildren = parentModuleCanSeeSymbolsDeclaredInChildren; + this.parentModuleCanSeeSymbolsDeclaredInChildren = + parentModuleCanSeeSymbolsDeclaredInChildren; } @Override public void process(Node externs, Node root) { + logger.fine("Moving functions + variable into deeper modules"); + // If there are <2 modules, then we will never move anything, so we're done if (graph != null && graph.getModuleCount() > 1) { - CrossModuleReferenceCollector referenceCollector = - new CrossModuleReferenceCollector(compiler, new Es6SyntacticScopeCreator(compiler)); - referenceCollector.process(root); - Deque declarationStatementGroups = - new DeclarationStatementGroupCollector() - .collectDeclarationStatementGroups(referenceCollector); - moveDeclarationStatementGroups(declarationStatementGroups); - } - } - private class DeclarationStatementGroupCollector { + // Traverse the tree and find the modules where a var is declared + used + collectReferences(root); - /** Stack to fill with DeclarationStatementGroups as they are discovered. */ - final Deque allDsgsStack = new ArrayDeque<>(); - /** - * Keep a stack of DeclarationStatementGroups for each global variable. As we traverse the - * statements in execution order order the top of the stack represents the most recently seen - * DSG for the variable. - */ - final Map> dsgStackForVar = new HashMap<>(); - - Deque collectDeclarationStatementGroups( - CrossModuleReferenceCollector referenceCollector) { - - for (TopLevelStatement statement : referenceCollector.getTopLevelStatements()) { - if (statement.isDeclarationStatement()) { - DeclarationStatementGroup statementDsg = addStatementToDsg(statement); - for (Reference ref : statement.getNonDeclarationReferences()) { - processReferenceFromDsg(statement, statementDsg, ref); - } - } else { - for (Reference ref : statement.getNonDeclarationReferences()) { - processImmovableReference(statement, ref); - } - } - } - return allDsgsStack; - } + // Move the functions + variables to a deeper module [if possible] + moveCode(); - /** - * Finds or creates the DeclarationStatementGroup appropriate for this statement, adds the - * statement to it and returns the DSG. - */ - private DeclarationStatementGroup addStatementToDsg(TopLevelStatement statement) { - DeclarationStatementGroup statementDsg; - Var declaredVar = statement.getDeclaredNameReference().getSymbol(); - Deque declaredVarDsgStack = - getVarDsgStack(declaredVar, statement.getModule()); - DeclarationStatementGroup lastDsg = declaredVarDsgStack.peek(); - if (lastDsg.currentModule.equals(statement.getModule())) { - // Still in the same module and no intervening declarations since the last declaration - // for this variable group. - statementDsg = lastDsg; - statementDsg.statementStack.push(statement); - } else { - // New module requires a new DSG - statementDsg = new DeclarationStatementGroup(statement); - // New DSG depends on the previous one - statementDsg.referencedStatementGroups.add(lastDsg); - lastDsg.referencingStatementGroups.add(statementDsg); - declaredVarDsgStack.push(statementDsg); - allDsgsStack.push(statementDsg); + // Make is so we can ignore constructor references in instanceof. + if (parentModuleCanSeeSymbolsDeclaredInChildren) { + addInstanceofGuards(); } - return statementDsg; } + } - private void processReferenceFromDsg( - TopLevelStatement statement, DeclarationStatementGroup statementDsg, Reference ref) { - Var refVar = ref.getSymbol(); - Deque rdsgStack = getVarDsgStack(refVar, statement.getModule()); - DeclarationStatementGroup rdsg = rdsgStack.peek(); - - if (rdsg == statementDsg) { - return; - } else if (parentModuleCanSeeSymbolsDeclaredInChildren) { - // It is possible to move the declaration of `Foo` after - // `'undefined' != typeof Foo && x instanceof Foo`. - // We'll add the undefined check, if necessary. - Node n = ref.getNode(); - if (isGuardedInstanceofReference(n) || isUndefinedTypeofGuardReference(n)) { - return; - } else if (isUnguardedInstanceofReference(n)) { - // add a guard, if rdsg moves - InstanceofReference instanceofReference = - new InstanceofReference(statementDsg.currentModule, ref); - rdsg.instanceofReferencesToGuard.push(instanceofReference); - statementDsg.containedUnguardedInstanceofReferences.push(instanceofReference); - return; + /** move the code accordingly */ + private void moveCode() { + for (NamedInfo info : namedInfo.values()) { + if (info.shouldBeMoved()) { + // Find the appropriate spot to move it to + JSModule preferredModule = info.getPreferredModule(); + Node destParent = moduleVarParentMap.get(preferredModule); + if (destParent == null) { + destParent = compiler.getNodeForCodeInsertion(preferredModule); + moduleVarParentMap.put(preferredModule, destParent); } - } + for (TopLevelStatement declaringStatement : info.movableDeclaringStatementStack) { + Node statementNode = declaringStatement.getStatementNode(); - // reference restricts movement for all DSGs for the referenced variable - for (DeclarationStatementGroup varDsg : rdsgStack) { - // varDsg must be updated when this statement moves - statementDsg.referencedStatementGroups.add(varDsg); - varDsg.referencingStatementGroups.add(statementDsg); - } - } - - /** - * Get the DeclarationStatementGroup stack containing DSGs for the given Var. - * - *

If no stack exists, this means we're seeing the variable for the first time, so we create - * a stack with a single DSG for the current module. - * - * @param refVar the variable - * @param currentModule the module whose statements we're currently examining. - * @return a new or existing stack of DSGs for the given variable - */ - private Deque getVarDsgStack(Var refVar, JSModule currentModule) { - Deque rdsgStack = dsgStackForVar.get(refVar); - if (rdsgStack == null) { - // First reference is in a non-declaration statement. - // Create an empty DSG for this reference, which may be filled in if we find later - // declaration statements for it in the same module. - rdsgStack = new ArrayDeque<>(); - dsgStackForVar.put(refVar, rdsgStack); - DeclarationStatementGroup rdsg = new DeclarationStatementGroup(currentModule); - rdsgStack.push(rdsg); - allDsgsStack.push(rdsg); - } - return rdsgStack; - } + // Remove it + compiler.reportChangeToEnclosingScope(statementNode); + statementNode.detach(); - private void processImmovableReference(TopLevelStatement statement, Reference ref) { - Var refVar = ref.getSymbol(); - Deque rdsgStack = getVarDsgStack(refVar, statement.getModule()); - DeclarationStatementGroup rdsg = rdsgStack.peek(); + // Add it to the new spot + destParent.addChildToFront(statementNode); - if (parentModuleCanSeeSymbolsDeclaredInChildren) { - // It is possible to move the declaration of `Foo` after - // `'undefined' != typeof Foo && x instanceof Foo`. - // We'll add the undefined check, if necessary. - Node n = ref.getNode(); - if (isGuardedInstanceofReference(n) || isUndefinedTypeofGuardReference(n)) { - return; - } else if (isUnguardedInstanceofReference(n)) { - // add a guard, if rdsg moves - InstanceofReference instanceofReference = - new InstanceofReference(statement.getModule(), ref); - rdsg.instanceofReferencesToGuard.push(instanceofReference); - return; + compiler.reportChangeToEnclosingScope(statementNode); } - } - - // reference restricts movement for all DSGs for the referenced variable - for (DeclarationStatementGroup varDsg : rdsgStack) { - varDsg.addImmovableReference(statement.getModule()); + // Update variable declaration location. + info.wasMoved = true; + info.declModule = preferredModule; } } } - /** - * Moves all of the declaration statements that can move to their best possible module location. - * - * @param dsgStack DSGs in last to first order - */ - private void moveDeclarationStatementGroups(Deque dsgStack) { - for (DeclarationStatementGroup dsg : - new OrderAndCombineDeclarationStatementGroups(dsgStack).orderAndCombine()) { - // Ordering enforces that all DSGs with references to this one will have already moved or - // been found to be immovable before this one is considered. - // Their references will have been converted into bits in modulesWithImmovableReferences. - checkState(dsg.referencingStatementGroups.isEmpty()); - moveDeclarationStatementGroupIfPossible(dsg); - - for (DeclarationStatementGroup referencedDsg : dsg.referencedStatementGroups) { - // Convert all reference relationships to immovable references to be considered when it's - // referencedDsg's turn to move. - referencedDsg.addImmovableReference(dsg.currentModule); - // Technically the algorithm would still be correct if we left this reference alone, - // but removing it allows us to include the checkState() above to confirm that we never - // move groups in the wrong order. - referencedDsg.referencingStatementGroups.remove(dsg); - } - // It would just feel wrong to leave this dsg pointing to ones that no longer point back to - // it. This also allows for some GC, but probably not much. - dsg.referencedStatementGroups.clear(); - } - } + /** useful information for each variable candidate */ + private class NamedInfo { + private boolean allowMove = true; - /** - * Orders DeclarationStatementGroups so that each DSG appears in the list only after all of the - * DSGs that contain references to it. - * - *

DSGs that form cycles are combined into a single DSG. This happens when declarations within - * a module form cycles by referring to each other. - * - *

This is an implementation of the path-based strong component algorithm as it is described in - * the Wikipedia article https://en.wikipedia.org/wiki/Path-based_strong_component_algorithm. - */ - private static class OrderAndCombineDeclarationStatementGroups { - final Deque inputDsgs; - /** Tracks DSGs that may be part of a strongly connected component (reference cycle). */ - final Deque componentContents = new ArrayDeque<>(); - /** Tracks DSGs that may be the root of a strongly connected component (reference cycle). */ - final Deque componentRoots = new ArrayDeque<>(); - /** Filled with lists of strongly connected DSGs as they are discovered. */ - final Deque> stronglyConnectedDsgs; - - int preorderCounter = 0; - - OrderAndCombineDeclarationStatementGroups(Deque dsgs) { - inputDsgs = dsgs; - stronglyConnectedDsgs = new ArrayDeque<>(dsgs.size()); - } + // Indices of all modules referring to the global name. + private BitSet modulesWithReferences = null; - public Deque orderAndCombine() { - for (DeclarationStatementGroup dsg : inputDsgs) { - if (dsg.preorderNumber < 0) { - processDsg(dsg); - } // else already processed - } - // At this point stronglyConnectedDsgs has been filled. - Deque results = new ArrayDeque<>(stronglyConnectedDsgs.size()); - for (Deque dsgs : stronglyConnectedDsgs) { - DeclarationStatementGroup resultDsg = dsgs.pop(); - while (!dsgs.isEmpty()) { - DeclarationStatementGroup cycleDsg = dsgs.pop(); - resultDsg.merge(cycleDsg); - } - results.add(resultDsg); - } + // A place to stash the results of getPreferredModule() to avoid recalculating it unnecessarily. + private JSModule preferredModule = null; - return results; - } + // The module where declarations appear + private JSModule declModule = null; - /** - * Determines to which strongly connnected component this DeclarationStatementGroup belongs. - * - *

Called exactly once for each DSG. When this method returns we will have determined which - * strongly connnected component contains dsg. Either: - * - *

    - *
  • dsg was the first member of the strongly connected component we encountered, and the - * entire component has now been added to stronglyConnectedDsgs. - *
  • OR, we encountered the first member of the strongly connected component in an earlier - * call to this method on the call stack. In this case the top of componentRoots will be - * the first member, and we'll add the component to stronglyConnectedDsgs once execution - * has fallen back to the call made with that DSG. - *
- */ - void processDsg(DeclarationStatementGroup dsg) { - // preorderNumber is used to track whether we've already processed a DSG and the order in - // which they have been processed. It is initially -1. - checkState(dsg.preorderNumber < 0, "already processed: %s", dsg); - dsg.preorderNumber = preorderCounter++; - componentRoots.push(dsg); // could be the start of a new strongly connected component - componentContents.push(dsg); // could be part of an existing strongly connected component - for (DeclarationStatementGroup referringDsg : dsg.referencingStatementGroups) { - if (referringDsg.preorderNumber < 0) { - processDsg(referringDsg); + private boolean wasMoved = false; + + /** Stack of declaring statements. Last in is first to be moved. */ + private final Deque movableDeclaringStatementStack = new ArrayDeque<>(); + + void addMovableDeclaringStatement(TopLevelStatement declaringStatement) { + // Ignore declaring statements once we've decided we cannot move them. + if (allowMove) { + if (modulesWithReferences != null) { + // We've already started seeing non-declaration references, so we cannot actually + // move this statement and must treat it like a non-declaration reference. + addReferringStatement(declaringStatement); + } else if (declModule == null) { + declModule = declaringStatement.getModule(); + movableDeclaringStatementStack.push(declaringStatement); + } else if (declModule.equals(declaringStatement.getModule())) { + movableDeclaringStatementStack.push(declaringStatement); } else { - if (!referringDsg.hasBeenAssignedToAStronglyConnectedComponent) { - // This DSG is part of a not-yet-completed strongly connected component. - // Back off the potential roots stack to the earliest DSG that is part of the - // component. - while (componentRoots.peek().preorderNumber > referringDsg.preorderNumber) { - componentRoots.pop(); - } - } + // Cannot move declarations not in the same module with the first declaration. + addReferringStatement(declaringStatement); } } - if (componentRoots.peek().equals(dsg)) { - // After exploring all paths from here, this DSG is still at the top of the potential - // component roots stack, so this DSG is the root of a strongly connected component. - componentRoots.pop(); - Deque stronglyConnectedDsgs = new ArrayDeque<>(); - // Gather this DSG and all of the ones we visited that are part of this component. - DeclarationStatementGroup connectedDsg; - do { - connectedDsg = componentContents.pop(); - stronglyConnectedDsgs.push(connectedDsg); - connectedDsg.hasBeenAssignedToAStronglyConnectedComponent = true; - } while (!connectedDsg.equals(dsg)); - this.stronglyConnectedDsgs.add(stronglyConnectedDsgs); - } - } - } - - private void moveDeclarationStatementGroupIfPossible(DeclarationStatementGroup dsg) { - JSModule preferredModule = dsg.getPreferredModule(); - if (preferredModule == dsg.currentModule) { - return; - } - Node destParent = moduleInsertionPointMap.get(preferredModule); - if (destParent == null) { - destParent = compiler.getNodeForCodeInsertion(preferredModule); - moduleInsertionPointMap.put(preferredModule, destParent); } - for (TopLevelStatement statement : dsg.statementStack) { - Node statementNode = statement.getStatementNode(); - // Remove it - compiler.reportChangeToEnclosingScope(statementNode); - statementNode.detach(); - - // Add it to the new spot - destParent.addChildToFront(statementNode); - compiler.reportChangeToEnclosingScope(statementNode); - } - dsg.currentModule = preferredModule; - // update any unguarded instanceof references contained in this statement group - for (InstanceofReference instanceofReference : dsg.containedUnguardedInstanceofReferences) { - instanceofReference.module = preferredModule; - } - guardInstanceofReferences(dsg); - } - - /** - * Adds guards to any instanceof references to this statement group that might execute before the - * statements in this DeclarationStatementGroup. - */ - private void guardInstanceofReferences(DeclarationStatementGroup dsg) { - for (InstanceofReference instanceofRef : dsg.instanceofReferencesToGuard) { - if (!graph.dependsOn(instanceofRef.module, dsg.currentModule)) { - Node referenceNode = instanceofRef.reference.getNode(); - checkState( - isUnguardedInstanceofReference(referenceNode), - "instanceof Reference is already guarded: %s", - referenceNode); - Node instanceofNode = checkNotNull(referenceNode.getParent()); - Node referenceForTypeOf = referenceNode.cloneNode(); - Node tmp = IR.block(); - // Wrap "foo instanceof Bar" in - // "('undefined' != typeof Bar && foo instanceof Bar)" - instanceofNode.replaceWith(tmp); - Node and = - IR.and( - new Node( - Token.NE, IR.string("undefined"), new Node(Token.TYPEOF, referenceForTypeOf)), - instanceofNode); - and.useSourceInfoIfMissingFromForTree(instanceofNode); - tmp.replaceWith(and); - compiler.reportChangeToEnclosingScope(and); + void addReferringStatement(TopLevelStatement referringStatement) { + // Ignore referring statements if we cannot move declaration statements anyway. + if (allowMove) { + if (declModule == null) { + // First reference we see is not a declaration, so we cannot move any declaration + // statements. + allowMove = false; + } else if (referringStatement.getModule().equals(declModule)) { + // The first non-declaration reference we see is in the same module as the declaration + // statements, so we cannot move them. + allowMove = false; + movableDeclaringStatementStack.clear(); // save some memory + modulesWithReferences = null; + } else { + addUsedModule(referringStatement.getModule()); + } } } - } - - /** - * A group of declaration statements that must be moved (or not) as a group. - * - *

All of the statements must be in the same module initially. If there are declarations for - * the same variable in different modules, they will be grouped separately and the later module's - * group will refer to the earlier one. - */ - private class DeclarationStatementGroup { - /** module containing the statements */ - JSModule currentModule; - /** statements in the group, latest first */ - Deque statementStack = new ArrayDeque<>(); - - BitSet modulesWithImmovableReferences = new BitSet(graph.getModuleCount()); - /** - * DSGs containing references to this one that aren't covered by modulesWithImmovableReferences. - */ - Set referencingStatementGroups = new HashSet<>(); - /** - * This DSG has references to these that aren't covered by their modulesWithImmovableReferences. - */ - Set referencedStatementGroups = new HashSet<>(); - /** Instanceof references that may need to be guarded if this DSG moves. */ - Deque instanceofReferencesToGuard = new ArrayDeque<>(); - /** Unguarded instanceof references that will move with this DSG. */ - Deque containedUnguardedInstanceofReferences = new ArrayDeque<>(); - /** Used by OrderAndCombineDeclarationStatementGroups */ - int preorderNumber = -1; - /** Used by OrderAndCombineDeclarationStatementGroups */ - boolean hasBeenAssignedToAStronglyConnectedComponent = false; - - /** Creates a DSG for the case where the first reference we see is a declaration. */ - DeclarationStatementGroup(TopLevelStatement initialStatement) { - this(initialStatement.getModule()); - statementStack.push(initialStatement); + // Add a Module where it is used + private void addUsedModule(JSModule m) { + if (modulesWithReferences == null) { + // first call to this method + modulesWithReferences = new BitSet(graph.getModuleCount()); + } + modulesWithReferences.set(m.getIndex()); + // invalidate preferredModule, so it will be recalculated next time getPreferredModule() is + // called. + preferredModule = null; } /** - * Creates a DSG with no statements for the case where the first reference seen is not a - * declaration. + * Returns the root module of a dependency subtree that contains all of the modules which refer + * to this global name. */ - DeclarationStatementGroup(JSModule initialReferenceModule) { - this.currentModule = initialReferenceModule; - } - - void addImmovableReference(JSModule module) { - modulesWithImmovableReferences.set(module.getIndex()); - } - JSModule getPreferredModule() { - if (!allStatementsCanMove()) { - return currentModule; - } else if (modulesWithImmovableReferences.isEmpty()) { - // no information to use to choose a different module - return currentModule; - } else { - return graph.getSmallestCoveringSubtree(currentModule, modulesWithImmovableReferences); - } - } - - boolean allStatementsCanMove() { - for (TopLevelStatement s : statementStack) { - if (!s.isMovableDeclaration()) { - return false; + if (preferredModule == null) { + if (modulesWithReferences == null) { + // If we saw no references, we must at least have seen a declaration. + preferredModule = checkNotNull(declModule); + } else { + // Note that getSmallestCoveringDependency() will do this: + // checkState(!modulesWithReferences.isEmpty()) + preferredModule = graph.getSmallestCoveringDependency(modulesWithReferences); } } - return true; + return preferredModule; } - /** - * Combines the information from another DeclarationStatementGroup into this one, effectively - * making the other empty. - */ - void merge(DeclarationStatementGroup other) { - checkState( - currentModule.equals(other.currentModule), - "Attempt to merge declarations from %s and %s", - currentModule, - other.currentModule); - // statement order on the stack must be last statement first according to the original source - // order. - statementStack = mergeStatementsLatestFirst(statementStack, other.statementStack); - modulesWithImmovableReferences.or(other.modulesWithImmovableReferences); - for (DeclarationStatementGroup rdsg : other.referencingStatementGroups) { - rdsg.referencedStatementGroups.remove(other); - if (!rdsg.equals(this)) { - referencingStatementGroups.add(rdsg); - rdsg.referencedStatementGroups.add(this); - } - } - other.referencingStatementGroups.clear(); - for (DeclarationStatementGroup rdsg : other.referencedStatementGroups) { - rdsg.referencingStatementGroups.remove(other); - if (!rdsg.equals(this)) { - referencedStatementGroups.add(rdsg); - rdsg.referencingStatementGroups.add(this); - } - } - other.referencedStatementGroups.clear(); - instanceofReferencesToGuard.addAll(other.instanceofReferencesToGuard); - other.instanceofReferencesToGuard.clear(); - containedUnguardedInstanceofReferences.addAll(other.containedUnguardedInstanceofReferences); - other.containedUnguardedInstanceofReferences.clear(); + boolean shouldBeMoved() { + // Only move if all are true: + // a) allowMove is true + // b) it is declared somewhere (declModule != null) + // c) the all usages depend on the declModule by way of a different, preferred module + return allowMove && declModule != null && graph.dependsOn(getPreferredModule(), declModule); } } /** - * Creates a new stack of statements by merging two existing ones and maintaining the same - * last-statement-first order. - * - *

The input stacks will be cleared. + * get the information on a variable */ - private Deque mergeStatementsLatestFirst( - Deque stackA, Deque stackB) { - Deque newStack = new ArrayDeque<>(stackA.size() + stackB.size()); - TopLevelStatement a = stackA.peek(); - TopLevelStatement b = stackB.peek(); - while (true) { - if (a == null) { - newStack.addAll(stackB); - stackB.clear(); - break; - } else if (b == null) { - newStack.addAll(stackA); - stackA.clear(); - break; - } else if (a.getOriginalOrder() > b.getOriginalOrder()) { - newStack.add(stackA.pop()); - a = stackA.peek(); - } else { - newStack.add(stackB.pop()); - b = stackB.peek(); - } + private NamedInfo getNamedInfo(Var v) { + NamedInfo info = namedInfo.get(v); + if (info == null) { + info = new NamedInfo(); + namedInfo.put(v, info); } - return newStack; + return info; } - private static class InstanceofReference { - JSModule module; - Reference reference; - - InstanceofReference(JSModule module, Reference reference) { - this.module = module; - this.reference = reference; + private void collectReferences(Node root) { + CrossModuleReferenceCollector collector = new CrossModuleReferenceCollector( + compiler, + new Es6SyntacticScopeCreator(compiler)); + collector.process(root); + + for (TopLevelStatement statement : collector.getTopLevelStatements()) { + Var declaredVar = null; + if (statement.isDeclarationStatement()) { + declaredVar = statement.getDeclaredNameReference().getSymbol(); + NamedInfo declaredNameInfo = getNamedInfo(declaredVar); + if (statement.isMovableDeclaration()) { + declaredNameInfo.addMovableDeclaringStatement(statement); + } else { + // It's a declaration, but not movable, so treat its as a non-declaration reference. + declaredNameInfo.addReferringStatement(statement); + } + } + for (Reference ref : statement.getNonDeclarationReferences()) { + Var v = ref.getSymbol(); + // ignore recursive references + if (!Objects.equal(declaredVar, v)) { + NamedInfo info = getNamedInfo(v); + if (!parentModuleCanSeeSymbolsDeclaredInChildren) { + // Modules are loaded in such a way that Foo really must be defined before any + // expressions like `x instanceof Foo` are evaluated. + info.addReferringStatement(statement); + } else { + Node n = ref.getNode(); + if (isUnguardedInstanceofReference(n)) { + // Save a list of unguarded instanceof references. + // We'll add undefined typeof guards to them instead of allowing them to block code + // motion. + instanceofNodes.put(n.getParent(), new InstanceofInfo(getModule(ref), info)); + } else if (!(isUndefinedTypeofGuardReference(n) || isGuardedInstanceofReference(n))) { + // Ignore `'undefined' != typeof Ref && x instanceof Ref` + // Otherwise, it's a read + info.addReferringStatement(statement); + } + } + } + } } } /** - * Is the reference node the first {@code Ref} in an expression like {@code 'undefined' != typeof - * Ref && x instanceof Ref}? + * Is the reference node the first {@code Ref} in an expression like + * {@code 'undefined' != typeof Ref && x instanceof Ref}? * *

It's safe to ignore this kind of reference when moving the definition of {@code Ref}. */ @@ -593,8 +321,8 @@ private boolean isUndefinedTypeofGuardFor(Node expression, Node reference) { } /** - * Is the reference node the second {@code Ref} in an expression like {@code 'undefined' != typeof - * Ref && x instanceof Ref}? + * Is the reference node the second {@code Ref} in an expression like + * {@code 'undefined' != typeof Ref && x instanceof Ref}? * *

It's safe to ignore this kind of reference when moving the definition of {@code Ref}. */ @@ -632,4 +360,60 @@ private boolean isUnguardedInstanceofReference(Node reference) { private boolean isInstanceofFor(Node expression, Node reference) { return expression.isInstanceOf() && expression.getLastChild().isEquivalentTo(reference); } + + private JSModule getModule(Reference ref) { + return compiler.getInput(ref.getInputId()).getModule(); + } + + /** + * Transforms instanceof usages into an expression that short circuits to false if tested with a + * constructor that is undefined. This allows ignoring instanceof with respect to cross module + * code motion. + */ + private void addInstanceofGuards() { + Node tmp = IR.block(); + for (Map.Entry entry : instanceofNodes.entrySet()) { + Node n = entry.getKey(); + InstanceofInfo info = entry.getValue(); + // No need for a guard if: + // 1. the declaration wasn't moved + // 2. OR it was moved to the start of the module containing this instanceof reference + // 3. OR it was moved to a module the instanceof reference's module depends on + if (!info.namedInfo.wasMoved + || info.namedInfo.declModule.equals(info.module) + || graph.dependsOn(info.module, info.namedInfo.declModule)) { + continue; + } + // Wrap "foo instanceof Bar" in + // "('undefined' != typeof Bar && foo instanceof Bar)" + Node originalReference = n.getLastChild(); + checkState( + isUnguardedInstanceofReference(originalReference), + "instanceof Reference is already guarded: %s", + originalReference); + Node reference = originalReference.cloneNode(); + checkState(reference.isName()); + n.replaceWith(tmp); + Node and = IR.and( + new Node(Token.NE, + IR.string("undefined"), + new Node(Token.TYPEOF, reference) + ), + n + ); + and.useSourceInfoIfMissingFromForTree(n); + tmp.replaceWith(and); + compiler.reportChangeToEnclosingScope(and); + } + } + + private static class InstanceofInfo { + private final JSModule module; + private final NamedInfo namedInfo; + + InstanceofInfo(JSModule module, NamedInfo namedInfo) { + this.module = checkNotNull(module); + this.namedInfo = checkNotNull(namedInfo); + } + } } diff --git a/src/com/google/javascript/jscomp/CrossModuleReferenceCollector.java b/src/com/google/javascript/jscomp/CrossModuleReferenceCollector.java index e70b4990aff..30cd081432f 100644 --- a/src/com/google/javascript/jscomp/CrossModuleReferenceCollector.java +++ b/src/com/google/javascript/jscomp/CrossModuleReferenceCollector.java @@ -59,7 +59,6 @@ public final class CrossModuleReferenceCollector implements ScopedCallback, Comp */ private final AbstractCompiler compiler; - private int statementCounter = 0; private TopLevelStatementDraft topLevelStatementDraft = null; /** @@ -182,8 +181,7 @@ public boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent) } private TopLevelStatementDraft initializeDraftStatement(JSModule module, Node statementNode) { - TopLevelStatementDraft draft = - new TopLevelStatementDraft(statementCounter++, module, statementNode); + TopLevelStatementDraft draft = new TopLevelStatementDraft(module, statementNode); // Determine whether this statement declares a name or not. // If so, save its name node and value node, if any. if (statementNode.isVar()) { @@ -351,9 +349,6 @@ private boolean canMoveValue(Scope scope, Node valueNode) { /** Represents a top-level statement and the references to global names it contains. */ final class TopLevelStatement { - /** 0-based index indicating original order of this statement in the source. */ - private final int originalOrder; - private final JSModule module; private final Node statementNode; private final List nonDeclarationReferences; @@ -361,7 +356,6 @@ final class TopLevelStatement { private final Node declaredValueNode; TopLevelStatement(TopLevelStatementDraft draft) { - this.originalOrder = draft.originalOrder; this.module = draft.module; this.statementNode = draft.statementNode; this.nonDeclarationReferences = Collections.unmodifiableList(draft.nonDeclarationReferences); @@ -369,10 +363,6 @@ final class TopLevelStatement { this.declaredValueNode = draft.declaredValueNode; } - int getOriginalOrder() { - return originalOrder; - } - JSModule getModule() { return module; } @@ -407,9 +397,6 @@ boolean isMovableDeclaration() { /** Holds statement info temporarily while the statement is being traversed. */ private static final class TopLevelStatementDraft { - /** 0-based index indicating original order of this statement in the source. */ - private final int originalOrder; - final JSModule module; final Node statementNode; final List nonDeclarationReferences = new ArrayList<>(); @@ -417,8 +404,7 @@ private static final class TopLevelStatementDraft { Node declaredNameNode = null; Reference declaredNameReference = null; - TopLevelStatementDraft(int originalOrder, JSModule module, Node statementNode) { - this.originalOrder = originalOrder; + TopLevelStatementDraft(JSModule module, Node statementNode) { this.module = module; this.statementNode = statementNode; } diff --git a/src/com/google/javascript/jscomp/JSModuleGraph.java b/src/com/google/javascript/jscomp/JSModuleGraph.java index c249c7e0dca..fa98db0975a 100644 --- a/src/com/google/javascript/jscomp/JSModuleGraph.java +++ b/src/com/google/javascript/jscomp/JSModuleGraph.java @@ -262,19 +262,16 @@ public boolean dependsOn(JSModule src, JSModule m) { } /** - * Finds the module with the fewest transitive dependents on which all of the given modules depend - * and that is a subtree of the given parent module tree. - * - *

If no such subtree can be found, the parent module is returned. + * Finds the module with the fewest transitive dependents on which all of the given modules + * depend. * *

If multiple candidates have the same number of dependents, the module farthest down in the * total ordering of modules will be chosen. * - * @param parentTree module on which the result must depend * @param dependentModules indices of modules to consider * @return A module on which all of the argument modules depend */ - public JSModule getSmallestCoveringSubtree(JSModule parentTree, BitSet dependentModules) { + public JSModule getSmallestCoveringDependency(BitSet dependentModules) { checkState(!dependentModules.isEmpty()); // Candidate modules are those that all of the given dependent modules depend on, including @@ -296,21 +293,14 @@ public JSModule getSmallestCoveringSubtree(JSModule parentTree, BitSet dependent // Work backwards through the candidates starting with the dependent module with the smallest // index. For each candidate, we'll remove all of the modules it depends on from consideration, // since they must all have larger subtrees than the one we're considering. - int parentTreeIndex = parentTree.getIndex(); - // default to parent tree if we don't find anything better - int bestCandidateIndex = parentTreeIndex; - for (int candidateIndex = candidates.previousSetBit(minDependentModuleIndex); + int bestCandidateIndex = candidates.previousSetBit(minDependentModuleIndex); + for (int candidateIndex = bestCandidateIndex; candidateIndex >= 0; candidateIndex = candidates.previousSetBit(candidateIndex - 1)) { - - BitSet candidatePlusTransitiveDeps = selfPlusTransitiveDeps[candidateIndex]; - if (candidatePlusTransitiveDeps.get(parentTreeIndex)) { - // candidate is a subtree of parentTree - candidates.andNot(candidatePlusTransitiveDeps); - if (subtreeSize[candidateIndex] < subtreeSize[bestCandidateIndex]) { - bestCandidateIndex = candidateIndex; - } - } // eliminate candidates that are not a subtree of parentTree + candidates.andNot(selfPlusTransitiveDeps[candidateIndex]); + if (subtreeSize[candidateIndex] < subtreeSize[bestCandidateIndex]) { + bestCandidateIndex = candidateIndex; + } } return modules[bestCandidateIndex]; } diff --git a/test/com/google/javascript/jscomp/CrossModuleCodeMotionTest.java b/test/com/google/javascript/jscomp/CrossModuleCodeMotionTest.java index 93651cb9f33..8d47ebf7ba8 100644 --- a/test/com/google/javascript/jscomp/CrossModuleCodeMotionTest.java +++ b/test/com/google/javascript/jscomp/CrossModuleCodeMotionTest.java @@ -29,12 +29,6 @@ public CrossModuleCodeMotionTest() { super(EXTERNS); } - @Override - protected int getNumRepetitions() { - // A single run should be sufficient to move all definitions to their final destinations. - return 1; - } - @Override protected void setUp() throws Exception { super.setUp(); @@ -45,7 +39,9 @@ protected void setUp() throws Exception { @Override protected CompilerPass getProcessor(Compiler compiler) { return new CrossModuleCodeMotion( - compiler, compiler.getModuleGraph(), parentModuleCanSeeSymbolsDeclaredInChildren); + compiler, + compiler.getModuleGraph(), + parentModuleCanSeeSymbolsDeclaredInChildren); } public void testFunctionMovement1() { @@ -57,228 +53,216 @@ public void testFunctionMovement1() { // 5) h declared in m2 and never used. It stays put // 6) f4 declared in m1 and used in m2 as var. It moves to m2 - JSModule[] modules = - createModuleStar( - // m1 - LINE_JOINER.join( - "function f1(a) { alert(a); }", - "function f2(a) { alert(a); }", - "function f3(a) { alert(a); }", - "function f4() { alert(1); }", - "function g() { alert('ciao'); }"), - // m2 - "f1('hi'); f3('bye'); var a = f4; function h(a) { alert('h:' + a); }", - // m3 - "f2('hi'); f2('hi'); f3('bye');"); - - test( - modules, - new String[] { - // m1 - "function f3(a) { alert(a); } function g() { alert('ciao'); }", - // m2 - LINE_JOINER.join( - "function f1(a) { alert(a); }", - "function f4() { alert(1); }", - "f1('hi'); f3('bye'); var a = f4;", - "function h(a) { alert('h:' + a); }", - ""), - // m3 - "function f2(a) { alert(a); } f2('hi'); f2('hi'); f3('bye');", - }); + JSModule[] modules = createModuleStar( + // m1 + "function f1(a) { alert(a); }" + + "function f2(a) { alert(a); }" + + "function f3(a) { alert(a); }" + + "function f4() { alert(1); }" + + "function g() { alert('ciao'); }", + // m2 + "f1('hi'); f3('bye'); var a = f4;" + + "function h(a) { alert('h:' + a); }", + // m3 + "f2('hi'); f2('hi'); f3('bye');"); + + test(modules, new String[] { + // m1 + "function f3(a) { alert(a); }" + + "function g() { alert('ciao'); }", + // m2 + "function f4() { alert(1); }" + + "function f1(a) { alert(a); }" + + "f1('hi'); f3('bye'); var a = f4;" + + "function h(a) { alert('h:' + a); }", + // m3 + "function f2(a) { alert(a); }" + + "f2('hi'); f2('hi'); f3('bye');", + }); } public void testFunctionMovement2() { // having f declared as a local variable should block the migration to m2 - JSModule[] modules = - createModuleStar( - // m1 - "function f(a) { alert(a); } function g() {var f = 1; f++}", - // m2 - "f(1);"); - - test( - modules, - new String[] { - // m1 - "function g() {var f = 1; f++}", - // m2 - "function f(a) { alert(a); } f(1);", - }); + JSModule[] modules = createModuleStar( + // m1 + "function f(a) { alert(a); }" + + "function g() {var f = 1; f++}", + // m2 + "f(1);"); + + test(modules, new String[] { + // m1 + "function g() {var f = 1; f++}", + // m2 + "function f(a) { alert(a); }" + + "f(1);", + }); } public void testFunctionMovement3() { // having f declared as a arg should block the migration to m2 - JSModule[] modules = - createModuleStar( - // m1 - "function f(a) { alert(a); } function g(f) {f++}", - // m2 - "f(1);"); - - test( - modules, - new String[] { - // m1 - "function g(f) {f++}", - // m2 - "function f(a) { alert(a); } f(1);", - }); + JSModule[] modules = createModuleStar( + // m1 + "function f(a) { alert(a); }" + + "function g(f) {f++}", + // m2 + "f(1);"); + + test(modules, new String[] { + // m1 + "function g(f) {f++}", + // m2 + "function f(a) { alert(a); }" + + "f(1);", + }); } public void testFunctionMovement4() { // Try out moving a function which returns a closure - JSModule[] modules = - createModuleStar( - // m1 - "function f(){return function(a){}}", - // m2 - "var a = f();"); - - test( - modules, - new String[] { - // m1 - "", - // m2 - "function f(){return function(a){}} var a = f();", - }); + JSModule[] modules = createModuleStar( + // m1 + "function f(){return function(a){}}", + // m2 + "var a = f();" + ); + + test(modules, new String[] { + // m1 + "", + // m2 + "function f(){return function(a){}}" + + "var a = f();", + }); } public void testFunctionMovement5() { // Try moving a recursive function [using factorials for kicks] - JSModule[] modules = - createModuleStar( - // m1 - "function f(n){return (n<1)?1:f(n-1)}", - // m2 - "var a = f(4);"); - - test( - modules, - new String[] { - // m1 - "", - // m2 - "function f(n){return (n<1)?1:f(n-1)} var a = f(4);", - }); + JSModule[] modules = createModuleStar( + // m1 + "function f(n){return (n<1)?1:f(n-1)}", + // m2 + "var a = f(4);" + ); + + test(modules, new String[] { + // m1 + "", + // m2 + "function f(n){return (n<1)?1:f(n-1)}" + + "var a = f(4);", + }); } public void testFunctionMovement5b() { // Try moving a recursive function declared differently. - JSModule[] modules = - createModuleStar( - // m1 - "var f = function(n){return (n<1)?1:f(n-1)};", - // m2 - "var a = f(4);"); - - test( - modules, - new String[] { - // m1 - "", - // m2 - "var f = function(n){return (n<1)?1:f(n-1)}; var a = f(4);", - }); + JSModule[] modules = createModuleStar( + // m1 + "var f = function(n){return (n<1)?1:f(n-1)};", + // m2 + "var a = f(4);" + ); + + test(modules, new String[] { + // m1 + "", + // m2 + "var f = function(n){return (n<1)?1:f(n-1)};" + + "var a = f(4);", + }); } public void testFunctionMovement5c() { // Try moving a recursive function declared differently, in a nested block scope. - JSModule[] modules = - createModuleStar( - // m1 - "var f = function(n){if(true){if(true){return (n<1)?1:f(n-1)}}};", - // m2 - "var a = f(4);"); - - test( - modules, - new String[] { - // m1 - "", - // m2 - LINE_JOINER.join( - "var f = function(n){if(true){if(true){return (n<1)?1:f(n-1)}}};", "var a = f(4);") - }); + JSModule[] modules = createModuleStar( + // m1 + "var f = function(n){if(true){if(true){return (n<1)?1:f(n-1)}}};", + // m2 + "var a = f(4);" + ); + + test(modules, new String[] { + // m1 + "", + // m2 + LINE_JOINER.join( + "var f = function(n){if(true){if(true){return (n<1)?1:f(n-1)}}};", + "var a = f(4);") + }); } public void testFunctionMovement6() { // Try out moving to the common ancestor - JSModule[] modules = - createModuleChain( - // m1 - "function f(){return 1}", - // m2 - "var a = f();", - // m3 - "var b = f();"); - - test( - modules, - new String[] { - // m1 - "", - // m2 - "function f(){return 1} var a = f();", - // m3 - "var b = f();", - }); + JSModule[] modules = createModuleChain( + // m1 + "function f(){return 1}", + // m2 + "var a = f();", + // m3 + "var b = f();" + ); + + test(modules, new String[] { + // m1 + "", + // m2 + "function f(){return 1}" + + "var a = f();", + // m3 + "var b = f();", + }); } public void testFunctionMovement7() { // Try out moving to the common ancestor with deeper ancestry chain - JSModule[] modules = - createModules( - // m1 - "function f(){return 1}", - // m2 - "", - // m3 - "var a = f();", - // m4 - "var b = f();", - // m5 - "var c = f();"); + JSModule[] modules = createModules( + // m1 + "function f(){return 1}", + // m2 + "", + // m3 + "var a = f();", + // m4 + "var b = f();", + // m5 + "var c = f();" + ); + modules[1].addDependency(modules[0]); modules[2].addDependency(modules[1]); modules[3].addDependency(modules[1]); modules[4].addDependency(modules[1]); - test( - modules, - new String[] { - // m1 - "", - // m2 - "function f(){return 1}", - // m3 - "var a = f();", - // m4 - "var b = f();", - // m5 - "var c = f();", - }); + test(modules, new String[] { + // m1 + "", + // m2 + "function f(){return 1}", + // m3 + "var a = f();", + // m4 + "var b = f();", + // m5 + "var c = f();", + }); } public void testFunctionMovement8() { // Check what happens with named functions - JSModule[] modules = - createModuleChain( - // m1 - "var v = function f(){return 1}", - // m2 - "v();"); - - test( - modules, - new String[] { - // m1 - "", - // m2 - "var v = function f(){return 1}; v();", - }); + JSModule[] modules = createModuleChain( + // m1 + "var v = function f(){return 1}", + // m2 + "v();" + ); + + test(modules, new String[] { + // m1 + "", + // m2 + "var v = function f(){return 1};" + + "v();", + }); } public void testFunctionNonMovement1() { @@ -288,76 +272,78 @@ public void testFunctionNonMovement1() { // 3) if it's in an while statement, we can't move it [with some extra // block elements] setAcceptedLanguage(CompilerOptions.LanguageMode.ECMASCRIPT_2015); - testSame( - createModuleStar( - // m1 - LINE_JOINER.join( - "function f(){};f.prototype.bar=new f;", - "if(a)function f2(){}", - "{{while(a)function f3(){}}}"), - // m2 - "var a = new f();f2();f3();")); + testSame(createModuleStar( + // m1 + "function f(){};f.prototype.bar=new f;" + + "if(a)function f2(){}" + + "{{while(a)function f3(){}}}", + // m2 + "var a = new f();f2();f3();")); } public void testFunctionNonMovement2() { // A generic case where 2 modules depend on the first one. But it's the // common ancestor, so we can't move. - testSame( - createModuleStar( - // m1 - "function f(){return 1}", - // m2 - "var a = f();", - // m3 - "var b = f();")); + testSame(createModuleStar( + // m1 + "function f(){return 1}", + // m2 + "var a = f();", + // m3 + "var b = f();")); } public void testClassMovement1() { - test( - createModuleStar( - // m1 - "function f(){} f.prototype.bar=function (){};", - // m2 - "var a = new f();"), - new String[] {"", "function f(){} f.prototype.bar=function (){}; var a = new f();"}); + test(createModuleStar( + // m1 + "function f(){} f.prototype.bar=function (){};", + // m2 + "var a = new f();"), + new String[] { + "", + "function f(){} f.prototype.bar=function (){};" + + "var a = new f();" + }); } public void testClassMovement_instanceof() { parentModuleCanSeeSymbolsDeclaredInChildren = true; - test( - createModuleStar( - // m1 - "function f(){} f.prototype.bar=function (){}; 1 instanceof f;", - // m2 - "var a = new f();"), - new String[] { - "'undefined' != typeof f && 1 instanceof f;", - "function f(){} f.prototype.bar=function (){}; var a = new f();" - }); + test(createModuleStar( + // m1 + "function f(){} f.prototype.bar=function (){};" + + "1 instanceof f;", + // m2 + "var a = new f();"), + new String[] { + "'undefined' != typeof f && 1 instanceof f;", + "function f(){} f.prototype.bar=function (){};" + + "var a = new f();" + }); } public void testClassMovement_instanceofTurnedOff() { parentModuleCanSeeSymbolsDeclaredInChildren = false; - testSame( - createModuleStar( - // m1 - "function f(){} f.prototype.bar=function (){}; 1 instanceof f;", - // m2 - "var a = new f();")); + testSame(createModuleStar( + // m1 + "function f(){} f.prototype.bar=function (){};" + + "1 instanceof f;", + // m2 + "var a = new f();")); } public void testClassMovement_instanceof2() { parentModuleCanSeeSymbolsDeclaredInChildren = true; - test( - createModuleStar( - // m1 - "function f(){} f.prototype.bar=function (){}; (true && 1 instanceof f);", - // m2 - "var a = new f();"), - new String[] { - "(true && ('undefined' != typeof f && 1 instanceof f));", - "function f(){} f.prototype.bar=function (){}; var a = new f();" - }); + test(createModuleStar( + // m1 + "function f(){} f.prototype.bar=function (){};" + + "(true && 1 instanceof f);", + // m2 + "var a = new f();"), + new String[] { + "(true && ('undefined' != typeof f && 1 instanceof f));", + "function f(){} f.prototype.bar=function (){};" + + "var a = new f();" + }); } public void testClassMovement_alreadyGuardedInstanceof() { @@ -365,567 +351,549 @@ public void testClassMovement_alreadyGuardedInstanceof() { test( createModuleStar( // m1 - LINE_JOINER.join( - "function f(){} f.prototype.bar=function (){};", - "(true && ('undefined' != typeof f && 1 instanceof f));"), + "function f(){} f.prototype.bar=function (){};" + + "(true && ('undefined' != typeof f && 1 instanceof f));", // m2 "var a = new f();"), new String[] { "(true && ('undefined' != typeof f && 1 instanceof f));", - "function f(){} f.prototype.bar=function (){}; var a = new f();" + "function f(){} f.prototype.bar=function (){};" + "var a = new f();" }); } public void testClassMovement_instanceof3() { parentModuleCanSeeSymbolsDeclaredInChildren = true; - testSame( - createModuleStar( - // m1 - "function f(){} f.prototype.bar=function (){}; f instanceof 1", - // m2 - "var a = new f();")); + testSame(createModuleStar( + // m1 + "function f(){} f.prototype.bar=function (){};" + + "f instanceof 1", + // m2 + "var a = new f();")); } public void testClassMovement_instanceof_noRewriteRequired() { parentModuleCanSeeSymbolsDeclaredInChildren = true; - testSame( - createModuleStar( - // m1 - "function f(){} f.prototype.bar=function (){}; 1 instanceof f; new f;", - // m2 - "var a = new f();")); + testSame(createModuleStar( + // m1 + "function f(){} f.prototype.bar=function (){};" + + "1 instanceof f;" + + "new f;", + // m2 + "var a = new f();")); } public void testClassMovement_instanceof_noRewriteRequired2() { parentModuleCanSeeSymbolsDeclaredInChildren = true; - testSame( - createModuleChain( - // m1 - "function f(){} f.prototype.bar=function (){}; new f;", - // m2 - "1 instanceof f;", - // m3 - "var a = new f();")); + testSame(createModuleChain( + // m1 + "function f(){} f.prototype.bar=function (){};" + + "new f;", + // m2 + "1 instanceof f;", + // m3 + "var a = new f();")); } public void testClassMovement2() { - test( - createModuleChain( - // m1 - "function f(){} f.prototype.bar=3; f.prototype.baz=5;", - // m2 - "f.prototype.baq = 7;", - // m3 - "f.prototype.baz = 9;", - // m4 - "var a = new f();"), - new String[] { - // m1 - "", - // m2 - "", - // m3 - "", - // m4 - LINE_JOINER.join( - "function f(){}", - "f.prototype.bar=3;", - "f.prototype.baz=5;", - "f.prototype.baq = 7;", - "f.prototype.baz = 9;", - "var a = new f();") - }); + // NOTE: this is the result of two iterations + test(createModuleChain( + // m1 + "function f(){} f.prototype.bar=3; f.prototype.baz=5;", + // m2 + "f.prototype.baq = 7;", + // m3 + "f.prototype.baz = 9;", + // m4 + "var a = new f();"), + new String[] { + // m1 + "", + // m2 + "", + // m3 + "function f(){} f.prototype.bar=3; f.prototype.baz=5;" + + "f.prototype.baq = 7;" + + "f.prototype.baz = 9;", + // m4 + "var a = new f();" + }); } public void testClassMovement3() { - test( - createModuleChain( - // m1 - "var f = function() {}; f.prototype.bar=3; f.prototype.baz=5;", - // m2 - "f = 7;", - // m3 - "f = 9;", - // m4 - "f = 11;"), - new String[] { - // m1 - "", - // m2 - "", - // m3 - "", - // m4 - LINE_JOINER.join( - "var f = function() {};", - "f.prototype.bar=3;", - "f.prototype.baz=5;", - "f = 7;", - "f = 9;", - "f = 11;") - }); + // NOTE: this is the result of two iterations + test(createModuleChain( + // m1 + "var f = function() {}; f.prototype.bar=3; f.prototype.baz=5;", + // m2 + "f = 7;", + // m3 + "f = 9;", + // m4 + "f = 11;"), + new String[] { + // m1 + "", + // m2 + "", + // m3 + "var f = function() {}; f.prototype.bar=3; f.prototype.baz=5;" + + "f = 7;" + + "f = 9;", + // m4 + "f = 11;" + }); } public void testClassMovement4() { - testSame( - createModuleStar( - // m1 - "function f(){} f.prototype.bar=3; f.prototype.baz=5;", - // m2 - "f.prototype.baq = 7;", - // m3 - "var a = new f();")); + testSame(createModuleStar( + // m1 + "function f(){} f.prototype.bar=3; f.prototype.baz=5;", + // m2 + "f.prototype.baq = 7;", + // m3 + "var a = new f();")); } public void testClassMovement5() { - JSModule[] modules = - createModules( - // m1 - "function f(){} f.prototype.bar=3; f.prototype.baz=5;", - // m2 - "", - // m3 - "f.prototype.baq = 7;", - // m4 - "var a = new f();"); + JSModule[] modules = createModules( + // m1 + "function f(){} f.prototype.bar=3; f.prototype.baz=5;", + // m2 + "", + // m3 + "f.prototype.baq = 7;", + // m4 + "var a = new f();"); modules[1].addDependency(modules[0]); modules[2].addDependency(modules[1]); modules[3].addDependency(modules[1]); - test( - modules, - new String[] { - // m1 - "", - // m2 - "function f(){} f.prototype.bar=3; f.prototype.baz=5;", - // m3 - "f.prototype.baq = 7;", - // m4 + - "var a = new f();" - }); + test(modules, + new String[] { + // m1 + "", + // m2 + "function f(){} f.prototype.bar=3; f.prototype.baz=5;", + // m3 + "f.prototype.baq = 7;", + // m4 + + "var a = new f();" + }); } public void testClassMovement6() { - test( - createModuleChain( - // m1 - "function Foo(){} function Bar(){} goog.inherits(Bar, Foo); new Foo();", - // m2 - "new Bar();"), - new String[] { - // m1 - "function Foo(){} new Foo();", - // m2 - "function Bar(){} goog.inherits(Bar, Foo); new Bar();" - }); + test(createModuleChain( + // m1 + "function Foo(){} function Bar(){} goog.inherits(Bar, Foo);" + + "new Foo();", + // m2 + "new Bar();"), + new String[] { + // m1 + "function Foo(){} new Foo();", + // m2 + "function Bar(){} goog.inherits(Bar, Foo); new Bar();" + }); } public void testClassMovement7() { - testSame( - createModuleChain( - // m1 - "function Foo(){} function Bar(){} goog.inherits(Bar, Foo); new Bar();", - // m2 - "new Foo();")); + testSame(createModuleChain( + // m1 + "function Foo(){} function Bar(){} goog.inherits(Bar, Foo);" + + "new Bar();", + // m2 + "new Foo();")); } public void testStubMethodMovement1() { - test( - createModuleChain( - // m1 - "function Foo(){} Foo.prototype.bar = JSCompiler_stubMethod(x);", - // m2 - "new Foo();"), + test(createModuleChain( + // m1 + "function Foo(){} " + + "Foo.prototype.bar = JSCompiler_stubMethod(x);", + // m2 + "new Foo();"), new String[] { // m1 - "", "function Foo(){} Foo.prototype.bar = JSCompiler_stubMethod(x); new Foo();" + "", + "function Foo(){} " + + "Foo.prototype.bar = JSCompiler_stubMethod(x);" + + "new Foo();" }); } public void testStubMethodMovement2() { - test( - createModuleChain( - // m1 - "function Foo(){} Foo.prototype.bar = JSCompiler_unstubMethod(x);", - // m2 - "new Foo();"), + test(createModuleChain( + // m1 + "function Foo(){} " + + "Foo.prototype.bar = JSCompiler_unstubMethod(x);", + // m2 + "new Foo();"), new String[] { // m1 - "", "function Foo(){} Foo.prototype.bar = JSCompiler_unstubMethod(x); new Foo();" + "", + "function Foo(){} " + + "Foo.prototype.bar = JSCompiler_unstubMethod(x);" + + "new Foo();" }); } public void testNoMoveSideEffectProperty() { - testSame( - createModuleChain( - // m1 - "function Foo(){} Foo.prototype.bar = createSomething();", - // m2 - "new Foo();")); + testSame(createModuleChain( + // m1 + "function Foo(){} " + + "Foo.prototype.bar = createSomething();", + // m2 + "new Foo();")); } public void testAssignMovement() { - test( - createModuleChain( - // m1 - "var f = 3; f = 5;", - // m2 - "var h = f;"), + test(createModuleChain( + // m1 + "var f = 3;" + + "f = 5;", + // m2 + "var h = f;"), new String[] { // m1 "", // m2 - "var f = 3; f = 5; var h = f;" + "var f = 3;" + + "f = 5;" + + "var h = f;" }); // don't move nested assigns - testSame( - createModuleChain( - // m1 - "var f = 3; var g = f = 5;", - // m2 - "var h = f;")); + testSame(createModuleChain( + // m1 + "var f = 3;" + + "var g = f = 5;", + // m2 + "var h = f;")); } public void testNoClassMovement2() { - test( - createModuleChain( - // m1 - "var f = {}; f.h = 5;", - // m2 - "var h = f;"), + test(createModuleChain( + // m1 + "var f = {};" + + "f.h = 5;", + // m2 + "var h = f;"), new String[] { // m1 "", // m2 - "var f = {}; f.h = 5; var h = f;" + "var f = {};" + + "f.h = 5;" + + "var h = f;" }); // don't move nested getprop assigns - testSame( - createModuleChain( - // m1 - "var f = {}; var g = f.h = 5;", - // m2 - "var h = f;")); + testSame(createModuleChain( + // m1 + "var f = {};" + + "var g = f.h = 5;", + // m2 + "var h = f;")); } public void testLiteralMovement1() { - test( - createModuleChain( - // m1 - "var f = {'hi': 'mom', 'bye': function() {}};", - // m2 - "var h = f;"), + test(createModuleChain( + // m1 + "var f = {'hi': 'mom', 'bye': function() {}};", + // m2 + "var h = f;"), new String[] { // m1 "", // m2 - "var f = {'hi': 'mom', 'bye': function() {}}; var h = f;" + "var f = {'hi': 'mom', 'bye': function() {}};" + + "var h = f;" }); } public void testLiteralMovement2() { - testSame( - createModuleChain( - // m1 - "var f = {'hi': 'mom', 'bye': goog.nullFunction};", - // m2 - "var h = f;")); + testSame(createModuleChain( + // m1 + "var f = {'hi': 'mom', 'bye': goog.nullFunction};", + // m2 + "var h = f;")); } public void testLiteralMovement3() { - test( - createModuleChain( - // m1 - "var f = ['hi', function() {}];", - // m2 - "var h = f;"), + test(createModuleChain( + // m1 + "var f = ['hi', function() {}];", + // m2 + "var h = f;"), new String[] { // m1 "", // m2 - "var f = ['hi', function() {}]; var h = f;" + "var f = ['hi', function() {}];" + + "var h = f;" }); } public void testLiteralMovement4() { - testSame( - createModuleChain( - // m1 - "var f = ['hi', goog.nullFunction];", - // m2 - "var h = f;")); + testSame(createModuleChain( + // m1 + "var f = ['hi', goog.nullFunction];", + // m2 + "var h = f;")); } public void testVarMovement1() { // test moving a variable - JSModule[] modules = - createModuleStar( - // m1 - "var a = 0;", - // m2 - "var x = a;"); - - test( - modules, - new String[] { - // m1 - "", - // m2 - "var a = 0; var x = a;", - }); + JSModule[] modules = createModuleStar( + // m1 + "var a = 0;", + // m2 + "var x = a;" + ); + + test(modules, new String[] { + // m1 + "", + // m2 + "var a = 0;" + + "var x = a;", + }); } public void testVarMovement2() { // Test moving 1 variable out of the block - JSModule[] modules = - createModuleStar( - // m1 - "var a = 0; var b = 1; var c = 2;", - // m2 - "var x = b;"); - - test( - modules, - new String[] { - // m1 - "var a = 0; var c = 2;", - // m2 - "var b = 1; var x = b;" - }); + JSModule[] modules = createModuleStar( + // m1 + "var a = 0; var b = 1; var c = 2;", + // m2 + "var x = b;" + ); + + test(modules, new String[] { + // m1 + "var a = 0; var c = 2;", + // m2 + "var b = 1;" + + "var x = b;" + }); } public void testVarMovement3() { // Test moving all variables out of the block - JSModule[] modules = - createModuleStar( - // m1 - "var a = 0; var b = 1;", - // m2 - "var x = a + b;"); + JSModule[] modules = createModuleStar( + // m1 + "var a = 0; var b = 1;", + // m2 + "var x = a + b;" + ); - test( - modules, - new String[] { - // m1 - "", - // m2 - "var a = 0; var b = 1; var x = a + b;" - }); + test(modules, new String[] { + // m1 + "", + // m2 + "var b = 1;" + + "var a = 0;" + + "var x = a + b;" + }); } + public void testVarMovement4() { // Test moving a function - JSModule[] modules = - createModuleStar( - // m1 - "var a = function(){alert(1)};", - // m2 - "var x = a;"); + JSModule[] modules = createModuleStar( + // m1 + "var a = function(){alert(1)};", + // m2 + "var x = a;" + ); - test( - modules, - new String[] { - // m1 - "", - // m2 - "var a = function(){alert(1)}; var x = a;" - }); + test(modules, new String[] { + // m1 + "", + // m2 + "var a = function(){alert(1)};" + + "var x = a;" + }); } + public void testVarMovement5() { // Don't move a function outside of scope - testSame( - createModuleStar( - // m1 - "var a = alert;", - // m2 - "var x = a;")); + testSame(createModuleStar( + // m1 + "var a = alert;", + // m2 + "var x = a;")); } public void testVarMovement6() { // Test moving a var with no assigned value - JSModule[] modules = - createModuleStar( - // m1 - "var a;", - // m2 - "var x = a;"); - - test( - modules, - new String[] { - // m1 - "", - // m2 - "var a; var x = a;" - }); + JSModule[] modules = createModuleStar( + // m1 + "var a;", + // m2 + "var x = a;" + ); + + test(modules, new String[] { + // m1 + "", + // m2 + "var a;" + + "var x = a;" + }); } public void testVarMovement7() { // Don't move a variable higher in the dependency tree - testSame( - createModuleStar( - // m1 - "function f() {g();} f();", - // m2 - "function g(){};")); + testSame(createModuleStar( + // m1 + "function f() {g();}", + // m2 + "function g(){};")); } public void testVarMovement8() { - JSModule[] modules = - createModuleBush( - // m1 - "var a = 0;", - // m2 -> m1 - "", - // m3 -> m2 - "var x = a;", - // m4 -> m2 - "var y = a;"); - - test( - modules, - new String[] { - // m1 - "", - // m2 - "var a = 0;", - // m3 - "var x = a;", - // m4 - "var y = a;" - }); + JSModule[] modules = createModuleBush( + // m1 + "var a = 0;", + // m2 -> m1 + "", + // m3 -> m2 + "var x = a;", + // m4 -> m2 + "var y = a;" + ); + + test(modules, new String[] { + // m1 + "", + // m2 + "var a = 0;", + // m3 + "var x = a;", + // m4 + "var y = a;" + }); } public void testVarMovement9() { - JSModule[] modules = - createModuleTree( - // m1 - "var a = 0; var b = 1; var c = 3;", - // m2 -> m1 - "", - // m3 -> m1 - "", - // m4 -> m2 - "a;", - // m5 -> m2 - "a;c;", - // m6 -> m3 - "b;", - // m7 -> m4 - "b;c;"); - - test( - modules, - new String[] { - // m1 - "var c = 3;", - // m2 - "var a = 0;", - // m3 - "var b = 1;", - // m4 - "a;", - // m5 - "a;c;", - // m6 - "b;", - // m7 - "b;c;" - }); + JSModule[] modules = createModuleTree( + // m1 + "var a = 0; var b = 1; var c = 3;", + // m2 -> m1 + "", + // m3 -> m1 + "", + // m4 -> m2 + "a;", + // m5 -> m2 + "a;c;", + // m6 -> m3 + "b;", + // m7 -> m4 + "b;c;" + ); + + test(modules, new String[] { + // m1 + "var c = 3;", + // m2 + "var a = 0;", + // m3 + "var b = 1;", + // m4 + "a;", + // m5 + "a;c;", + // m6 + "b;", + // m7 + "b;c;" + }); } public void testClinit1() { - JSModule[] modules = - createModuleChain( - // m1 - "function Foo$clinit() { Foo$clinit = function() {}; }", - // m2 - "Foo$clinit();"); + JSModule[] modules = createModuleChain( + // m1 + "function Foo$clinit() { Foo$clinit = function() {}; }", + // m2 + "Foo$clinit();"); test( modules, new String[] { // m1 "", // m2 - "function Foo$clinit() { Foo$clinit = function() {}; } Foo$clinit();" + "function Foo$clinit() { Foo$clinit = function() {}; }" + + "Foo$clinit();" }); } public void testClinit2() { - JSModule[] modules = - createModuleChain( - // m1 - "var Foo$clinit = function() { Foo$clinit = function() {}; };", - // m2 - "Foo$clinit();"); + JSModule[] modules = createModuleChain( + // m1 + "var Foo$clinit = function() { Foo$clinit = function() {}; };", + // m2 + "Foo$clinit();"); test( modules, new String[] { // m1 "", // m2 - "var Foo$clinit = function() { Foo$clinit = function() {}; }; Foo$clinit();" + "var Foo$clinit = function() { Foo$clinit = function() {}; };" + + "Foo$clinit();" }); } public void testClone1() { - test( - createModuleChain( - // m1 - "function f(){} f.prototype.clone = function() { return new f };", - // m2 - "var a = (new f).clone();"), - new String[] { - // m1 - "", - // m2 - LINE_JOINER.join( - "function f(){}", - "f.prototype.clone = function() { return new f() };", - "var a = (new f).clone();", - "") - }); + test(createModuleChain( + // m1 + "function f(){} f.prototype.clone = function() { return new f };", + // m2 + "var a = (new f).clone();"), + new String[] { + // m1 + "", + // m2 + "function f(){} f.prototype.clone = function() { return new f() };" + + "var a = (new f).clone();" + }); } public void testClone2() { - test( - createModuleChain( - // m1 - LINE_JOINER.join( - "function f(){}", - "f.prototype.cloneFun = function() {", - " return function() {new f}", - "};"), - // m2 - "var a = (new f).cloneFun();"), - new String[] { - // m1 - "", - LINE_JOINER.join( - "function f(){}", - "f.prototype.cloneFun = function() {", - " return function() {new f}", - "};", - "var a = (new f).cloneFun();") - }); + test(createModuleChain( + // m1 + "function f(){}" + + "f.prototype.cloneFun = function() {" + + " return function() {new f}" + + "};", + // m2 + "var a = (new f).cloneFun();"), + new String[] { + // m1 + "", + "function f(){}" + + "f.prototype.cloneFun = function() {" + + " return function() {new f}" + + "};" + + // m2 + "var a = (new f).cloneFun();" + }); } public void testBug4118005() { - testSame( - createModuleChain( - // m1 - "var m = 1;\n" - + "(function () {\n" - + " var x = 1;\n" - + " m = function() { return x };\n" - + "})();\n", - // m2 - "m();")); + testSame(createModuleChain( + // m1 + "var m = 1;\n" + + "(function () {\n" + + " var x = 1;\n" + + " m = function() { return x };\n" + + "})();\n", + // m2 + "m();")); } public void testEmptyModule() { @@ -949,215 +917,31 @@ public void testEmptyModule() { m3.add(SourceFile.fromCode("m3", "x()")); m3.addDependency(empty); - test(new JSModule[] {m1, empty, m2, m3}, new String[] {"", "function x() {}", "x()", "x()"}); - } - - public void testAbstractMethod() { - test( - createModuleStar( - // m1 - "var abstractMethod = function () {};" - + "function F(){} F.prototype.bar=abstractMethod;" - + "function G(){} G.prototype.bar=abstractMethod;", - // m2 - "var f = new F();", - // m3 - "var g = new G();"), - new String[] { - "var abstractMethod = function () {};", - "function F(){} F.prototype.bar=abstractMethod; var f = new F();", - "function G(){} G.prototype.bar=abstractMethod; var g = new G();" - }); - } - - public void testMovableUseBeforeDeclaration() { - test( - createModuleChain( - // m0 - "function f() { g(); } function g() {}", - // m1 - "f();"), - new String[] { - // m0 - "", - // m1 - "function g() {} function f() { g(); } f();" - }); - } - - public void testImmovableUseBeforeDeclaration() { - testSame( - createModuleChain( - // m0 - LINE_JOINER.join( - "g();", // must recognize this as a reference to the following declaration - "function g() {}"), - // m1 - "g();")); - - testSame( - createModuleChain( - // m0 - LINE_JOINER.join( - "function f() { g(); }", - "function g() {}", - "f();"), // f() cannot move, so neither can g() - // m1 - "g();")); - } - - public void testSplitDeclaration() { - test( - createModuleChain( - // m0 - LINE_JOINER.join( - "function a() { b(); }", - "function b() {}", - "function c() {}", - "a.prototype.x = function() { c(); };"), - // m1 - "a();"), - new String[] { - // m0 - "", - // m1 - LINE_JOINER.join( - "function b() {}", - "function c() {}", - "function a() { b(); }", - "a.prototype.x = function() { c(); };", - "a();"), - }); - } - - public void testOutOfOrderAfterSplitDeclaration() { - test( - createModuleChain( - // m0 - LINE_JOINER.join( - "function a() { c(); }", - "function b() {}", - "a.prototype.x = function() { c(); };", - "function c() {}", - ""), - // m1 - "a();"), - new String[] { - // m0 - "function b() {}", - // m1 - LINE_JOINER.join( - "function c() {}", - "function a() { c(); }", - "a.prototype.x = function() { c(); };", - "a();", - ""), - }); - } - - public void testOutOfOrderWithInterveningReferrer() { - test( - createModuleChain( - // m0 - "function a() { c(); } function b() { a(); } function c() {}", - // m1 - "b();"), - new String[] { - // m0 - "", - // m1 - "function c() {} function a() { c(); } function b() { a(); } b();", - }); - } - - public void testOutOfOrderWithDifferentReferrers() { - test( - createModuleChain( - // m0 - "function a() { b(); } function b() {}", - // m1 - "b();", - // m2 - "a();"), - new String[] { - // m0 - "", - // m1 - "function b() { } b();", - "function a() { b(); } a();", - }); - } - - public void testCircularWithDifferentReferrers() { - test( - createModuleChain( - // m0 - "function a() { b(); } function b() { a(); }", - // m1 - "b();", - // m2 - "a();"), + test(new JSModule[] {m1,empty,m2,m3}, new String[] { - // m0 "", - // m1 - "function a() { b(); } function b() { a(); } b();", - "a();", - }); + "function x() {}", + "x()", + "x()" + }); } - public void testSmallestCoveringDependencyDoesNotDependOnDeclarationModule() { - // m0 - // / \ - // m1 m2 // declaration in m1 - // | /| // smallest common dep is m2 - // m3_ | | // best place for declaration is m3 - // / | X | - // / |/ \ / - // m4 m5 m6 // references in m5 and m6 - JSModule[] m = - createModules( - // m0 - "", - // m1 - "function f() {}", - // m2 - "", - // m3 - "", - // m4 - "", - // m5 - "f();", - // m6 - "f();"); - - m[1].addDependency(m[0]); - m[2].addDependency(m[0]); - m[3].addDependency(m[1]); - m[4].addDependency(m[3]); - m[5].addDependency(m[2]); - m[5].addDependency(m[3]); - m[6].addDependency(m[2]); - m[6].addDependency(m[3]); - - test( - m, - new String[] { - // m0 - "", - // m1 - "", - // m2 - "", - // m3 - "function f() {}", - // m4 - "", - // m5 - "f();", - // m6 - "f();", - }); + public void testAbstractMethod() { + test(createModuleStar( + // m1 + "var abstractMethod = function () {};" + + "function F(){} F.prototype.bar=abstractMethod;" + + "function G(){} G.prototype.bar=abstractMethod;", + // m2 + "var f = new F();", + // m3 + "var g = new G();"), + new String[] { + "var abstractMethod = function () {};", + "function F(){} F.prototype.bar=abstractMethod;" + + "var f = new F();", + "function G(){} G.prototype.bar=abstractMethod;" + + "var g = new G();" + }); } } diff --git a/test/com/google/javascript/jscomp/CrossModuleReferenceCollectorTest.java b/test/com/google/javascript/jscomp/CrossModuleReferenceCollectorTest.java index 2c61abfa3e3..95a5c335983 100644 --- a/test/com/google/javascript/jscomp/CrossModuleReferenceCollectorTest.java +++ b/test/com/google/javascript/jscomp/CrossModuleReferenceCollectorTest.java @@ -172,22 +172,16 @@ public void testTopLevelStatements() { assertThat(topLevelStatements).hasSize(4); // var x = 1; TopLevelStatement xEquals1 = topLevelStatements.get(0); - assertThat(xEquals1.getOriginalOrder()).isEqualTo(0); assertThat(xEquals1.getDeclaredNameReference()).isEqualTo(xReferences.get(0)); assertThat(xEquals1.getNonDeclarationReferences()).isEmpty(); // const y = x; - TopLevelStatement yEqualsX = topLevelStatements.get(1); - assertThat(yEqualsX.getOriginalOrder()).isEqualTo(1); - assertThat(yEqualsX.getNonDeclarationReferences()) + assertThat(topLevelStatements.get(1).getNonDeclarationReferences()) .containsExactly(yReferences.get(0), xReferences.get(1)); // let z = x - y; - TopLevelStatement zEqualsXMinusY = topLevelStatements.get(2); - assertThat(zEqualsXMinusY.getOriginalOrder()).isEqualTo(2); - assertThat(zEqualsXMinusY.getNonDeclarationReferences()) + assertThat(topLevelStatements.get(2).getNonDeclarationReferences()) .containsExactly(zReferences.get(0), xReferences.get(2), yReferences.get(1)); // function f(x, y) { return x + y + z; } TopLevelStatement functionDeclaration = topLevelStatements.get(3); - assertThat(functionDeclaration.getOriginalOrder()).isEqualTo(3); assertThat(functionDeclaration.getDeclaredNameReference()).isEqualTo(fReferences.get(0)); assertThat(functionDeclaration.getNonDeclarationReferences()) .containsExactly(zReferences.get(1)); diff --git a/test/com/google/javascript/jscomp/JSModuleGraphTest.java b/test/com/google/javascript/jscomp/JSModuleGraphTest.java index 82c36ed07a4..3dbf7b9f710 100644 --- a/test/com/google/javascript/jscomp/JSModuleGraphTest.java +++ b/test/com/google/javascript/jscomp/JSModuleGraphTest.java @@ -19,6 +19,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.gson.JsonArray; @@ -49,13 +50,13 @@ public final class JSModuleGraphTest extends TestCase { @Override public void setUp() throws Exception { super.setUp(); - 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 + 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(); @@ -94,9 +95,7 @@ public void testSmallerTreeBeatsDeeperTree() { h.addDependency(d); JSModuleGraph graph = new JSModuleGraph(new JSModule[] {a, b, c, d, e, f, g, h}); // d is deeper, but it also has an extra dependent node, so b is the better choice. - assertSmallestCoveringSubtree(b, graph, a, e, f, g); - // However, if the parent tree we're looking at is c, then b isn't an option - assertSmallestCoveringSubtree(d, graph, c, e, f, g); + assertSmallestCoveringDependency(b, graph, e, f, g); } public void testModuleDepth() { @@ -156,28 +155,28 @@ public void testDeepestCommonDepInclusive() { assertDeepestCommonDepInclusive(F, F, F); } - 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); + public void testSmallestCoveringDependency() { + assertSmallestCoveringDependency(A, A, A); + assertSmallestCoveringDependency(A, A, B); + assertSmallestCoveringDependency(A, A, C); + assertSmallestCoveringDependency(A, A, D); + assertSmallestCoveringDependency(A, A, E); + assertSmallestCoveringDependency(A, A, F); + assertSmallestCoveringDependency(B, B, B); + assertSmallestCoveringDependency(A, B, C); + assertSmallestCoveringDependency(B, B, D); + assertSmallestCoveringDependency(B, B, E); + assertSmallestCoveringDependency(B, B, F); + assertSmallestCoveringDependency(C, C, C); + assertSmallestCoveringDependency(A, C, D); + assertSmallestCoveringDependency(C, C, E); + assertSmallestCoveringDependency(C, C, F); + assertSmallestCoveringDependency(D, D, D); + assertSmallestCoveringDependency(B, D, E); + assertSmallestCoveringDependency(B, D, F); + assertSmallestCoveringDependency(E, E, E); + assertSmallestCoveringDependency(E, E, F); + assertSmallestCoveringDependency(F, F, F); } public void testGetTransitiveDepsDeepestFirst() { @@ -202,7 +201,9 @@ public void testManageDependencies1() throws Exception { assertInputs(C); // no inputs assertInputs(E, "c1", "e1", "e2"); - assertEquals(ImmutableList.of("a1", "a3", "a2", "b2", "c1", "e1", "e2"), sourceNames(results)); + assertEquals( + ImmutableList.of("a1", "a3", "a2", "b2", "c1", "e1", "e2"), + sourceNames(results)); } public void testManageDependencies2() throws Exception { @@ -210,7 +211,8 @@ public void testManageDependencies2() throws Exception { DependencyOptions depOptions = new DependencyOptions(); depOptions.setDependencySorting(true); depOptions.setDependencyPruning(true); - depOptions.setEntryPoints(ImmutableList.of(ModuleIdentifier.forClosure("c2"))); + depOptions.setEntryPoints( + ImmutableList.of(ModuleIdentifier.forClosure("c2"))); List results = graph.manageDependencies(depOptions, inputs); assertInputs(A, "a1", "a3"); @@ -219,7 +221,8 @@ public void testManageDependencies2() throws Exception { assertInputs(E, "e1", "e2"); assertEquals( - ImmutableList.of("a1", "a3", "a2", "b2", "c1", "c2", "e1", "e2"), sourceNames(results)); + ImmutableList.of("a1", "a3", "a2", "b2", "c1", "c2", "e1", "e2"), + sourceNames(results)); } public void testManageDependencies3Impl() throws Exception { @@ -228,7 +231,8 @@ public void testManageDependencies3Impl() throws Exception { depOptions.setDependencySorting(true); depOptions.setDependencyPruning(true); depOptions.setMoocherDropping(true); - depOptions.setEntryPoints(ImmutableList.of(ModuleIdentifier.forClosure("c2"))); + depOptions.setEntryPoints( + ImmutableList.of(ModuleIdentifier.forClosure("c2"))); List results = graph.manageDependencies(depOptions, inputs); // Everything gets pushed up into module c, because that's @@ -262,13 +266,15 @@ public void testManageDependencies4() throws Exception { assertInputs(E, "e1", "e2"); assertEquals( - ImmutableList.of("a1", "a2", "a3", "b1", "b2", "c1", "c2", "e1", "e2"), + ImmutableList.of( + "a1", "a2", "a3", "b1", "b2", "c1", "c2", "e1", "e2"), sourceNames(results)); } - // NOTE: The newline between the @provideGoog comment and the var statement is required. - private static final String BASEJS = - "/** @provideGoog */\nvar COMPILED = false; var goog = goog || {}"; + private static final String BASEJS = Joiner.on("\n").join( + "/** @provideGoog */", + "var COMPILED = false;", + "var goog = goog || {}"); public void testManageDependencies5Impl() throws Exception { A.add(code("a2", provides("a2"), requires("a1"))); @@ -284,7 +290,8 @@ public void testManageDependencies5Impl() throws Exception { List inputs = new ArrayList<>(); inputs.addAll(A.getInputs()); - List results = graph.manageDependencies(depOptions, inputs); + List results = graph.manageDependencies( + depOptions, inputs); assertInputs(A, "base.js", "a1", "a2"); @@ -296,7 +303,8 @@ public void testNoFiles() throws Exception { depOptions.setDependencySorting(true); List inputs = new ArrayList<>(); - List results = graph.manageDependencies(depOptions, inputs); + List results = graph.manageDependencies( + depOptions, inputs); assertThat(results).isEmpty(); } @@ -313,7 +321,8 @@ public void testToJson() { JsonObject m = modules.get(3).getAsJsonObject(); assertEquals("D", m.get("name").getAsString()); assertEquals("[\"B\"]", m.get("dependencies").getAsJsonArray().toString()); - assertEquals(2, m.get("transitive-dependencies").getAsJsonArray().size()); + assertEquals(2, + m.get("transitive-dependencies").getAsJsonArray().size()); assertEquals("[]", m.get("inputs").getAsJsonArray().toString()); } @@ -344,8 +353,10 @@ private List setUpManageDependenciesTest() { return inputs; } - private void assertInputs(JSModule module, String... sourceNames) { - assertEquals(ImmutableList.copyOf(sourceNames), sourceNames(module.getInputs())); + private void assertInputs(JSModule module, String ... sourceNames) { + assertEquals( + ImmutableList.copyOf(sourceNames), + sourceNames(module.getInputs())); } private List sourceNames(List inputs) { @@ -356,12 +367,14 @@ private List sourceNames(List inputs) { return inputNames; } - private SourceFile code(String sourceName, List provides, List requires) { + private SourceFile code( + String sourceName, List provides, List requires) { return code(sourceName, "", provides, requires); } private SourceFile code( - String sourceName, String source, List provides, List requires) { + String sourceName, String source, + List provides, List requires) { String text = ""; for (String p : provides) { text += "goog.provide('" + p + "');\n"; @@ -372,44 +385,44 @@ private SourceFile code( return SourceFile.fromCode(sourceName, text + source); } - private List provides(String... strings) { + private List provides(String ... strings) { return ImmutableList.copyOf(strings); } - private List requires(String... strings) { + private List requires(String ... strings) { return ImmutableList.copyOf(strings); } - private void assertSmallestCoveringSubtree( - JSModule expected, JSModule parentTree, JSModule... modules) { - assertSmallestCoveringSubtree(expected, graph, parentTree, modules); + private void assertSmallestCoveringDependency(JSModule expected, JSModule... modules) { + assertSmallestCoveringDependency(expected, graph, modules); } - private void assertSmallestCoveringSubtree( - JSModule expected, JSModuleGraph graph, JSModule parentTree, JSModule... modules) { + private void assertSmallestCoveringDependency( + JSModule expected, JSModuleGraph graph, JSModule... modules) { BitSet modulesBitSet = new BitSet(); for (JSModule m : modules) { modulesBitSet.set(m.getIndex()); } - assertSmallestCoveringSubtree(expected, graph, parentTree, modulesBitSet); + assertSmallestCoveringDependency(expected, graph, modulesBitSet); } - private void assertSmallestCoveringSubtree( - JSModule expected, JSModuleGraph graph, JSModule parentTree, BitSet modules) { - JSModule actual = graph.getSmallestCoveringSubtree(parentTree, modules); + private void assertSmallestCoveringDependency( + JSModule expected, JSModuleGraph graph, BitSet modules) { + JSModule actual = graph.getSmallestCoveringDependency(modules); assertWithMessage( - "Smallest covering subtree of %s in %s should be %s but was %s", - parentTree, modules, expected, actual) + "Smallest covering dep of %s should be %s but was %s", modules, expected, actual) .that(actual) .isEqualTo(expected); } - private void assertDeepestCommonDepInclusive(JSModule expected, JSModule m1, JSModule m2) { + private void assertDeepestCommonDepInclusive( + JSModule expected, JSModule m1, JSModule m2) { assertDeepestCommonDepOneWay(expected, m1, m2, true); assertDeepestCommonDepOneWay(expected, m2, m1, true); } - private void assertDeepestCommonDep(JSModule expected, JSModule m1, JSModule m2) { + private void assertDeepestCommonDep( + JSModule expected, JSModule m1, JSModule m2) { assertDeepestCommonDepOneWay(expected, m1, m2, false); assertDeepestCommonDepOneWay(expected, m2, m1, false); } @@ -421,18 +434,17 @@ private void assertDeepestCommonDepOneWay( ? graph.getDeepestCommonDependencyInclusive(m1, m2) : graph.getDeepestCommonDependency(m1, m2); if (actual != expected) { - fail( - String.format( - "Deepest common dep of %s and %s should be %s but was %s", - m1.getName(), - m2.getName(), - expected == null ? "null" : expected.getName(), - actual == null ? "null" : actual.getName())); + fail(String.format( + "Deepest common dep of %s and %s should be %s but was %s", + m1.getName(), m2.getName(), + expected == null ? "null" : expected.getName(), + actual == null ? "null" : actual.getName())); } } private void assertTransitiveDepsDeepestFirst(JSModule m, JSModule... deps) { Iterable actual = graph.getTransitiveDepsDeepestFirst(m); - assertEquals(Arrays.toString(deps), Arrays.toString(Iterables.toArray(actual, JSModule.class))); + assertEquals(Arrays.toString(deps), + Arrays.toString(Iterables.toArray(actual, JSModule.class))); } }