diff --git a/src/com/google/javascript/jscomp/RemoveUnusedVars.java b/src/com/google/javascript/jscomp/RemoveUnusedVars.java index a7c241fbcf6..06d45c9d659 100644 --- a/src/com/google/javascript/jscomp/RemoveUnusedVars.java +++ b/src/com/google/javascript/jscomp/RemoveUnusedVars.java @@ -16,19 +16,26 @@ package com.google.javascript.jscomp; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; -import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.HashMultimap; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Multimap; import com.google.javascript.jscomp.CodingConvention.SubclassRelationship; import com.google.javascript.rhino.IR; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Token; +import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Deque; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; +import javax.annotation.Nullable; /** * Garbage collection for variable and function definitions. Basically performs @@ -89,46 +96,36 @@ class RemoveUnusedVars implements CompilerPass { private final boolean preserveFunctionExpressionNames; /** - * Keep track of variables that we've referenced. + * Used to hold continuations that need to be invoked. + * + * When we find a subtree of the AST that may not need to be traversed, we create a Continuation + * for it. If we later discover that we do need to traverse it, we add it to this worklist + * rather than traversing it immediately. If we invoked the traversal immediately, we could + * end up modifying a data structure in the traversal as we're iterating over it. */ - private final Set referenced = new HashSet<>(); + private final Deque worklist = new ArrayDeque<>(); - /** - * Keep track of variables that might be unreferenced. - */ - private List maybeUnreferenced = new ArrayList<>(); + private final Map varInfoMap = new HashMap<>(); - /** - * Keep track of scopes that we've traversed. - */ - private final List allFunctionParamScopes = new ArrayList<>(); + private final Set referencedPropertyNames = new HashSet<>(); /** - * Keep track of assigns to variables that we haven't referenced. - */ - private final Multimap assignsByVar = - ArrayListMultimap.create(); - - /** - * The assigns, indexed by the NAME node that they assign to. - */ - private final Set assignsByNode = new HashSet<>(); - - /** - * Subclass name -> class-defining call EXPR node. (like inherits) + * Map from property name to variables on which the property is defined. + * + * TODO(bradfordcsmith): Rework to avoid the need for this map. */ - private final Multimap classDefiningCalls = - ArrayListMultimap.create(); + private final Multimap varInfoForPropertyNameMap = HashMultimap.create(); /** - * Keep track of continuations that are finished iff the variable they're - * indexed by is referenced. + * Keep track of scopes that we've traversed. */ - private final Multimap continuations = - ArrayListMultimap.create(); + private final List allFunctionParamScopes = new ArrayList<>(); private final ScopeCreator scopeCreator; + // TODO(bradfordcsmith): Make this a constructor option that can be enabled + private final boolean removeUnusedProperties = false; + RemoveUnusedVars( AbstractCompiler compiler, boolean removeGlobals, @@ -154,20 +151,30 @@ public void process(Node externs, Node root) { * Traverses a node recursively. Call this once per pass. */ private void traverseAndRemoveUnusedReferences(Node root) { + // TODO(bradfordcsmith): Include externs in the scope. + // Since we don't do this now, scope.getVar(someExtern) returns null. Scope scope = scopeCreator.createScope(root, null); - traverseNode(root, null, scope); - - if (removeGlobals) { - collectMaybeUnreferencedVars(scope); + worklist.add(new Continuation(root, scope)); + while (!worklist.isEmpty()) { + Continuation continuation = worklist.remove(); + continuation.apply(); } - interpretAssigns(); removeUnreferencedVars(); + if (removeUnusedProperties) { + removeUnreferencedProperties(); + } for (Scope fparamScope : allFunctionParamScopes) { removeUnreferencedFunctionArgs(fparamScope); } } + private void removeUnreferencedProperties() { + for (VarInfo varInfo : varInfoForPropertyNameMap.values()) { + varInfo.removeUnreferencedProperties(); + } + } + /** * Traverses everything in the current scope and marks variables that * are referenced. @@ -177,255 +184,555 @@ private void traverseAndRemoveUnusedReferences(Node root) { * traversing those subtrees, we create a continuation for them, * and traverse them lazily. */ - private void traverseNode(Node n, Node parent, Scope scope) { + private void traverseNode(Node n, Scope scope) { + Node parent = n.getParent(); Token type = n.getToken(); Var var = null; switch (type) { - case FUNCTION: - // If this function is a removable var, then create a continuation - // for it instead of traversing immediately. - if (NodeUtil.isFunctionDeclaration(n)) { - var = scope.getVar(n.getFirstChild().getString()); - } - - if (var != null && isRemovableVar(var)) { - continuations.put(var, new Continuation(n, scope)); - } else { - traverseFunction(n, scope); - } - return; + case CATCH: + traverseCatch(n, scope); + break; - case ASSIGN: - Assign maybeAssign = Assign.maybeCreateAssign(n); - if (maybeAssign != null) { - // Put this in the assign map. It might count as a reference, - // but we won't know that until we have an index of all assigns. - var = scope.getVar(maybeAssign.nameNode.getString()); - if (var != null) { - assignsByVar.put(var, maybeAssign); - assignsByNode.add(maybeAssign.nameNode); - - if (isRemovableVar(var) - && !maybeAssign.mayHaveSecondarySideEffects) { - // If the var is unreferenced and performing this assign has - // no secondary side effects, then we can create a continuation - // for it instead of traversing immediately. - continuations.put(var, new Continuation(n, scope)); - return; + case FUNCTION: + { + VarInfo varInfo = null; + // If this function is a removable var, then create a continuation + // for it instead of traversing immediately. + if (NodeUtil.isFunctionDeclaration(n)) { + varInfo = getVarInfo(scope.getVar(n.getFirstChild().getString())); + FunctionDeclaration functionDeclaration = + new RemovableBuilder() + .addContinuation(new Continuation(n, scope)) + .buildFunctionDeclaration(n); + varInfo.addRemovable(functionDeclaration); + if (parent.isExport()) { + varInfo.markAsReferenced(); } + } else { + traverseFunction(n, scope); } } break; - case CALL: - Var modifiedVar = null; - - // Look for calls to inheritance-defining calls (such as goog.inherits). - SubclassRelationship subclassRelationship = - codingConvention.getClassesDefinedByCall(n); - if (subclassRelationship != null) { - modifiedVar = scope.getVar(subclassRelationship.subclassName); - } else { - // Look for calls to addSingletonGetter calls. - String className = codingConvention.getSingletonGetterClassName(n); - if (className != null) { - modifiedVar = scope.getVar(className); - } - } + case ASSIGN: + traverseAssign(n, scope); + break; - // Don't try to track the inheritance calls for non-globals. It would - // be more correct to only not track when the subclass does not - // reference a constructor, but checking that it is a global is - // easier and mostly the same. - if (modifiedVar != null && modifiedVar.isGlobal() - && !referenced.contains(modifiedVar)) { - // Save a reference to the EXPR node. - classDefiningCalls.put(modifiedVar, parent); - continuations.put(modifiedVar, new Continuation(n, scope)); - return; - } + case CALL: + traverseCall(n, scope); break; - // This case if for if there are let and const variables in block scopes. - // Otherwise other variables will be hoisted up into the global scope and already be handled. case BLOCK: - // check if we are already traversing that block node - if (NodeUtil.createsBlockScope(n)) { - Scope blockScope = scopeCreator.createScope(n, scope); - collectMaybeUnreferencedVars(blockScope); - scope = blockScope; - } + // This case if for if there are let and const variables in block scopes. + // Otherwise other variables will be hoisted up into the global scope and already be + // handled. + traverseChildren( + n, NodeUtil.createsBlockScope(n) ? scopeCreator.createScope(n, scope) : scope); break; - case CLASS: - // If this class is a removable var, then create a continuation - if (NodeUtil.isClassDeclaration(n)) { - var = scope.getVar(n.getFirstChild().getString()); - } + case MODULE_BODY: + traverseChildren(n, scopeCreator.createScope(n, scope)); + break; - if (var != null && isRemovableVar(var)) { - continuations.put(var, new Continuation(n, scope)); - } - return; + case CLASS: + traverseClass(n, scope); + break; - case DEFAULT_VALUE: { - Node target = n.getFirstChild(); - if (target.isName()) { - Node value = n.getLastChild(); - var = scope.getVar(target.getString()); - if (!NodeUtil.mayHaveSideEffects(value)) { - continuations.put(var, new Continuation(n, scope)); - assignsByVar.put(var, new DestructuringAssign(n, target)); - return; - } else { - // TODO(johnlenz): we don't really need to retain all uses of the variable, just enough - // to host the default value assignment. - markReferencedVar(var); - } - assignsByNode.add(target); - } - } - break; + case DEFAULT_VALUE: + traverseDefaultValue(n, scope); + break; - case REST: { - Node target = n.getFirstChild(); - if (target.isName()) { - assignsByNode.add(target); - var = scope.getVar(target.getString()); - assignsByVar.put(var, new DestructuringAssign(n, target)); - } - } - break; + case REST: + traverseRest(n, scope); + break; case ARRAY_PATTERN: - // Iterate in reverse order so we remove the last first, if possible - for (Node c = n.getLastChild(); c != null; c = c.getPrevious()) { - if (c.isName()) { - assignsByNode.add(c); - var = scope.getVar(c.getString()); - assignsByVar.put(var, new DestructuringAssign(c, c)); - } - } + traverseArrayPattern(n, scope); break; - case COMPUTED_PROP: - if (n.getParent().isObjectPattern()) { - // In a destructuring assignment, the target and the value name - // are backward from a normal assignment (the rhs is the receiver). - Node target = n.getLastChild(); - // If the computed properties calculation has side-effects, we have to leave it - Node value = n.getFirstChild(); - if (!NodeUtil.mayHaveSideEffects(value)) { - if (target.isName()) { - var = scope.getVar(target.getString()); - assignsByNode.add(target); - assignsByVar.put(var, new DestructuringAssign(n, target)); - return; - } - } else if (target.isDefaultValue() && target.getFirstChild().isName()) { - // TODO(johnlenz): this is awkward, consider refactoring this. - Node defaultTarget = target.getFirstChild(); - var = scope.getVar(defaultTarget.getString()); - markReferencedVar(var); - } - } + case OBJECT_PATTERN: + traverseObjectPattern(n, scope); break; - case STRING_KEY: - if (n.getParent().isObjectPattern()) { - Node target = n.getLastChild(); - if (target.isName()) { - var = scope.getVar(target.getString()); - assignsByNode.add(target); - assignsByVar.put(var, new DestructuringAssign(n, target)); - } - } + case OBJECTLIT: + traverseObjectLiteral(n, scope); break; - case NAME: - // the parameter declaration is not a read of the name, but we need to traverse - // to find default values and destructuring assignments - if (parent.isParamList()) { - break; - } + case FOR: + traverseVanillaFor(n, scope); + break; - var = scope.getVar(n.getString()); - if (NodeUtil.isNameDeclaration(parent)) { - Node value = n.getFirstChild(); - if (value != null && var != null && isRemovableVar(var) - && !NodeUtil.mayHaveSideEffects(value, compiler)) { - // If the var is unreferenced and creating its value has no side - // effects, then we can create a continuation for it instead - // of traversing immediately. - continuations.put(var, new Continuation(n, scope)); - return; - } - } else { - // If arguments is escaped, we just assume the worst and continue - // on all the parameters. Ignored if we are in block scope - if (var != null - && "arguments".equals(n.getString()) - && var.equals(scope.getArgumentsVar())) { - Scope fnScope = var.getScope(); - Node paramList = NodeUtil.getFunctionParameters(fnScope.getRootNode()); - for (Node p : NodeUtil.findLhsNodesInNode(paramList)) { - Var paramVar = fnScope.getOwnSlot(p.getString()); - checkNotNull(paramVar); - markReferencedVar(paramVar); - } - } + case FOR_IN: + case FOR_OF: + traverseEnhancedFor(n, scope); + break; + + case LET: + case CONST: + case VAR: + // for-loop cases are handled by custom traversal methods. + checkState(NodeUtil.isStatement(n)); + traverseDeclarationStatement(n, scope); + break; - // All name references that aren't declarations or assigns - // are references to other vars. + case NAME: + // The only cases that should reach this point are parameter declarations and references + // to names. The name node does not have children in these cases. + checkState(!n.hasChildren()); + // the parameter declaration is not a read of the name + if (!parent.isParamList()) { + // var|let|const name; + // are handled at a higher level. + checkState(!NodeUtil.isNameDeclaration(parent)); + // function name() {} + // class name() {} + // handled at a higher level + checkState(!((parent.isFunction() || parent.isClass()) && parent.getFirstChild() == n)); + var = scope.getVar(n.getString()); if (var != null) { - // If that var hasn't already been marked referenced, then - // start tracking it. If this is an assign, do nothing - // for now. - if (isRemovableVar(var)) { - if (!assignsByNode.contains(n)) { - markReferencedVar(var); - } - } else { - markReferencedVar(var); - } + // All name references that aren't handled elsewhere are references to vars. + getVarInfo(var).markAsReferenced(); } } break; + case GETPROP: + Node objectNode = n.getFirstChild(); + Node propertyNameNode = objectNode.getNext(); + String propertyName = propertyNameNode.getString(); + markPropertyNameReferenced(propertyName); + traverseNode(objectNode, scope); + break; + default: + traverseChildren(n, scope); break; } + } - traverseChildren(n, scope); + private void traverseCall(Node callNode, Scope scope) { + Node parent = callNode.getParent(); + String classVarName = null; + + // A call that is a statement unto itself or the left side of a comma expression might be + // a call to a known method for doing class setup + // e.g. $jscomp.inherits(Class, BaseClass) or goog.addSingletonGetter(Class) + // Such methods never have meaningful return values, so we won't look for them in other + // contexts + if (parent.isExprResult() || parent.isComma() && parent.getFirstChild() == callNode) { + SubclassRelationship subclassRelationship = + codingConvention.getClassesDefinedByCall(callNode); + if (subclassRelationship != null) { + // e.g. goog.inherits(DerivedClass, BaseClass); + // NOTE: DerivedClass and BaseClass must be QNames. Otherwise getClassesDefinedByCall() will + // return null. + classVarName = subclassRelationship.subclassName; + } else { + // Look for calls to addSingletonGetter calls. + classVarName = codingConvention.getSingletonGetterClassName(callNode); + } + } + + Var classVar = (classVarName == null) ? null : scope.getVar(classVarName); + + if (classVar == null || !classVar.isGlobal()) { + // This isn't one of the special call types, or it isn't acting on a global class name. + // It would be more correct to only not track when the class name does not + // reference a constructor, but checking that it is a global is easier and mostly the same. + traverseChildren(callNode, scope); + } else { + VarInfo classVarInfo = getVarInfo(classVar); + RemovableBuilder builder = new RemovableBuilder(); + for (Node child = callNode.getFirstChild(); child != null; child = child.getNext()) { + builder.addContinuation(new Continuation(child, scope)); + } + classVarInfo.addRemovable(builder.buildClassSetupCall(callNode)); + } } - private void traverseChildren(Node n, Scope scope) { - for (Node c = n.getFirstChild(); c != null; c = c.getNext()) { - traverseNode(c, n, scope); + private void traverseRest(Node restNode, Scope scope) { + Node target = restNode.getOnlyChild(); + if (!target.isName()) { + traverseNode(target, scope); + } else { + Var var = scope.getVar(target.getString()); + if (var != null) { + VarInfo varInfo = getVarInfo(var); + // NOTE: DestructuringAssign is currently used for both actual destructuring and + // default or rest parameters. + // TODO(bradfordcsmith): Maybe distinguish between these 2 cases. + varInfo.addRemovable(new RemovableBuilder().buildDestructuringAssign(restNode, target)); + } } } - private boolean isRemovableVar(Var var) { - // If this is a functions "arguments" object, it isn't removable - if (var.equals(var.getScope().getArgumentsVar())) { - return false; + private void traverseObjectLiteral(Node objectLiteral, Scope scope) { + for (Node propertyNode = objectLiteral.getFirstChild(); + propertyNode != null; + propertyNode = propertyNode.getNext()) { + if (propertyNode.isStringKey() && !propertyNode.isQuotedString()) { + // An unquoted property name in an object literal counts as a reference to that property + // name, because of some reflection patterns. + // TODO(bradfordcsmith): Handle this better for `Foo.prototype = {a: 1, b: 2}` + markPropertyNameReferenced(propertyNode.getString()); + traverseNode(propertyNode.getFirstChild(), scope); + } else { + traverseNode(propertyNode, scope); + } } + } - // Global variables are off-limits if the user might be using them. - if (!removeGlobals && var.isGlobal()) { - return false; + private void traverseCatch(Node catchNode, Scope scope) { + Node exceptionNameNode = catchNode.getFirstChild(); + Node block = exceptionNameNode.getNext(); + VarInfo exceptionVarInfo = getVarInfo(scope.getVar(exceptionNameNode.getString())); + exceptionVarInfo.setCannotRemoveAnything(); + traverseNode(block, scope); + } + + private void traverseEnhancedFor(Node enhancedFor, Scope scope) { + Scope forScope = scopeCreator.createScope(enhancedFor, scope); + // for (iterationTarget in|of collection) body; + Node iterationTarget = enhancedFor.getFirstChild(); + Node collection = iterationTarget.getNext(); + Node body = collection.getNext(); + if (iterationTarget.isName()) { + // using previously-declared loop variable. e.g. + // `for (varName of collection) {}` + Var var = forScope.getVar(iterationTarget.getString()); + // NOTE: var will be null if it was declared in externs + if (var != null) { + VarInfo varInfo = getVarInfo(var); + varInfo.setCannotRemoveAnything(); + } + } else if (NodeUtil.isNameDeclaration(iterationTarget)) { + // loop has const/var/let declaration + Node declNode = iterationTarget.getOnlyChild(); + if (declNode.isDestructuringLhs()) { + // e.g. + // `for (const [a, b] of pairList) {}` + // destructuring is handled at a lower level + // Note that destructuring assignments are always considered to set an unknown value + // equivalent to what we set for the var name case above and below. + // It isn't necessary to set the variable names as not removable, though, because the + // thing that isn't removable is the destructuring pattern itself, which we never remove. + // TODO(bradfordcsmith): The need to explain all the above shows this should be reworked. + traverseNode(declNode, forScope); + } else { + // e.g. + // `for (const varName of collection) {}` + checkState(declNode.isName()); + checkState(!declNode.hasChildren()); + // We can never remove the loop variable of a for-in or for-of loop, because it's + // essential to loop syntax. + VarInfo varInfo = getVarInfo(forScope.getVar(declNode.getString())); + varInfo.setCannotRemoveAnything(); + } + } else { + // using some general LHS value e.g. + // `for ([a, b] of collection) {}` destructuring with existing vars + // `for (a.x of collection) {}` using a property as the loop var + // TODO(bradfordcsmith): This should be considered a write if it's a property reference. + traverseNode(iterationTarget, forScope); } - // Variables declared in for in and for of loops are off limits - if (var.getParentNode() != null && NodeUtil.isEnhancedFor(var.getParentNode().getParent())) { - return false; + traverseNode(collection, forScope); + traverseNode(body, forScope); + } + + private void traverseVanillaFor(Node forNode, Scope scope) { + Scope forScope = scopeCreator.createScope(forNode, scope); + Node initialization = forNode.getFirstChild(); + Node condition = initialization.getNext(); + Node update = condition.getNext(); + Node block = update.getNext(); + if (NodeUtil.isNameDeclaration(initialization)) { + traverseVanillaForNameDeclarations(initialization, forScope); + } else { + traverseNode(initialization, forScope); } - // Referenced variables are off-limits. - if (referenced.contains(var)) { - return false; + traverseNode(condition, forScope); + traverseNode(update, forScope); + traverseNode(block, forScope); + } + + private void traverseVanillaForNameDeclarations(Node nameDeclaration, Scope scope) { + for (Node child = nameDeclaration.getFirstChild(); child != null; child = child.getNext()) { + if (!child.isName()) { + // TODO(bradfordcsmith): Customize handling of destructuring + traverseNode(child, scope); + } else { + Node nameNode = child; + @Nullable Node valueNode = child.getFirstChild(); + VarInfo varInfo = getVarInfo(scope.getVar(nameNode.getString())); + if (valueNode == null) { + varInfo.addRemovable(new RemovableBuilder().buildVanillaForNameDeclaration(nameNode)); + } else if (NodeUtil.mayHaveSideEffects(valueNode)) { + // TODO(bradfordcsmith): Actually allow for removing the variable while keeping the + // valueNode for its side-effects. + varInfo.setIsExplicitlyNotRemovable(); + traverseNode(valueNode, scope); + } else { + VanillaForNameDeclaration vanillaForNameDeclaration = + new RemovableBuilder() + .setAssignedValue(valueNode) + .addContinuation(new Continuation(valueNode, scope)) + .buildVanillaForNameDeclaration(nameNode); + varInfo.addRemovable(vanillaForNameDeclaration); + } + } } + } - // Exported variables are off-limits. - return !codingConvention.isExported(var.getName()); + private void traverseDeclarationStatement(Node declarationStatement, Scope scope) { + // Normalization should ensure that declaration statements always have just one child. + Node nameNode = declarationStatement.getOnlyChild(); + if (!nameNode.isName()) { + // TODO(bradfordcsmith): Customize handling of destructuring + traverseNode(nameNode, scope); + } else { + Node valueNode = nameNode.getFirstChild(); + VarInfo varInfo = getVarInfo(checkNotNull(scope.getVar(nameNode.getString()))); + RemovableBuilder builder = new RemovableBuilder(); + if (valueNode == null) { + varInfo.addRemovable(builder.buildNameDeclarationStatement(declarationStatement)); + } else { + if (NodeUtil.mayHaveSideEffects(valueNode)) { + traverseNode(valueNode, scope); + } else { + builder.addContinuation(new Continuation(valueNode, scope)); + } + NameDeclarationStatement removable = + builder.setAssignedValue(valueNode).buildNameDeclarationStatement(declarationStatement); + varInfo.addRemovable(removable); + } + } + } + + private void traverseAssign(Node assignNode, Scope scope) { + checkState(NodeUtil.isAssignmentOp(assignNode)); + + Node lhs = assignNode.getFirstChild(); + Node nameNode = null; + Node propertyNode = null; + boolean isVariableAssign = false; + boolean isComputedPropertyAssign = false; + boolean isNamedPropertyAssign = false; + + if (lhs.isName()) { + isVariableAssign = true; + nameNode = lhs; + } else if (NodeUtil.isGet(lhs)) { + propertyNode = lhs.getLastChild(); + Node possibleNameNode = lhs.getFirstChild(); + // Handle assignments to properties of a variable or its prototype property. + // However, don't handle any longer qualified names, because it gets hard to track + // properties of properties. + if (possibleNameNode.isGetProp() + && possibleNameNode.getSecondChild().getString().equals("prototype")) { + possibleNameNode = possibleNameNode.getFirstChild(); + } + if (possibleNameNode.isName()) { + nameNode = possibleNameNode; + if (lhs.isGetProp()) { + isNamedPropertyAssign = true; + } else { + checkState(lhs.isGetElem()); + isComputedPropertyAssign = true; + } + } + } + // else LHS is something else, like a destructuring pattern, which will be handled by + // traverseChildren() below + // TODO(bradfordcsmith): Handle destructuring at this level for better clarity and so we can + // do a better job with removal. + + // If we successfully identified a name node & there is a corresponding Var, + // then we have a removable assignment. + Var var = (nameNode == null) ? null : scope.getVar(nameNode.getString()); + if (var == null) { + traverseChildren(assignNode, scope); + } else { + Node valueNode = assignNode.getLastChild(); + RemovableBuilder builder = new RemovableBuilder().setAssignedValue(valueNode); + if (NodeUtil.isExpressionResultUsed(assignNode) || NodeUtil.mayHaveSideEffects(valueNode)) { + traverseNode(valueNode, scope); + } else { + builder.addContinuation(new Continuation(valueNode, scope)); + } + + VarInfo varInfo = getVarInfo(var); + if (isNamedPropertyAssign) { + varInfo.addRemovable(builder.buildNamedPropertyAssign(assignNode, nameNode, propertyNode)); + } else if (isVariableAssign) { + varInfo.addRemovable(builder.buildVariableAssign(assignNode, nameNode)); + } else { + checkState(isComputedPropertyAssign); + if (NodeUtil.mayHaveSideEffects(propertyNode)) { + traverseNode(propertyNode, scope); + } else { + builder.addContinuation(new Continuation(propertyNode, scope)); + } + varInfo.addRemovable( + builder.buildComputedPropertyAssign(assignNode, nameNode, propertyNode)); + } + } + } + + private void traverseDefaultValue(Node defaultValueNode, Scope scope) { + Var var; + Node target = defaultValueNode.getFirstChild(); + Node value = target.getNext(); + if (!target.isName()) { + traverseNode(target, scope); + traverseNode(value, scope); + } else { + var = scope.getVar(target.getString()); + if (var == null) { + traverseNode(value, scope); + } else { + VarInfo varInfo = getVarInfo(var); + if (NodeUtil.mayHaveSideEffects(value)) { + // TODO(johnlenz): we don't really need to retain all uses of the variable, just + // enough to host the default value assignment. + varInfo.markAsReferenced(); + traverseNode(value, scope); + } else { + DestructuringAssign assign = + new RemovableBuilder() + .addContinuation(new Continuation(value, scope)) + .buildDestructuringAssign(defaultValueNode, target); + varInfo.addRemovable(assign); + } + } + } + } + + private void traverseArrayPattern(Node arrayPattern, Scope scope) { + for (Node c = arrayPattern.getFirstChild(); c != null; c = c.getNext()) { + if (!c.isName()) { + // TODO(bradfordcsmith): Treat destructuring assignments to properties as removable writes. + traverseNode(c, scope); + } else { + Var var = scope.getVar(c.getString()); + if (var != null) { + VarInfo varInfo = getVarInfo(var); + varInfo.addRemovable(new RemovableBuilder().buildDestructuringAssign(c, c)); + } + } + } + } + + private void traverseObjectPattern(Node objectPattern, Scope scope) { + for (Node propertyNode = objectPattern.getFirstChild(); + propertyNode != null; + propertyNode = propertyNode.getNext()) { + traverseObjectPatternElement(propertyNode, scope); + } + } + + private void traverseObjectPatternElement(Node elm, Scope scope) { + // non-null for computed properties + // `{[propertyExpression]: target} = ...` + Node propertyExpression = null; + // non-null for named properties + // `{propertyName: target} = ...` + String propertyName = null; + Node target = null; + Node defaultValue = null; + + // Get correct values for all the variables above. + if (elm.isComputedProp()) { + propertyExpression = elm.getFirstChild(); + target = elm.getLastChild(); + } else { + checkState(elm.isStringKey()); + target = elm.getOnlyChild(); + // Treat `{'a': x} = ...` like `{['a']: x} = ...`, but it never has side-effects and we + // have no propertyExpression to traverse. + // NOTE: The parser will convert `{1: x} = ...` to `{'1': x} = ...` + if (!elm.isQuotedString()) { + propertyName = elm.getString(); + } + } + + if (target.isDefaultValue()) { + target = target.getFirstChild(); + defaultValue = checkNotNull(target.getNext()); + } + + // TODO(bradfordcsmith): Handle property assignments also + Var var = target.isName() ? scope.getVar(target.getString()) : null; + + // TODO(bradfordcsmith): Arrange to safely remove side-effect cases. + boolean cannotRemove = + var == null + || (propertyExpression != null && NodeUtil.mayHaveSideEffects(propertyExpression)) + || (defaultValue != null && NodeUtil.mayHaveSideEffects(defaultValue)); + + if (cannotRemove) { + if (propertyExpression != null) { + traverseNode(propertyExpression, scope); + } + if (propertyName != null) { + markPropertyNameReferenced(propertyName); + } + traverseNode(target, scope); + if (defaultValue != null) { + traverseNode(defaultValue, scope); + } + if (var != null) { + // Since we cannot remove it, we must now treat this usage as a reference. + getVarInfo(var).markAsReferenced(); + } + } else { + RemovableBuilder builder = new RemovableBuilder(); + if (propertyName != null) { + // TODO(bradfordcsmith): Use a continuation here. + markPropertyNameReferenced(propertyName); + } + if (propertyExpression != null) { + builder.addContinuation(new Continuation(propertyExpression, scope)); + } + if (defaultValue != null) { + builder.addContinuation(new Continuation(defaultValue, scope)); + } + getVarInfo(var).addRemovable(builder.buildDestructuringAssign(elm, target)); + } + } + + private void traverseChildren(Node n, Scope scope) { + for (Node c = n.getFirstChild(); c != null; c = c.getNext()) { + traverseNode(c, scope); + } + } + + private void traverseClass(Node classNode, Scope scope) { + checkArgument(classNode.isClass()); + Node classNameNode = classNode.getFirstChild(); + Node baseClassExpression = classNameNode.getNext(); + Node classBodyNode = baseClassExpression.getNext(); + Scope classScope = scopeCreator.createScope(classNode, scope); + if (!NodeUtil.isNamedClass(classNode) || classNode.getParent().isExport()) { + // If this isn't a named class, there's no var to consider here. + // If it is exported, it definitely cannot be removed. + traverseNode(baseClassExpression, classScope); + traverseNode(classBodyNode, classScope); + } else if (NodeUtil.mayHaveSideEffects(baseClassExpression)) { + // TODO(bradfordcsmith): implement removal without losing side-effects for this case + traverseNode(baseClassExpression, classScope); + traverseNode(classBodyNode, classScope); + } else { + RemovableBuilder builder = + new RemovableBuilder() + .addContinuation(new Continuation(baseClassExpression, classScope)) + .addContinuation(new Continuation(classBodyNode, classScope)); + VarInfo varInfo = getVarInfo(classScope.getVar(classNameNode.getString())); + if (NodeUtil.isClassDeclaration(classNode)) { + varInfo.addRemovable(builder.buildClassDeclaration(classNode)); + } else { + varInfo.addRemovable(builder.buildNamedClassExpression(classNode)); + } + } } /** @@ -449,27 +756,40 @@ private void traverseFunction(Node function, Scope parentScope) { // Checking the parameters Scope fparamScope = scopeCreator.createScope(function, parentScope); - collectMaybeUnreferencedVars(fparamScope); // Checking the function body Scope fbodyScope = scopeCreator.createScope(body, fparamScope); - collectMaybeUnreferencedVars(fbodyScope); + + // for cases like + // var x = function funcName() {}; + // make sure funcName gets into the varInfoMap so it will be considered for removal. + getFunctionNameVarInfo(function, fparamScope); traverseChildren(paramlist, fparamScope); + if (NodeUtil.isVarArgsFunction(function)) { + // if arguments is referenced anywhere, we'll assume we cannot remove any parameters + for (Node p : NodeUtil.findLhsNodesInNode(paramlist)) { + Var paramVar = checkNotNull(fparamScope.getOwnSlot(p.getString())); + getVarInfo(paramVar).markAsReferenced(); + } + } traverseChildren(body, fbodyScope); allFunctionParamScopes.add(fparamScope); } - /** - * For each variable in this scope that we haven't found a reference - * for yet, add it to the list of variables to check later. - */ - private void collectMaybeUnreferencedVars(Scope scope) { - for (Var var : scope.getVarIterable()) { - if (isRemovableVar(var)) { - maybeUnreferenced.add(var); - } + @Nullable + private VarInfo getFunctionNameVarInfo(Node function, Scope scope) { + Node nameNode = checkNotNull(function.getFirstChild()); + checkState(nameNode.isName()); + String name = nameNode.getString(); + if (name.isEmpty()) { + // function() {} + return null; + } else { + // function name() {} + Var var = checkNotNull(scope.getVar(name)); + return getVarInfo(var); } } @@ -515,6 +835,15 @@ private void removeUnreferencedFunctionArgs(Scope fparamScope) { markUnusedParameters(argList, fparamScope); } + private void markPropertyNameReferenced(String propertyName) { + if (referencedPropertyNames.add(propertyName)) { + // on first reference, find all vars having that property and tell them it is now referenced. + for (VarInfo varInfo : varInfoForPropertyNameMap.get(propertyName)) { + varInfo.markPropertyNameReferenced(propertyName); + } + } + } + /** * Mark any remaining unused parameters as being unused so it can be used elsewhere. * @@ -535,7 +864,8 @@ private void markUnusedParameters(Node paramList, Scope fparamScope) { continue; } Var var = fparamScope.getVar(lValue.getString()); - if (!referenced.contains(var)) { + VarInfo varInfo = getVarInfo(var); + if (varInfo.isRemovable()) { param.setUnusedParameter(true); compiler.reportChangeToEnclosingScope(paramList); } @@ -582,7 +912,8 @@ private void maybeRemoveUnusedTrailingParameters(Node argList, Scope fparamScope } Var var = fparamScope.getVar(lValue.getString()); - if (!referenced.contains(var)) { + VarInfo varInfo = getVarInfo(var); + if (varInfo.isRemovable()) { NodeUtil.deleteNode(lastArg, compiler); } else { break; @@ -590,202 +921,49 @@ private void maybeRemoveUnusedTrailingParameters(Node argList, Scope fparamScope } } - /** - * Look at all the property assigns to all variables. - * These may or may not count as references. For example, - * - * - * var x = {}; - * x.foo = 3; // not a reference. - * var y = foo(); - * y.foo = 3; // is a reference. - * - * - * Interpreting assignments could mark a variable as referenced that - * wasn't referenced before, in order to keep it alive. Because we find - * references by lazily traversing subtrees, marking a variable as - * referenced could trigger new traversals of new subtrees, which could - * find new references. - * - * Therefore, this interpretation needs to be run to a fixed point. - */ - private void interpretAssigns() { - boolean changes = false; - do { - changes = false; - - // We can't use traditional iterators and iterables for this list, - // because our lazily-evaluated continuations will modify it while - // we traverse it. - int removedCount = 0; - for (int current = 0; current < maybeUnreferenced.size(); current++) { - Var var = maybeUnreferenced.get(current); - if (var == null) { - continue; - } - if (referenced.contains(var)) { - maybeUnreferenced.set(current, null); - removedCount++; - } else { - boolean assignedToUnknownValue = false; - - if (NodeUtil.isNameDeclaration(var.getParentNode()) - && !var.getParentNode().getParent().isForIn()) { - Node value = var.getInitialValue(); - assignedToUnknownValue = value != null - && !NodeUtil.isLiteralValue(value, true); - } else if (NodeUtil.isFunctionDeclaration(var.getParentNode())) { - assignedToUnknownValue = false; - } else { - // This was initialized to a function arg or a catch param - // or a for...in variable. - assignedToUnknownValue = true; - } - - boolean maybeEscaped = false; - boolean hasPropertyAssign = false; - for (Removable removable : assignsByVar.get(var)) { - if (removable instanceof DestructuringAssign) { - assignedToUnknownValue = true; - continue; - } - Assign assign = (Assign) removable; - if (assign.isPropertyAssign) { - hasPropertyAssign = true; - } else if (!NodeUtil.isLiteralValue( - assign.assignNode.getLastChild(), true)) { - assignedToUnknownValue = true; - } - if (assign.maybeAliased) { - maybeEscaped = true; - } - } - - if ((assignedToUnknownValue || maybeEscaped) && hasPropertyAssign) { - changes = markReferencedVar(var) || changes; - maybeUnreferenced.set(current, null); - removedCount++; - } - } - } - - // Removing unused items from the middle of an array list is relatively expensive, - // so we batch them up and remove them all at the end. - if (removedCount > 0) { - int size = maybeUnreferenced.size(); - ArrayList refreshed = new ArrayList<>(size - removedCount); - for (int i = 0; i < size; i++) { - Var var = maybeUnreferenced.get(i); - if (var != null) { - refreshed.add(var); - } - } - maybeUnreferenced = refreshed; - } - } while (changes); - } - - /** - * Remove all assigns to a var. - */ - private void removeAllAssigns(Var var) { - for (Removable removable : assignsByVar.get(var)) { - removable.remove(compiler); - } - } - - /** - * Marks a var as referenced, recursing into any values of this var - * that we skipped. - * @return True if this variable had not been referenced before. - */ - private boolean markReferencedVar(Var var) { - if (referenced.add(var)) { - for (Continuation c : continuations.get(var)) { - c.apply(); - } - return true; + private VarInfo getVarInfo(Var var) { + checkNotNull(var); + VarInfo varInfo = varInfoMap.get(var); + if (varInfo == null) { + varInfo = new VarInfo(var); + varInfoMap.put(var, varInfo); } - return false; + return varInfo; } /** - * Removes any vars in the scope that were not referenced. Removes any - * assignments to those variables as well. + * Removes any vars in the scope that were not referenced. Removes any assignments to those + * variables as well. */ private void removeUnreferencedVars() { - for (Var var : maybeUnreferenced) { - // Remove calls to inheritance-defining functions where the unreferenced - // class is the subclass. - for (Node exprCallNode : classDefiningCalls.get(var)) { - compiler.reportChangeToEnclosingScope(exprCallNode); - NodeUtil.removeChild(exprCallNode.getParent(), exprCallNode); + for (VarInfo varInfo : varInfoMap.values()) { + if (!varInfo.isRemovable()) { + continue; } // Regardless of what happens to the original declaration, // we need to remove all assigns, because they may contain references // to other unreferenced variables. - removeAllAssigns(var); + varInfo.removeAllRemovables(); - compiler.addToDebugLog("Unreferenced var: ", var.name); - Node nameNode = var.nameNode; + compiler.addToDebugLog("Unreferenced var: ", varInfo.var.name); + Node nameNode = varInfo.var.nameNode; Node toRemove = nameNode.getParent(); - if (toRemove == null) { - // array pattern assignments may have already been removed - continue; - } - - Node parent = toRemove != null ? toRemove.getParent() : null; - Node grandParent = parent != null ? parent.getParent() : null; - - if (toRemove.isDefaultValue() || toRemove.isRest()) { - // Rest and default value declarations should already have been removed. - checkState(parent == null || grandParent == null); - } else if (toRemove.isStringKey() || toRemove.isComputedProp()) { - checkState(parent == null, "unremoved destructuring ", toRemove); - } else if (toRemove.isParamList()) { - // Don't remove function arguments here. That's a special case - // that's taken care of in removeUnreferencedFunctionArgs. - } else if (toRemove.isComputedProp()) { - // Don't remove a computed property + if (toRemove == null || alreadyRemoved(toRemove)) { + // varInfo.removeAllRemovables () already removed it } else if (NodeUtil.isFunctionExpression(toRemove)) { + // TODO(bradfordcsmith): Add a Removable for this case. if (!preserveFunctionExpressionNames) { Node fnNameNode = toRemove.getFirstChild(); compiler.reportChangeToEnclosingScope(fnNameNode); fnNameNode.setString(""); } - // Don't remove bleeding functions. - } else if (toRemove.isArrayPattern() && grandParent.isParamList()) { - compiler.reportChangeToEnclosingScope(toRemove); - NodeUtil.removeChild(toRemove, nameNode); - } else if (parent.isForIn()) { - // foreach iterations have 3 children. Leave them alone. - } else if (parent.isDestructuringPattern()) { - compiler.reportChangeToEnclosingScope(toRemove); - NodeUtil.removeChild(parent, toRemove); - } else if (parent.isDestructuringLhs()) { - compiler.reportChangeToEnclosingScope(nameNode); - NodeUtil.removeChild(toRemove, nameNode); - } else if (NodeUtil.isNameDeclaration(toRemove) - && nameNode.hasChildren() - && NodeUtil.mayHaveSideEffects(nameNode.getFirstChild(), compiler)) { - // If this is a single var declaration, we can at least remove the - // declaration itself and just leave the value, e.g., - // var a = foo(); => foo(); - if (toRemove.hasOneChild()) { - compiler.reportChangeToEnclosingScope(toRemove); - parent.replaceChild(toRemove, - IR.exprResult(nameNode.removeFirstChild())); - } - } else if (NodeUtil.isNameDeclaration(toRemove) && toRemove.hasMoreThanOneChild()) { - // For var declarations with multiple names (i.e. var a, b, c), - // only remove the unreferenced name - compiler.reportChangeToEnclosingScope(toRemove); - toRemove.removeChild(nameNode); - } else if (parent != null) { - compiler.reportChangeToEnclosingScope(toRemove); - NodeUtil.removeChild(parent, toRemove); - NodeUtil.markFunctionsDeleted(toRemove, compiler); + } else if (toRemove.isParamList()) { + // TODO(bradfordcsmith): handle parameter declarations with removables + // Don't remove function arguments here. That's a special case + // that's taken care of in removeUnreferencedFunctionArgs. + } else { + throw new IllegalStateException("unremoved code"); } } } @@ -805,25 +983,161 @@ private class Continuation { } void apply() { - if (NodeUtil.isFunctionDeclaration(node)) { + if (node.isFunction()) { + // Calling traverseNode here would create infinite recursion for a function declaration traverseFunction(node, scope); } else { - for (Node child = node.getFirstChild(); child != null; child = child.getNext()) { - traverseNode(child, node, scope); + traverseNode(node, scope); + } + } + } + + /** Represents a portion of the AST that can be removed. */ + private abstract class Removable { + + private final List continuations; + @Nullable private final String propertyName; + @Nullable private final Node assignedValue; + + private boolean continuationsAreApplied = false; + private boolean isRemoved = false; + + Removable(RemovableBuilder builder) { + continuations = builder.continuations; + propertyName = builder.propertyName; + assignedValue = builder.assignedValue; + } + + String getPropertyName() { + checkState(isNamedPropertyAssignment()); + return checkNotNull(propertyName); + } + + /** Remove the associated nodes from the AST. */ + abstract void removeInternal(AbstractCompiler compiler); + + /** Remove the associated nodes from the AST, unless they've already been removed. */ + void remove(AbstractCompiler compiler) { + if (!isRemoved) { + isRemoved = true; + removeInternal(compiler); + } + } + + public void applyContinuations() { + if (!continuationsAreApplied) { + continuationsAreApplied = true; + for (Continuation c : continuations) { + // Enqueue the continuation for processing. + // Don't invoke the continuation immediately, because that can lead to concurrent + // modification of data structures. + worklist.add(c); } + continuations.clear(); } } + + boolean isLiteralValueAssignment() { + // An assigned value of null occurs for a name declaration when no initializer is given. + // It is the same as assigning `undefined`, so it is a literal value. + return assignedValue == null + || NodeUtil.isLiteralValue(assignedValue, /* includeFunctions */ true); + } + + /** True if this object represents assignment to a variable. */ + boolean isVariableAssignment() { + return false; + } + + /** True if this object represents assignment of a value to a property. */ + boolean isPropertyAssignment() { + return false; + } + + /** True if this object represents assignment to a named property. */ + boolean isNamedPropertyAssignment() { + return propertyName != null; + } + + /** + * True if this object has an assigned value that may escape to another context through aliasing + * or some other means. + */ + boolean assignedValueMayEscape() { + return false; + } + + boolean isPrototypeAssignment() { + return isNamedPropertyAssignment() && propertyName.equals("prototype"); + } } - private static interface Removable { - public void remove(AbstractCompiler compiler); + private class RemovableBuilder { + final List continuations = new ArrayList<>(); + + @Nullable String propertyName = null; + @Nullable public Node assignedValue = null; + + RemovableBuilder addContinuation(Continuation continuation) { + continuations.add(continuation); + return this; + } + + RemovableBuilder setAssignedValue(@Nullable Node assignedValue) { + this.assignedValue = assignedValue; + return this; + } + + DestructuringAssign buildDestructuringAssign(Node removableNode, Node nameNode) { + return new DestructuringAssign(this, removableNode, nameNode); + } + + ClassDeclaration buildClassDeclaration(Node classNode) { + return new ClassDeclaration(this, classNode); + } + + NamedClassExpression buildNamedClassExpression(Node classNode) { + return new NamedClassExpression(this, classNode); + } + + FunctionDeclaration buildFunctionDeclaration(Node functionNode) { + return new FunctionDeclaration(this, functionNode); + } + + NameDeclarationStatement buildNameDeclarationStatement(Node declarationStatement) { + return new NameDeclarationStatement(this, declarationStatement); + } + + Assign buildNamedPropertyAssign(Node assignNode, Node nameNode, Node propertyNode) { + this.propertyName = propertyNode.getString(); + checkNotNull(assignedValue); + return new Assign(this, assignNode, nameNode, Kind.NAMED_PROPERTY, propertyNode); + } + + Assign buildComputedPropertyAssign(Node assignNode, Node nameNode, Node propertyNode) { + checkNotNull(assignedValue); + return new Assign(this, assignNode, nameNode, Kind.COMPUTED_PROPERTY, propertyNode); + } + + Assign buildVariableAssign(Node assignNode, Node nameNode) { + return new Assign(this, assignNode, nameNode, Kind.VARIABLE, /* propertyNode */ null); + } + + ClassSetupCall buildClassSetupCall(Node callNode) { + return new ClassSetupCall(this, callNode); + } + + VanillaForNameDeclaration buildVanillaForNameDeclaration(Node nameNode) { + return new VanillaForNameDeclaration(this, nameNode); + } } - private class DestructuringAssign implements Removable { + private class DestructuringAssign extends Removable { final Node removableNode; final Node nameNode; - DestructuringAssign(Node removableNode, Node nameNode) { + DestructuringAssign(RemovableBuilder builder, Node removableNode, Node nameNode) { + super(builder); checkState(nameNode.isName()); this.removableNode = removableNode; this.nameNode = nameNode; @@ -835,7 +1149,24 @@ private class DestructuringAssign implements Removable { } @Override - public void remove(AbstractCompiler compiler) { + boolean isVariableAssignment() { + // TODO(bradfordcsmith): Handle destructuring assignments to properties. + return true; + } + + @Override + boolean isLiteralValueAssignment() { + // TODO(bradfordcsmith): Determine assigned value when possible. + // We don't look at the rhs of destructuring assignments at all right now, + // so assume they always assign some non-literal value. + return false; + } + + @Override + public void removeInternal(AbstractCompiler compiler) { + if (alreadyRemoved(removableNode)) { + return; + } Node removableParent = removableNode.getParent(); if (removableParent.isArrayPattern()) { // [a, removableName, b] = something; @@ -895,104 +1226,222 @@ && canRemoveParameters(removableParent)) { } } - private static class Assign implements Removable { + private class ClassDeclaration extends Removable { + final Node classDeclarationNode; - final Node assignNode; + ClassDeclaration(RemovableBuilder builder, Node classDeclarationNode) { + super(builder); + this.classDeclarationNode = classDeclarationNode; + } + + @Override + public void removeInternal(AbstractCompiler compiler) { + NodeUtil.deleteNode(classDeclarationNode, compiler); + } + } + + private class NamedClassExpression extends Removable { + final Node classNode; + + NamedClassExpression(RemovableBuilder builder, Node classNode) { + super(builder); + this.classNode = classNode; + } + + @Override + public void removeInternal(AbstractCompiler compiler) { + if (!alreadyRemoved(classNode)) { + Node nameNode = classNode.getFirstChild(); + if (!nameNode.isEmpty()) { + // Just empty the class's name. If the expression is assigned to an unused variable, + // then the whole class might still be removed as part of that assignment. + classNode.replaceChild(nameNode, IR.empty().useSourceInfoFrom(nameNode)); + compiler.reportChangeToEnclosingScope(classNode); + } + } + } + } + + private class FunctionDeclaration extends Removable { + final Node functionDeclarationNode; + + FunctionDeclaration(RemovableBuilder builder, Node functionDeclarationNode) { + super(builder); + this.functionDeclarationNode = functionDeclarationNode; + } + @Override + public void removeInternal(AbstractCompiler compiler) { + NodeUtil.deleteNode(functionDeclarationNode, compiler); + } + } + + private class NameDeclarationStatement extends Removable { + private final Node declarationStatement; + + public NameDeclarationStatement(RemovableBuilder builder, Node declarationStatement) { + super(builder); + this.declarationStatement = declarationStatement; + } + + @Override + void removeInternal(AbstractCompiler compiler) { + Node nameNode = declarationStatement.getOnlyChild(); + Node valueNode = nameNode.getFirstChild(); + if (valueNode != null && NodeUtil.mayHaveSideEffects(valueNode)) { + compiler.reportChangeToEnclosingScope(declarationStatement); + valueNode.detach(); + declarationStatement.replaceWith(IR.exprResult(valueNode).useSourceInfoFrom(valueNode)); + } else { + NodeUtil.deleteNode(declarationStatement, compiler); + } + } + + @Override + boolean isVariableAssignment() { + return true; + } + + } + + enum Kind { + // X = something; + VARIABLE, + // X.propertyName = something; + // X.prototype.propertyName = something; + NAMED_PROPERTY, + // X[expression] = something; + // X.prototype[expression] = something; + COMPUTED_PROPERTY; + } + + private class Assign extends Removable { + + final Node assignNode; final Node nameNode; + final Kind kind; - // If false, then this is an assign to the normal variable. Otherwise, - // this is an assign to a property of that variable. - final boolean isPropertyAssign; - - // Secondary side effects are any side effects in this assign statement - // that aren't caused by the assignment operation itself. For example, - // a().b = 3; - // a = b(); - // var foo = (a = b); - // In the first two cases, the sides of the assignment have side-effects. - // In the last one, the result of the assignment is used, so we - // are conservative and assume that it may be used in a side-effecting - // way. - final boolean mayHaveSecondarySideEffects; + @Nullable final Node propertyNode; // If true, the value may have escaped and any modification is a use. final boolean maybeAliased; - Assign(Node assignNode, Node nameNode, boolean isPropertyAssign) { + Assign( + RemovableBuilder builder, + Node assignNode, + Node nameNode, + Kind kind, + @Nullable Node propertyNode) { + super(builder); checkState(NodeUtil.isAssignmentOp(assignNode)); + if (kind == Kind.VARIABLE) { + checkArgument( + propertyNode == null, + "got property node for simple variable assignment: %s", + propertyNode); + } else { + checkArgument(propertyNode != null, "missing property node"); + if (kind == Kind.NAMED_PROPERTY) { + checkArgument(propertyNode.isString(), "property name is not a string: %s", propertyNode); + } + } this.assignNode = assignNode; this.nameNode = nameNode; - this.isPropertyAssign = isPropertyAssign; + this.kind = kind; + this.propertyNode = propertyNode; this.maybeAliased = NodeUtil.isExpressionResultUsed(assignNode); - this.mayHaveSecondarySideEffects = - maybeAliased - || NodeUtil.mayHaveSideEffects(assignNode.getFirstChild()) - || NodeUtil.mayHaveSideEffects(assignNode.getLastChild()); } - /** - * If this is an assign to a variable or its property, return it. - * Otherwise, return null. - */ - static Assign maybeCreateAssign(Node assignNode) { - checkState(NodeUtil.isAssignmentOp(assignNode)); + @Override + boolean assignedValueMayEscape() { + return maybeAliased; + } - // Skip one level of GETPROPs or GETELEMs. - // - // Don't skip more than one level, because then we get into - // situations where assigns to properties of properties will always - // trigger side-effects, and the variable they're on cannot be removed. - boolean isPropAssign = false; - Node current = assignNode.getFirstChild(); - if (NodeUtil.isGet(current)) { - current = current.getFirstChild(); - isPropAssign = true; - - if (current.isGetProp() - && current.getLastChild().getString().equals("prototype")) { - // Prototype properties sets should be considered like normal - // property sets. - current = current.getFirstChild(); - } - } + /** True for `varName = value` assignments. */ + @Override + boolean isVariableAssignment() { + return kind == Kind.VARIABLE; + } - if (current.isName()) { - return new Assign(assignNode, current, isPropAssign); - } - return null; + @Override + boolean isPropertyAssignment() { + return isNamedPropertyAssignment() || isComputedPropertyAssignment(); } - /** Replace the current assign with its right hand side. */ + /** True for `varName.propName = value` and `varName.prototype.propName = value` assignments. */ @Override - public void remove(AbstractCompiler compiler) { - compiler.reportChangeToEnclosingScope(assignNode); - if (mayHaveSecondarySideEffects) { - Node replacement = assignNode.getLastChild().detach(); - - // Aggregate any expressions in GETELEMs. - for (Node current = assignNode.getFirstChild(); - !current.isName(); - current = current.getFirstChild()) { - if (current.isGetElem()) { - replacement = IR.comma( - current.getLastChild().detach(), replacement); - replacement.useSourceInfoIfMissingFrom(current); - } - } + boolean isNamedPropertyAssignment() { + return kind == Kind.NAMED_PROPERTY; + } + /** True for `varName[expr] = value` and `varName.prototype[expr] = value` assignments. */ + boolean isComputedPropertyAssignment() { + return kind == Kind.COMPUTED_PROPERTY; + } + + /** Replace the current assign with its right hand side. */ + @Override + public void removeInternal(AbstractCompiler compiler) { + if (alreadyRemoved(assignNode)) { + return; + } + Node parent = assignNode.getParent(); + compiler.reportChangeToEnclosingScope(parent); + Node lhs = assignNode.getFirstChild(); + Node rhs = assignNode.getSecondChild(); + boolean mustPreserveRhs = + NodeUtil.mayHaveSideEffects(rhs) || NodeUtil.isExpressionResultUsed(assignNode); + boolean mustPreserveGetElmExpr = + lhs.isGetElem() && NodeUtil.mayHaveSideEffects(lhs.getLastChild()); + + if (mustPreserveRhs && mustPreserveGetElmExpr) { + Node replacement = + IR.comma(lhs.getLastChild().detach(), rhs.detach()).useSourceInfoFrom(assignNode); assignNode.replaceWith(replacement); + } else if (mustPreserveGetElmExpr) { + assignNode.replaceWith(lhs.getLastChild().detach()); + NodeUtil.markFunctionsDeleted(rhs, compiler); + } else if (mustPreserveRhs) { + assignNode.replaceWith(rhs.detach()); + NodeUtil.markFunctionsDeleted(lhs, compiler); + } else if (parent.isExprResult()) { + parent.detach(); + NodeUtil.markFunctionsDeleted(parent, compiler); } else { - Node parent = assignNode.getParent(); - if (parent.isExprResult()) { - parent.detach(); - NodeUtil.markFunctionsDeleted(parent, compiler); - } else { - // mayHaveSecondarySideEffects is false, which means the value isn't needed, - // but we need to keep the AST valid. - assignNode.replaceWith(IR.number(0).srcref(assignNode)); - } + // value isn't needed, but we need to keep the AST valid. + assignNode.replaceWith(IR.number(0).useSourceInfoFrom(assignNode)); + NodeUtil.markFunctionsDeleted(assignNode, compiler); + } + } + } + + /** + * Represents a call to a class setup method such as `goog.inherits()` or + * `goog.addSingletonGetter()`. + */ + private class ClassSetupCall extends Removable { + + final Node callNode; + + ClassSetupCall(RemovableBuilder builder, Node callNode) { + super(builder); + this.callNode = callNode; + } + + @Override + public void removeInternal(AbstractCompiler compiler) { + Node parent = callNode.getParent(); + // NOTE: The call must either be its own statement or the LHS of a comma expression, + // because it doesn't have a meaningful return value. + if (parent.isExprResult()) { + NodeUtil.deleteNode(parent, compiler); + } else { + // `(goog.inherits(A, B), something)` -> `something` + checkState(parent.isComma()); + Node rhs = checkNotNull(callNode.getNext()); + compiler.reportChangeToEnclosingScope(parent); + parent.replaceWith(rhs.detach()); } } } @@ -1007,4 +1456,262 @@ private static boolean alreadyRemoved(Node n) { } return alreadyRemoved(parent); } + + private class VarInfo { + final Var var; + + /** + * Objects that represent variable declarations, assignments, or class setup calls that can + * be removed. + * + * NOTE: Once we realize that we cannot remove the variable, this list will be cleared and + * no more will be added. + */ + final List removables = new ArrayList<>(); + + /** + * Objects that represent assignments to named properties on the variable or on + * `varName.prototype`. These can be considered for removal even if the variable itself + * cannot. + * + * NOTE: Once we realize that we cannot remove a property, the removables for that property + * will be dropped and no more will be added. + */ + Multimap namedPropertyRemovables = null; + + boolean isEntirelyRemovable = true; + + /** + * Is it OK to remove properties that appear unused when we are leaving the variable itself in + * place? + * + *

Defaults to true if we allow this behavior at all. + * Once set to false, it will not be changed back to true. + */ + boolean unreferencedPropertiesMayBeRemoved = removeUnusedProperties; + + /** + * Used along with hasPropertyAssignments to handle cases where property assignment may have + * an unknown side-effect, and so make it unsafe to remove the variable. + * + * If we're unsure where the variable's value comes from, then setting a property on it may + * have a side-effect we cannot easily detect. + */ + boolean propertyAssignmentsWillPreventRemoval = false; + + /** + * Records whether any properties are set on the variable. + * + * This includes both named properties (`x.propName =`) and computed ones (`x[expr] = `). + * It is used in combination with propertyAssignmentsWillPreventRemoval. + */ + boolean hasPropertyAssignments = false; + + VarInfo(Var var) { + this.var = var; + + boolean isGlobal = var.isGlobal(); + if (isGlobal && !removeGlobals) { + // TODO(bradfordcsmith): Should we allow removal of properties here? + setCannotRemoveAnything(); + } else if (codingConvention.isExported(var.getName(), !isGlobal)) { + setCannotRemoveAnything(); + } else if (var.isArguments()) { + // TODO(bradfordcsmith): Create a single VarInfo to represent all "arguments" to save space. + setCannotRemoveAnything(); + } else if (var.getParentNode().isParamList()) { + propertyAssignmentsWillPreventRemoval = true; + unreferencedPropertiesMayBeRemoved = false; + } + } + + void addRemovable(Removable removable) { + // determine how this removable affects removability + if (removable.isPropertyAssignment()) { + hasPropertyAssignments = true; + if (removable.isPrototypeAssignment() && removable.assignedValueMayEscape()) { + // Assignment to properties could have unexpected side-effects. + // x = varName.prototype = {}; + // foo(varName.prototype = {}); + // NOTE: Arguably we should also check for literal value assignment, but that would + // prevent us from removing cases like this one. + // Foo.prototype = { + // constructor: Foo, // not considered a literal value. + // ... + // }; + propertyAssignmentsWillPreventRemoval = true; + } + if (propertyAssignmentsWillPreventRemoval) { + setIsExplicitlyNotRemovable(); + } + } else if (removable.isVariableAssignment() + && (removable.assignedValueMayEscape() || !removable.isLiteralValueAssignment())) { + // Assignment to properties could have unexpected side-effects. + // x = varName = {}; + // foo(varName = {}); + // varName = foo(); + propertyAssignmentsWillPreventRemoval = true; + if (hasPropertyAssignments) { + setIsExplicitlyNotRemovable(); + } + } + + // immediately apply continuations, or save the removable for possible removal + if (removable.isNamedPropertyAssignment()) { + String propertyName = removable.getPropertyName(); + + if (isPropertyRemovable(propertyName)) { + if (namedPropertyRemovables == null) { + namedPropertyRemovables = HashMultimap.create(); + } + namedPropertyRemovables.put(propertyName, removable); + varInfoForPropertyNameMap.put(propertyName, this); + } else { + removable.applyContinuations(); + } + } else if (isEntirelyRemovable) { + removables.add(removable); + } else { + removable.applyContinuations(); + } + } + + /** + * Marks this variable as referenced and evaluates any continuations if not previously marked as + * referenced. + * + * @return true if the variable was not already marked as referenced + */ + boolean markAsReferenced() { + return setIsExplicitlyNotRemovable(); + } + + void markPropertyNameReferenced(String propertyName) { + // Only apply continuations and drop the removals for the name if we've decided we cannot + // remove this variable entirely. + if (!isEntirelyRemovable && namedPropertyRemovables != null) { + for (Removable r : namedPropertyRemovables.removeAll(propertyName)) { + r.applyContinuations(); + } + } + } + + boolean isRemovable() { + return isEntirelyRemovable; + } + + /** + * Do not remove this variable or any of its property assignments. + */ + void setCannotRemoveAnything() { + unreferencedPropertiesMayBeRemoved = false; + setIsExplicitlyNotRemovable(); + } + + boolean isPropertyRemovable(String propertyName) { + return isEntirelyRemovable + || unreferencedPropertiesMayBeRemoved && !referencedPropertyNames.contains(propertyName); + } + + boolean setIsExplicitlyNotRemovable() { + if (isEntirelyRemovable) { + isEntirelyRemovable = false; + for (Removable r : removables) { + r.applyContinuations(); + } + removables.clear(); + if (namedPropertyRemovables != null) { + // iterate over a copy to avoid ConcurrentModificationException when we remove keys + // within the loop + for (String propertyName : ImmutableList.copyOf(namedPropertyRemovables.keySet())) { + if (!isPropertyRemovable(propertyName)) { + for (Removable r : namedPropertyRemovables.removeAll(propertyName)) { + r.applyContinuations(); + } + } + } + } + return true; + } else { + return false; + } + } + + void removeAllRemovables() { + checkState(isEntirelyRemovable); + for (Removable removable : removables) { + removable.remove(compiler); + } + removables.clear(); + if (namedPropertyRemovables != null) { + for (Removable removable : namedPropertyRemovables.values()) { + removable.remove(compiler); + } + namedPropertyRemovables.clear(); + namedPropertyRemovables = null; + } + } + + void removeUnreferencedProperties() { + checkState(!isEntirelyRemovable && unreferencedPropertiesMayBeRemoved); + if (namedPropertyRemovables != null) { + // iterate over a copy to avoid ConcurrentModificationException when we remove keys + // within the loop + for (String propertyName : ImmutableList.copyOf(namedPropertyRemovables.keySet())) { + // There shouldn't be any entries in namedPropertyRemovables for properties we know are + // referenced. + checkState(!referencedPropertyNames.contains(propertyName)); + for (Removable r : namedPropertyRemovables.removeAll(propertyName)) { + r.remove(compiler); + } + } + } + } + } + + /** + * Represents declarations in the standard for-loop initialization. + * + * e.g. the `let i = 0` part of `for (let i = 0; i < 10; ++i) {...}`. + * These must be handled differently from declaration statements because: + * + *

    + *
  1. + * For-loop declarations may declare more than one variable. + * The normalization doesn't break them up as it does for declaration statements. + *
    2. + *
  2. + * Removal must be handled differently. + *
    3. + *
  3. + * We don't currently preserve initializers with side effects here. + * Instead, we just consider such cases non-removable. + *
    4. + *
+ */ + private class VanillaForNameDeclaration extends Removable { + + private final Node nameNode; + + private VanillaForNameDeclaration(RemovableBuilder builder, Node nameNode) { + super(builder); + this.nameNode = nameNode; + } + + @Override + void removeInternal(AbstractCompiler compiler) { + Node declaration = checkNotNull(nameNode.getParent()); + compiler.reportChangeToEnclosingScope(declaration); + // NOTE: We don't need to preserve the initializer value, because we currently do not remove + // for-loop vars whose initializing values have side effects. + if (nameNode.getPrevious() == null && nameNode.getNext() == null) { + // only child, so we can remove the whole declaration + declaration.replaceWith(IR.empty().useSourceInfoFrom(declaration)); + } else { + declaration.removeChild(nameNode); + } + NodeUtil.markFunctionsDeleted(nameNode, compiler); + } + + } } diff --git a/test/com/google/javascript/jscomp/RemoveUnusedVarsTest.java b/test/com/google/javascript/jscomp/RemoveUnusedVarsTest.java index 9c27c033236..08cb523440e 100644 --- a/test/com/google/javascript/jscomp/RemoveUnusedVarsTest.java +++ b/test/com/google/javascript/jscomp/RemoveUnusedVarsTest.java @@ -22,8 +22,10 @@ public final class RemoveUnusedVarsTest extends CompilerTestCase { private boolean removeGlobal; private boolean preserveFunctionExpressionNames; + public RemoveUnusedVarsTest() { - super("function alert() {}"); + // Set up externs to be used in the test cases. + super("function alert() {} var externVar;"); } @Override @@ -50,6 +52,76 @@ public void process(Node externs, Node root) { }; } + public void testPrototypeIsAliased() { + // without alias C is removed + test("function C() {} C.prototype = {};", ""); + // with alias C must stay + testSame("function C() {} C.prototype = {}; var x = C.prototype; x;"); + testSame("var x; function C() {} C.prototype = {}; x = C.prototype; x;"); + testSame("var x; function C() {} x = C.prototype = {}; x;"); + } + + public void testWholePrototypeAssignment() { + test("function C() {} C.prototype = { constructor: C };", ""); + } + + public void testUsageBeforeDefinition() { + test("function f(a) { x[a] = 1; } var x; x = {}; f();", "function f() {} f();"); + } + + public void testReferencedPropertiesOnUnreferencedVar() { + test("var x = {}; x.a = 1; var y = {a: 2}; y.a;", "var y = {a: 2}; y.a;"); + } + + public void testPropertyValuesAddedAfterReferenceAreRemoved() { + // Make sure property assignments added after the first reference to the var are kept and their + // values traversed. + testSame("var x = 1; var y = {}; y; y.foo = x;"); + } + + public void testReferenceInObjectLiteral() { + testSame(LINE_JOINER.join( + "function f(a) {", + " return {a: a};", + "}", + "f(1);")); + } + + public void testSelfOverwrite() { + // Test for possible ConcurrentModificationException + // Reference to `a` triggers traversal of its function value, which recursively adds another + // definition of `a` to consider. + testSame("var a = function() { a = function() {}; }; a();"); + } + + public void testPropertyReferenceAddsPropertyReference() { + // Test for possible ConcurrentModificationException + // Reference to `a.foo()` triggers traversal of its function value, which recursively adds + // the `foo` property to another variable. + testSame("var a = {}; a.foo = function() { b.foo = 1; }; var b = {}; a.foo(); b.foo;"); + } + + public void testUnknownVarDestructuredAssign() { + // TODO(bradfordcsmith): Require that vars actually be defined in externs instead of assuming + // unknowns are externs. + testSame("({a:x} = {a:1});"); + testSame("({a:x = 1} = {});"); + testSame("({['a']:x} = {a:1});"); + testSame("({['a']:x = 1} = {});"); + testSame("[x] = [1];"); + testSame("[x = 1] = [];"); + testSame("[, ...x] = [1];"); + testSame("[, ...[x = 1]] = [1];"); + } + + public void testRemoveVarDeclaration1() { + test("var a = 0, b = a = 1", ""); + } + + public void testRemoveVarDeclaration2() { + test("var a;var b = 0, c = a = b = 1", ""); + } + public void testRemoveUnusedVarsFn0() { // Test with function expressions in another function call test("function A(){}" + @@ -180,6 +252,17 @@ public void testFunctionArgRemoval() { "var b=function(e,c,f,d){return c+d};b(1,2)"); } + public void testDollarSuperParameterNotRemoved() { + // supports special parameter expected by the prototype open-source library + testSame("function f($super) {} f();"); + } + + public void testFunctionArgRemovalWithLeadingUnderscore() { + // Coding convention usually prevents removal of variables beginning with a leading underscore, + // but that makes no sense for parameter names. + test("function f(__$jscomp$1) {__$jscomp$1 = 1;} f();", "function f() {} f();"); + } + public void testComputedPropertyDestructuring() { // Don't remove variables accessed by computed properties testSame("var {['a']:a, ['b']:b} = {a:1, b:2}; a; b;"); @@ -566,6 +649,14 @@ public void testUnusedAssign9() { testSame("function b(a) { a = 1; arguments=1; }; b(6)"); } + public void testES6ModuleExports() { + test("const X = 1; function f() {}", ""); + test("const X = 1; export function f() {}", "function f() {} export {f as f}"); + test("const X = 1; export class C {}", "class C {} export { C as C }"); + test("const X = 1; export default function f() {};", "export default function f() {}"); + test("const X = 1; export default class C {}", "export default class C {}"); + } + public void testUnusedPropAssign1() { test("var x = {}; x.foo = 3;", ""); } @@ -915,6 +1006,29 @@ public void testRemoveInheritedClass11() { "goog$inherits(b.foo, a)"); } + public void testRemoveInheritedClass12() { + // An inherits call must be a statement unto itself or the left side of a comma to be considered + // specially. These calls have no return values, so this restriction avoids false positives. + // It also simplifies removal logic. + testSame( + LINE_JOINER.join( + "function goog$inherits(){}", + "function a(){}", + "function b(){}", + "goog$inherits(b, a) + 1;")); + + // Although a human is unlikely to write code like this, some optimizations may end up + // converting an inherits call statement into the left side of a comma. We should still + // remove this case. + test( + LINE_JOINER.join( + "function goog$inherits(){}", + "function a(){}", + "function b(){}", + "(goog$inherits(b, a), 1);"), + "1"); + } + public void testReflectedMethods() { testSame( "/** @constructor */" + @@ -1094,12 +1208,16 @@ public void testArrowFunctions() { } public void testClasses() { + test("var C = class {};", ""); + test("class C {}", ""); test("class C {constructor(){} g() {}}", ""); testSame("class C {}; new C;"); + test("var C = class X {}; new C;", "var C = class {}; new C;"); + testSame( lines( "class C{", @@ -1144,6 +1262,18 @@ public void testClasses() { ); } + public void testReferencesInClasses() { + testSame( + LINE_JOINER.join( + "const A = 15;", + "const C = class {", + " constructor() {", + " this.a = A;", + " }", + "}", + "new C;")); + } + public void testRemoveGlobalClasses() { removeGlobal = false; testSame("class C{}"); @@ -1188,19 +1318,21 @@ public void testGenerators() { public void testForLet() { // Could be optimized so that unused lets in the for loop are removed - testSame("for(let x; ;){}"); + test("for(let x; ;){}", "for(;;){}"); - testSame("for(let x, y; ;) {x} "); + test("for(let x, y; ;) {x}", "for(let x; ;) {x}"); + test("for(let x, y; ;) {y}", "for(let y; ;) {y}"); - testSame("for(let x=0,y=0;;y++){}"); + test("for(let x=0,y=0;;y++){}", "for(let y=0;;y++){}"); } public void testForInLet() { - testSame("for(item in items){}"); + testSame("let item; for(item in items){}"); + testSame("for(let item in items){}"); } public void testForOf() { - testSame("for(item of items){}"); + testSame("let item; for(item of items){}"); testSame("for(var item of items){}"); @@ -1230,6 +1362,15 @@ public void testForOf() { ); } + public void testEnhancedForWithExternVar() { + testSame("for(externVar in items){}"); + testSame("for(externVar of items){}"); + } + + public void testExternVarNotRemovable() { + testSame("externVar = 5;"); + } + public void testTemplateStrings() { testSame( lines(