diff --git a/src/com/google/javascript/jscomp/RemoveUnusedCode.java b/src/com/google/javascript/jscomp/RemoveUnusedCode.java index 273085ec48f..4c9bf39b08d 100644 --- a/src/com/google/javascript/jscomp/RemoveUnusedCode.java +++ b/src/com/google/javascript/jscomp/RemoveUnusedCode.java @@ -16,6 +16,7 @@ package com.google.javascript.jscomp; +import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; @@ -28,11 +29,14 @@ import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Token; import com.google.javascript.rhino.jstype.JSType; +import com.google.javascript.rhino.jstype.JSTypeNative; +import com.google.javascript.rhino.jstype.ObjectType; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Deque; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -127,6 +131,14 @@ class RemoveUnusedCode implements CompilerPass { */ private final List allFunctionParamScopes = new ArrayList<>(); + /** + * Stores the names of all "leaf" properties that are polyfilled, to avoid unnecessary qualified + * name matching and searches for all the other properties. This includes global names such as + * "Promise" and "Map", static methods on global names such as "Array.from" and "Math.fround", and + * instance properties such as "String.prototype.repeat" and "Promise.prototype.finally". + */ + private final Multimap polyfills = HashMultimap.create(); + private final Es6SyntacticScopeCreator scopeCreator; private final boolean removeUnusedPrototypeProperties; @@ -134,6 +146,7 @@ class RemoveUnusedCode implements CompilerPass { private final boolean removeUnusedThisProperties; private final boolean removeUnusedStaticProperties; private final boolean removeUnusedObjectDefinePropertiesDefinitions; + private final boolean removeUnusedPolyfills; RemoveUnusedCode(Builder builder) { this.compiler = builder.compiler; @@ -147,6 +160,7 @@ class RemoveUnusedCode implements CompilerPass { this.removeUnusedStaticProperties = builder.removeUnusedStaticProperties; this.removeUnusedObjectDefinePropertiesDefinitions = builder.removeUnusedObjectDefinePropertiesDefinitions; + this.removeUnusedPolyfills = builder.removeUnusedPolyfills; this.scopeCreator = new Es6SyntacticScopeCreator(builder.compiler); // All Vars that are completely unremovable will share this VarInfo instance. @@ -165,6 +179,7 @@ public static class Builder { private boolean removeUnusedThisProperties = false; private boolean removeUnusedStaticProperties = false; private boolean removeUnusedObjectDefinePropertiesDefinitions = false; + private boolean removeUnusedPolyfills = false; Builder(AbstractCompiler compiler) { this.compiler = compiler; @@ -210,6 +225,11 @@ Builder removeUnusedObjectDefinePropertiesDefinitions(boolean value) { return this; } + Builder removeUnusedPolyfills(boolean value) { + this.removeUnusedPolyfills = value; + return this; + } + RemoveUnusedCode build() { return new RemoveUnusedCode(this); } @@ -249,7 +269,7 @@ private void traverseAndRemoveUnusedReferences(Node root) { continuation.apply(); } - removeUnreferencedVars(); + removeUnreferencedVarsAndPolyfills(); removeIndependentlyRemovableProperties(); for (Scope fparamScope : allFunctionParamScopes) { removeUnreferencedFunctionArgs(fparamScope); @@ -436,6 +456,12 @@ private void traverseGetProp(Node getProp, Scope scope) { Node propertyNameNode = objectNode.getNext(); String propertyName = propertyNameNode.getString(); + if (polyfills.containsKey(propertyName)) { + for (PolyfillInfo info : polyfills.get(propertyName)) { + info.considerPossibleReference(getProp); + } + } + if (NodeUtil.isExpressionResultUsed(getProp)) { // must record as reference to the property and continue traversal. markPropertyNameReferenced(propertyName); @@ -528,6 +554,12 @@ private void traverseCompoundAssign(Node compoundAssignNode, Scope scope) { } private VarInfo traverseNameNode(Node n, Scope scope) { + if (polyfills.containsKey(n.getString())) { + for (PolyfillInfo info : polyfills.get(n.getString())) { + info.considerPossibleReference(n); + } + } + return traverseVar(getVarForNameNode(n, scope)); } @@ -544,6 +576,13 @@ private void traverseCall(Node callNode, Scope scope) { } else if (NodeUtil.isObjectDefinePropertiesDefinition(callNode)) { // TODO(bradfordcsmith): Should also handle Object.create() and Object.defineProperty(). traverseObjectDefinePropertiesCall(callNode, scope); + } else if (removeUnusedPolyfills && isJscompPolyfill(callee)) { + Node firstArg = callee.getNext(); + String polyfillName = firstArg.getString(); + PolyfillInfo info = createPolyfillInfo(callNode, scope, polyfillName); + polyfills.put(info.key, info); + // Only traverse the callee (to mark it as used). The arguments may be traversed later. + traverseNode(callNode.getFirstChild(), scope); } else { Node parent = callNode.getParent(); String classVarName = null; @@ -589,6 +628,23 @@ private void traverseCall(Node callNode, Scope scope) { } } + /** Checks whether this is a recognizable call to $jscomp.polyfill. */ + private static boolean isJscompPolyfill(Node n) { + switch (n.getToken()) { + case NAME: + // Need to work correctly after CollapseProperties. + return n.getString().equals("$jscomp$polyfill") && n.getNext().isString(); + case GETPROP: + // Need to work correctly without CollapseProperties. + return n.getLastChild().getString().equals("polyfill") + && n.getFirstChild().isName() + && n.getFirstChild().getString().equals("$jscomp") + && n.getNext().isString(); + default: + return false; + } + } + /** Traverse `Object.defineProperties(someObject, propertyDefinitions);`. */ private void traverseObjectDefinePropertiesCall(Node callNode, Scope scope) { // First child is Object.defineProperties or some equivalent of it. @@ -1474,8 +1530,8 @@ private VarInfo getVarInfo(Var var) { * Removes any vars in the scope that were not referenced. Removes any assignments to those * variables as well. */ - private void removeUnreferencedVars() { - for (Entryentry : varInfoMap.entrySet()) { + private void removeUnreferencedVarsAndPolyfills() { + for (Entry entry : varInfoMap.entrySet()) { Var var = entry.getKey(); VarInfo varInfo = entry.getValue(); @@ -1512,6 +1568,15 @@ private void removeUnreferencedVars() { toRemove); } } + + Iterator iter = polyfills.values().iterator(); + while (iter.hasNext()) { + PolyfillInfo polyfill = iter.next(); + if (polyfill.isRemovable) { + polyfill.removable.remove(compiler); + iter.remove(); + } + } } /** @@ -1683,6 +1748,10 @@ DestructuringAssign buildDestructuringAssign(Node targetNode) { return new DestructuringAssign(this, targetNode); } + Polyfill buildPolyfill(Node polyfillNode) { + return new Polyfill(this, polyfillNode); + } + ClassDeclaration buildClassDeclaration(Node classNode) { return new ClassDeclaration(this, classNode); } @@ -1996,6 +2065,26 @@ && canRemoveParameters(targetParent)) { } + /** A call to $jscomp.polyfill that can be removed if it is no longer referenced. */ + private class Polyfill extends Removable { + final Node polyfillNode; + + Polyfill(RemovableBuilder builder, Node polyfillNode) { + super(builder); + this.polyfillNode = polyfillNode; + } + + @Override + public void removeInternal(AbstractCompiler compiler) { + NodeUtil.deleteNode(polyfillNode, compiler); + } + + @Override + public String toString() { + return "Polyfill:" + polyfillNode; + } + } + private class ClassDeclaration extends Removable { final Node classDeclarationNode; @@ -2494,24 +2583,220 @@ void removeAllRemovables() { } + /** + * Makes a new PolyfillInfo, including the correct Removable. Parses the name to determine whether + * this is a global, static, or prototype polyfill. + */ + private PolyfillInfo createPolyfillInfo(Node call, Scope scope, String name) { + checkState(scope.isGlobal()); + checkState(call.getParent().isExprResult()); + // Make the removable and polyfill info. Add continuations for all arguments. + RemovableBuilder builder = new RemovableBuilder(); + for (Node n = call.getFirstChild().getNext(); n != null; n = n.getNext()) { + builder.addContinuation(new Continuation(n, scope)); + } + Polyfill removable = builder.buildPolyfill(call.getParent()); + int lastDot = name.lastIndexOf("."); + if (lastDot < 0) { + return new GlobalPolyfillInfo(removable, name); + } + String owner = name.substring(0, lastDot); + String prop = name.substring(lastDot + 1); + boolean typed = call.getJSType() != null; + if (owner.endsWith(DOT_PROTOTYPE)) { + owner = owner.substring(0, owner.length() - DOT_PROTOTYPE.length()); + return new PrototypePropertyPolyfillInfo( + removable, prop, typed ? compiler.getTypeRegistry().getType(scope, owner) : null); + } + ObjectType ownerInstanceType = + typed ? ObjectType.cast(compiler.getTypeRegistry().getType(scope, owner)) : null; + JSType ownerCtorType = ownerInstanceType != null ? ownerInstanceType.getConstructor() : null; + return new StaticPropertyPolyfillInfo(removable, prop, ownerCtorType, owner); + } + + private static final String DOT_PROTOTYPE = ".prototype"; + + /** + * Stores information about definitions and usages of polyfills. + * + *

The polyfill removal strategy is as follows. First, look for all the polyfill definitions, + * whose names are stores as strings passed as the first argument to {@code $jscomp.polyfill}. + * Each definition falls into one of three categories: (1) global names, such as {@code Map} or + * {@code Promise}; (2) static properties, such as {@code Array.from} or {@code Reflect.get}, + * which must always have exactly two name components; or (3) prototype properties, such as {@code + * String.prototype.repeat} or {@code Promise.prorotype.finally}, which must always have exactly + * three name components. The definition can be removed once it is found that there is an + * unguarded reference to it. References guarded by existence or truthiness checks (such as + * {@code if (Promise) return Promise.resolve('ok');} do not prevent removal of the polyfill + * definition. + * + *

Determining whether a node is a reference (guarded or not) depends on the type of polyfill. + * When type information is available, the type of the expected owner (i.e. the global object for + * global polyfills, the namespace or class for static polyfills, or an instance of the owning + * class (or its implicit prototype) for prototype polyfills) is used exclusively to determine + * this with very good accuracy. Types are considered to match if a direct cast would be allowed + * without a warning (i.e. some element of the union is a direct subtype or supertype). + * + *

When type information is not available (or is too loose) then we fall back on a heuristic: + * + *

    + *
  • globals are referenced by any same-named NAME node or any GETPROP node whose last child + * has the same string (this allows matching {@code goog.global.Map}, but will also match + * {@code MyOuter.Map}). + *
  • static properties are referenced by any GETPROP node whose last child is the same as the + * polyfill's property name and whose owner references the polyfill owner per the above + * rule. + *
  • prototype properties are referenced by any GETPROP node whose last child is the same as + * the polyfill's property name, regardless of its owner. + *
+ * + *

Note that this results in both false positives and false negatives in untyped code: we may + * remove polyfills that are actually used (e.g. if {@code Array.from} is accessed via a subclass + * as {@code SubArray.from} or in a subclass' static method as {@code this.from}) and we may + * retain polyfills that are not used (e.g. if a user-defined nested class shares the same name as + * a global builtin, as in {@code Foo.Map}). For greater consistency we may shift this balance in + * the future to eliminate the possibility of incorrect removals, at the cost of more incorrect + * retentions. + */ + private abstract static class PolyfillInfo { + /** The {@link Polyfill} instance corresponding to the polyfill's definition. */ + final Polyfill removable; + /** The rightmost component of the polyfill's qualified name (does not contain a dot). */ + final String key; + /** Whether the polyfill is unreferenced and this can be removed safely. */ + boolean isRemovable = true; + + PolyfillInfo(Polyfill removable, String key) { + this.removable = removable; + this.key = key; + } + + /** + * Accepts a NAME or GETPROP node whose (property) string matches {@code key} and checks whether + * the node should be considered as a possible reference to this polyfill. If so, mark the + * polyfill as referenced and therefore not removable. + */ + void considerPossibleReference(Node n) { + if (isRemovable) { + considerPossibleReferenceInternal(n); + if (!isRemovable) { + removable.applyContinuations(); + } + } + } + + /** Template method to check the node. */ + abstract void considerPossibleReferenceInternal(Node n); + } + + private class GlobalPolyfillInfo extends PolyfillInfo { + + GlobalPolyfillInfo(Polyfill removable, String name) { + super(removable, name); + } + + @Override + void considerPossibleReferenceInternal(Node possiblyReferencingNode) { + if (possiblyReferencingNode.isName()) { + // A matching NAME node must be a reference (there's no need to check that the referenced + // Var is global, since local variables have all been renamed by normalization). + isRemovable = false; + } else if (possiblyReferencingNode.isGetProp()) { + // Does the owner have type information? If so then this is a reference only if the owner + // could be the global 'this' type. Absent type information, just always assume it might be. + JSType ownerType = possiblyReferencingNode.getFirstChild().getJSType(); + if (ownerType == null + || ownerType.canCastTo( + compiler.getTypeRegistry().getNativeType(JSTypeNative.GLOBAL_THIS))) { + isRemovable = false; + } + } + } + } + + private class StaticPropertyPolyfillInfo extends PolyfillInfo { + // Type of the owner, if available. + @Nullable final JSType polyfillOwnerType; + // Name of the owning type, used only when there's no type information. + final String polyfillOwnerName; + + StaticPropertyPolyfillInfo( + Polyfill removable, String key, @Nullable JSType owner, String ownerName) { + super(removable, key); + this.polyfillOwnerName = ownerName; + this.polyfillOwnerType = owner; + } + + @Override + void considerPossibleReferenceInternal(Node possiblyReferencingNode) { + if (!possiblyReferencingNode.isGetProp()) { + return; + } + Node nodeOwner = possiblyReferencingNode.getFirstChild(); + JSType nodeOwnerType = nodeOwner.getJSType(); + if (polyfillOwnerType != null && nodeOwnerType != null) { + if (typesAreRelated(polyfillOwnerType, nodeOwnerType)) { + isRemovable = false; + } + return; + } + // no type information: check polyfillOwnerName instead. + if ((nodeOwner.isGetProp() && nodeOwner.getLastChild().getString().equals(polyfillOwnerName)) + || (nodeOwner.isName() && nodeOwner.getString().equals(polyfillOwnerName))) { + isRemovable = false; + } + } + } + + private class PrototypePropertyPolyfillInfo extends PolyfillInfo { + @Nullable final JSType polyfillOwnerType; + + PrototypePropertyPolyfillInfo(Polyfill removable, String key, @Nullable JSType owner) { + super(removable, key); + this.polyfillOwnerType = owner; + } + + @Override + void considerPossibleReferenceInternal(Node possiblyReferencingNode) { + if (!possiblyReferencingNode.isGetProp()) { + return; + } + JSType nodeOwnerType = possiblyReferencingNode.getFirstChild().getJSType(); + if (polyfillOwnerType != null && nodeOwnerType != null) { + if (typesAreRelated(polyfillOwnerType, nodeOwnerType)) { + isRemovable = false; + } + return; + } + // Fallback for no type information: prototype properties are simply not removable. + isRemovable = false; + } + } + + private static boolean typesAreRelated(JSType expected, JSType actual) { + if (actual.isConstructor() && expected.isConstructor()) { + // All constructors can cast to one another, even if they're incompatible. + // If both types are constructors then compare the instance types instead. + actual = actual.toMaybeFunctionType().getInstanceType(); + expected = expected.toMaybeFunctionType().getInstanceType(); + } + actual = firstNonNull(actual.autobox(), actual); + + return actual.canCastTo(expected); + } + /** * 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: + *

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. + *
  4. For-loop declarations may declare more than one variable. The normalization doesn't break + * them up as it does for declaration statements. + *
  5. Removal must be handled differently. + *
  6. We don't currently preserve initializers with side effects here. Instead, we just + * consider such cases non-removable. *
*/ private class VanillaForNameDeclaration extends Removable { diff --git a/test/com/google/javascript/jscomp/RemoveUnusedCodeTest.java b/test/com/google/javascript/jscomp/RemoveUnusedCodeTest.java index e23b1a55635..de606ca3fbc 100644 --- a/test/com/google/javascript/jscomp/RemoveUnusedCodeTest.java +++ b/test/com/google/javascript/jscomp/RemoveUnusedCodeTest.java @@ -34,16 +34,32 @@ public RemoveUnusedCodeTest() { lines( "var undefined;", "var goog = {};", + "/** @const {!Global} */ goog.global;", "goog.reflect = {};", "goog.reflect.object = function(obj, propertiesObj) {};", "function goog$inherits(subClass, superClass) {}", - "function goog$mixin(dstPrototype, srcPrototype){}", + "function goog$mixin(dstPrototype, srcPrototype) {}", "function alert() {}", "function use() {}", "function externFunction() {}", "var externVar;", - "var window;")); - } + "var window;", + "var console = {};", + "console.log = function(var_args) {};", + "/** @constructor @return {!Array} */ function Array(/** ...* */ var_args) {}", + "/** @constructor @return {string} */ function String(/** *= */ opt_arg) {}", + "/** @constructor */ function Map() {}", + "/** @constructor */ function Set() {}", + "/** @constructor */ function WeakMap() {}", + "")); + } + + private static final String JSCOMP_POLYFILL = + lines( + "var $jscomp = {};", + "$jscomp.polyfill = function(", + " /** string */ name, /** Function */ func, /** string */ from, /** string */ to) {};", + ""); @Override protected int getNumRepetitions() { @@ -68,6 +84,7 @@ public void process(Node externs, Node root) { new RemoveUnusedCode.Builder(compiler) .removeLocalVars(true) .removeGlobals(removeGlobal) + .removeUnusedPolyfills(true) .preserveFunctionExpressionNames(preserveFunctionExpressionNames) .build() .process(externs, root); @@ -1906,5 +1923,805 @@ public void testRemoveUnusedVarsDeclaredInDestructuring1() { // Same as above case without the destructuring declaration test("var a, b = alert();", "alert();"); } -} + @Test + public void testRemoveUnusedPolyfills_global_untyped() { + Externs externs = + externs( + new TestExternsBuilder() + .addConsole() + .addExtra(JSCOMP_POLYFILL, "/** @constructor */ function Map() {}") + .build()); + + // Unused polyfill is removed. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Map', function() {}, 'es6', 'es3');", // + "console.log();")), + expected("console.log();")); + + // Used polyfill is not removed. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('Map', function() {}, 'es6', 'es3');", // + "console.log(new Map());"))); + + // Local names shadowing global polyfills are not themselves polyfill references. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Map', function() {}, 'es6', 'es3');", + "console.log(function(Map) {", + " console.log(new Map());", + "});")), + expected( + lines( + "console.log(function(Map) {", // + " console.log(new Map());", + "});"))); + } + + @Test + public void testRemoveUnusedPolyfills_global_typed() { + Externs externs = + externs( + new TestExternsBuilder() + .addConsole() + .addExtra(JSCOMP_POLYFILL, "/** @constructor */ function Map() {}", "") + .build()); + enableTypeCheck(); + + // Unused polyfill is removed. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Map', function() {}, 'es6', 'es3');", // + "console.log();")), + expected("console.log();")); + + // Used polyfill is not removed. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('Map', function() {}, 'es6', 'es3');", // + "console.log(new Map());"))); + + // Local names shadowing global polyfills are not themselves polyfill references. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Map', function() {}, 'es6', 'es3');", + "console.log(function(Map) {", + " console.log(new Map());", + "});")), + expected( + lines( + "console.log(function(Map) {", // + " console.log(new Map());", + "});"))); + } + + @Test + public void testRemoveUnusedPolyfills_propertyOfGlobalObject_untyped() { + Externs externs = + externs( + new TestExternsBuilder() + .addConsole() + .addExtra( + JSCOMP_POLYFILL, + "/** @constructor */ function Map() {}", + "/** @const */ var goog = {};") + .build()); + + // Global polyfills may be accessed as properties on the global object. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('Map', function() {}, 'es6', 'es3');", // + "console.log(new goog.global.Map());"))); + + // NOTE: Without type information we don't see that x is not the global object. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('Map', function() {}, 'es6', 'es3');", // + "console.log(new goog.structs.Map());"))); + + // Global polyfills may be accessed as properties on the global object. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('Map', function() {}, 'es6', 'es3');", + "var x = {Map: /** @constructor */ function() {}};", + "console.log(new x.Map());"))); + } + + @Test + public void testRemoveUnusedPolyfills_propertyOfGlobalObject_typed() { + Externs externs = + externs( + new TestExternsBuilder() + .addConsole() + .addExtra( + JSCOMP_POLYFILL, + "/** @constructor */ function Map() {}", + "/** @type {!Global} */ var someGlobal;", + "/** @const */ var notGlobal = {};", + "/** @constructor */ notGlobal.Map = function() {};", + "") + .build()); + enableTypeCheck(); + + // Global polyfills may be accessed as properties on the global object. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('Map', function() {}, 'es6', 'es3');", + "console.log(new someGlobal.Map());"))); + + // With proper type information we can tell that notGlobal.Map is not Map. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Map', function() {}, 'es6', 'es3');", + "console.log(new notGlobal.Map());")), + expected("console.log(new notGlobal.Map());")); + + // Global polyfills may be accessed as properties on the global object. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Map', function() {}, 'es6', 'es3');", + "var alsoNotGlobal = {Map: /** @constructor */ function() {}};", + "console.log(new alsoNotGlobal.Map());")), + expected( + lines( + "var alsoNotGlobal = {Map: /** @constructor */ function() {}};", // + "console.log(new alsoNotGlobal.Map());"))); + } + + @Test + public void testRemoveUnusedPolyfills_staticProperty_untyped() { + Externs externs = + externs( + new TestExternsBuilder() + .addConsole() + .addArray() + .addExtra( + JSCOMP_POLYFILL, + "/** @constructor */ function Map() {}", + "/** @const */ var goog = {};") + .build()); + + // Used polyfill is retained. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", // + "console.log(Array.from());"))); + + // Used polyfill is retained, even if accessed as a property of global. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", + "console.log(goog.global.Array.from());"))); + + // Unused polyfill is removed if there's sufficient type information. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", + "class NotArray { static from() {} }", + "console.log(NotArray.from());")), + expected( + lines( + "class NotArray { static from() {} }", // + "console.log(NotArray.from());"))); + + // Without type information, we can't correctly remove this polyfill. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", + "var x = {Array: {from: function() {}}};", + "console.log(x.Array.from());"))); + + // Without type information, we don't recognize the aliased call. + // https://github.com/google/closure-compiler/issues/3171 + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", + "/** @const */ var MyArray = Array;", + "console.log(MyArray.from());")), + expected( + lines( + "/** @const */ var MyArray = Array;", // + "console.log(MyArray.from());"))); + + // Without type information, we don't recognize the subclass call. + // https://github.com/google/closure-compiler/issues/3171 + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", + "class SubArray extends Array {}", + "console.log(SubArray.from());")), + expected( + lines( + "class SubArray extends Array {}", // + "console.log(SubArray.from());"))); + + // Heurisitic is still able to remove Set.from while retaining Array.from. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", + "$jscomp.polyfill('Set.from', function() {}, 'es6', 'es3');", + "console.log(Array.from());")), + expected( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", // + "console.log(Array.from());"))); + } + + @Test + public void testRemoveUnusedPolyfills_staticProperty_typed() { + Externs externs = + externs( + new TestExternsBuilder() + .addArray() + .addConsole() + .addExtra( + JSCOMP_POLYFILL, + "Array.from = function() {};", + "/** @constructor */ function Set() {}", + // NOTE: this is not a real API but it allows testing that we can tell it apart. + "Set.from = function() {};", + "") + .build()); + enableTypeCheck(); + + // Used polyfill is retained. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", // + "console.log(Array.from());"))); + + // Used polyfill is retained, even if accessed via a global object. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", + "console.log($jscomp.global.Array.from());"))); + + // Used polyfill is retained, even if accessed via a global object. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", + "class NotArray { static from() {} }", + "console.log(NotArray.from());")), + expected( + lines( + "class NotArray { static from() {} }", // + "console.log(NotArray.from());"))); + + // Unused polyfill is deleted since compiler knows `x` is not the global object. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", + "var x = {Array: {from: function() {}}};", + "console.log(x.Array.from());")), + expected( + lines( + "var x = {Array: {from: function() {}}};", // + "console.log(x.Array.from());"))); + + // Used polyfill via aliased owner: retains definition. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", + "/** @const */ var MyArray = Array;", + "console.log(MyArray.from());"))); + + // Used polyfill via subclass: retains definition. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", + "class SubArray extends Array {}", + "console.log(SubArray.from());"))); + + // Distinguish static polyfills on different types: remove the unused one. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", + "$jscomp.polyfill('Set.from', function() {}, 'es6', 'es3');", + "console.log(Array.from());")), + expected( + lines( + "$jscomp.polyfill('Array.from', function() {}, 'es6', 'es3');", // + "console.log(Array.from());"))); + } + + @Test + public void testRemoveUnusedPolyfills_prototypeProperty_untyped() { + Externs externs = + externs( + new TestExternsBuilder() + .addArray() + .addConsole() + .addString() + .addExtra(JSCOMP_POLYFILL, "function externFunction() {}", "function alert() {}") + .build()); + + // Unused polyfill is removed. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.repeat', function() {}, 'es6', 'es3');", + "console.log();")), + expected("console.log();")); + + // Used polyfill is retained. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.repeat', function() {}, 'es6', 'es3');", + "''.repeat();"))); + + // Used polyfill (directly via String.prototype) is retained. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.repeat', function() {}, 'es6', 'es3');", + "String.prototype.repeat();"))); + + // Used polyfill (directly String.prototype and Function.prototype.call) is retained. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.repeat', function() {}, 'es6', 'es3');", + "String.prototype.repeat.call('');"))); + + // Unknown type is not removed. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.repeat', function() {}, 'es6', 'es3');", + "var x = externFunction();", + "x.repeat();"))); + + // Without type information, cannot remove the polyfill. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.repeat', function() {}, 'es6', 'es3');", + "String.repeat();"))); + + // Without type information, cannot remove the polyfill. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.repeat', function() {}, 'es6', 'es3');", + "var /** number */ x = 42;", + "x.repeat();"))); + + // Multiple same-name methods + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.includes', function() {}, 'es6', 'es3');", + "$jscomp.polyfill('Array.prototype.includes', function() {}, 'es6', 'es3');", + "[].includes(5);"))); + } + + @Test + public void testRemoveUnusedPolyfills_prototypeProperty_typed() { + Externs externs = + externs( + new TestExternsBuilder() + .addFunction() + .addString() + .addArray() + .addConsole() + .addExtra( + JSCOMP_POLYFILL, + "/** @this {string} */", + "String.prototype.repeat = function() {}", + "String.prototype.includes = function(arg) {}", + "Array.prototype.includes = function(arg) {}", + "function externFunction() {}") + .build()); + enableTypeCheck(); + + // Unused polyfill is removed. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.repeat', function() {}, 'es6', 'es3');", + "console.log();")), + expected("console.log();")); + + // Used polyfill is retained. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.repeat', function() {}, 'es6', 'es3');", + "''.repeat();"))); + + // Used polyfill (via String.prototype) is retained. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.repeat', function() {}, 'es6', 'es3');", + "String.prototype.repeat();"))); + + // Used polyfill (via String.prototype and Function.prototype.call) is retained. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.repeat', function() {}, 'es6', 'es3');", + "String.prototype.repeat.call('');"))); + + // Unknown type is not removed. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.repeat', function() {}, 'es6', 'es3');", + "var x = externFunction();", + "x.repeat();"))); + + // Calling a prototype property like a static allows removing the prototype. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.repeat', function() {}, 'es6', 'es3');", + "String.repeat();")), + expected("String.repeat();"), + warning(TypeCheck.INEXISTENT_PROPERTY)); + + // Correctly discern between string and array. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.includes', function() {}, 'es6', 'es3');", + "var x = [];", + "x.includes(1);")), + expected( + lines( + "var x = [];", // + "x.includes(1);"))); + + // Union type prevents removal. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.includes', function() {}, 'es6', 'es3');", + "var /** string|Array */ x = [];", + "x.includes(1);"))); + + // Multiple same-name methods removes the right one. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.includes', function() {}, 'es6', 'es3');", + "$jscomp.polyfill('Array.prototype.includes', function() {}, 'es6', 'es3');", + "'x'.includes(5);")), + expected( + lines( + "$jscomp.polyfill('String.prototype.includes', function() {}, 'es6', 'es3');", + "'x'.includes(5);"))); + + // Multiple same-name methods removes the right one. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('String.prototype.includes', function() {}, 'es6', 'es3');", + "$jscomp.polyfill('Array.prototype.includes', function() {}, 'es6', 'es3');", + "Array.prototype.includes.call('x', 1);")), + expected( + lines( + "$jscomp.polyfill('Array.prototype.includes', function() {}, 'es6', 'es3');", + "Array.prototype.includes.call('x', 1);")), + warning(TypeValidator.TYPE_MISMATCH_WARNING)); + } + + @Test + public void testRemoveUnusedPolyfills_globalWithPrototypePolyfill_untyped() { + Externs externs = + externs( + new TestExternsBuilder().addPromise().addConsole().addExtra(JSCOMP_POLYFILL).build()); + + // Both the base polyfill and the extra method are removed when unused. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Promise', function() {}, 'es6', 'es3');", + "$jscomp.polyfill('Promise.prototype.finally', function() {}, 'es8', 'es3');", + "console.log();")), + expected("console.log();")); + + // The extra method polyfill is removed if not used, even when the base is retained. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Promise', function() {}, 'es6', 'es3');", + "$jscomp.polyfill('Promise.prototype.finally', function() {}, 'es8', 'es3');", + "console.log(Promise.resolve());")), + expected( + lines( + "$jscomp.polyfill('Promise', function() {}, 'es6', 'es3');", + "console.log(Promise.resolve());"))); + + // Can't remove finally without type information + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Promise', function() {}, 'es6', 'es3');", + "$jscomp.polyfill('Promise.prototype.finally', function() {}, 'es8', 'es3');", + "const p = {finally() {}};", + "p.finally();")), + expected( + lines( + "$jscomp.polyfill('Promise.prototype.finally', function() {}, 'es8', 'es3');", + "const p = {finally() {}};", + "p.finally();"))); + + // Retain both the base and the extra method when both are used. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('Promise', function() {}, 'es6', 'es3');", + "$jscomp.polyfill('Promise.prototype.finally', function() {}, 'es8', 'es3');", + "console.log(Promise.resolve().finally(() => {}));"))); + + // The base polyfill is removed and the extra method retained if the constructor never shows up + // anywhere in the source code. NOTE: this is probably the wrong thing to do. This situation + // should only be possible if async function transpilation happens *after* RemoveUnusedCode + // (since we have an async function). The fact that we inserted the Promise polyfill in the + // first case indicates the output language is < ES6. When that later transpilation occurs, we + // will end up adding uses of the Promise constructor. We need to keep this in mind when moving + // transpilation after optimizations. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Promise', function() {}, 'es6', 'es3');", + "$jscomp.polyfill('Promise.prototype.finally', function() {}, 'es8', 'es3');", + "async function f() {}", + "f().finally(() => {});")), + expected( + lines( + "$jscomp.polyfill('Promise.prototype.finally', function() {}, 'es8', 'es3');", + "async function f() {}", + "f().finally(() => {});"))); + } + + @Test + public void testRemoveUnusedPolyfills_globalWithPrototypePolyfill_typed() { + Externs externs = + externs( + new TestExternsBuilder().addPromise().addConsole().addExtra(JSCOMP_POLYFILL).build()); + enableTypeCheck(); + + // Both the base polyfill and the extra method are removed when unused. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Promise', function() {}, 'es6', 'es3');", + "$jscomp.polyfill('Promise.prototype.finally', function() {}, 'es8', 'es3');", + "console.log();")), + expected("console.log();")); + + // The extra method polyfill is removed if not used, even when the base is retained. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Promise', function() {}, 'es6', 'es3');", + "$jscomp.polyfill('Promise.prototype.finally', function() {}, 'es8', 'es3');", + "console.log(Promise.resolve());")), + expected( + lines( + "$jscomp.polyfill('Promise', function() {}, 'es6', 'es3');", + "console.log(Promise.resolve());"))); + + // Calls a different finally so both polyfills can be removed. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Promise', function() {}, 'es6', 'es3');", + "$jscomp.polyfill('Promise.prototype.finally', function() {}, 'es8', 'es3');", + "const p = {finally() {}};", + "p.finally();")), + expected(lines("const p = {finally() {}};", "p.finally();"))); + + // Retain both the base and the extra method when both are used. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('Promise', function() {}, 'es6', 'es3');", + "$jscomp.polyfill('Promise.prototype.finally', function() {}, 'es8', 'es3');", + "console.log(Promise.resolve().finally(() => {}));"))); + + // The base polyfill is removed and the extra method retained if the constructor never shows up + // anywhere in the source code. NOTE: this is probably the wrong thing to do. This situation + // should only be possible if async function transpilation happens *after* RemoveUnusedCode + // (since we have an async function). The fact that we inserted the Promise polyfill in the + // first case indicates the output language is < ES6. When that later transpilation occurs, we + // will end up adding uses of the Promise constructor. We need to keep this in mind when moving + // transpilation after optimizations. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Promise', function() {}, 'es6', 'es3');", + "$jscomp.polyfill('Promise.prototype.finally', function() {}, 'es8', 'es3');", + "async function f() {}", + "f().finally(() => {});")), + expected( + lines( + "$jscomp.polyfill('Promise.prototype.finally', function() {}, 'es8', 'es3');", + "async function f() {}", + "f().finally(() => {});"))); + } + + @Test + public void testRemoveUnusedPolyfills_chained() { + Externs externs = + externs( + new TestExternsBuilder() + .addConsole() + .addExtra( + JSCOMP_POLYFILL, + "/** @constructor */ function Map() {}", + "/** @constructor */ function Set() {}", + "/** @constructor */ function WeakMap() {}") + .build()); + + // Removes polyfills that are only referenced in other (removed) polyfills' definitions. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('WeakMap', function() { console.log(); }, 'es6', 'es3');", + "$jscomp.polyfill('Map', function() { new WeakMap(); }, 'es6', 'es3');", + "$jscomp.polyfill('Set', function() { new Map(); }, 'es6', 'es3');", + "function unused() { new Set(); }", + "console.log();")), + expected("console.log();")); + + // Chains can be partially removed if just an outer-most symbol is unreferenced. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('WeakMap', function() { console.log(); }, 'es6', 'es3');", + "$jscomp.polyfill('Map', function() { new WeakMap(); }, 'es6', 'es3');", + "$jscomp.polyfill('Set', function() { new Map(); }, 'es6', 'es3');", + "console.log(new Map());")), + expected( + lines( + "$jscomp.polyfill('WeakMap', function() { console.log(); }, 'es6', 'es3');", + "$jscomp.polyfill('Map', function() { new WeakMap(); }, 'es6', 'es3');", + "console.log(new Map());"))); + + // Only requires a single reference to the outermost symbol to retain the whole chain. + testSame( + externs, + srcs( + lines( + "$jscomp.polyfill('WeakMap', function() { console.log(); }, 'es6', 'es3');", + "$jscomp.polyfill('Map', function() { new WeakMap(); }, 'es6', 'es3');", + "$jscomp.polyfill('Set', function() { new Map(); }, 'es6', 'es3');", + "console.log(new Set())"))); + } + + @Test + public void testRemoveUnusedPolyfills_continued() { + Externs externs = + externs( + new TestExternsBuilder() + .addConsole() + .addExtra(JSCOMP_POLYFILL, "/** @constructor */ function Map() {}") + .build()); + + // Ensure that continuations occur so that retained polyfill definitions are still optimized. + test( + externs, + srcs( + lines( + "$jscomp.polyfill('Map', function() { var x; }, 'es6', 'es3');", + "console.log(new Map());")), + expected( + lines( + "$jscomp.polyfill('Map', function() {}, 'es6', 'es3');", // + "console.log(new Map());"))); + } + + @Test + public void testRemoveUnusedPolyfills_collapsedPolyfillFunction() { + Externs externs = + externs( + new TestExternsBuilder() + .addConsole() + .addExtra("function $jscomp$polyfill() {}", "/** @constructor */ function Map() {}") + .build()); + + // The pass should also work after CollapseProperties. + test( + externs, + srcs( + lines( + "$jscomp$polyfill('Map', function() {}, 'es6', 'es3');", // + "console.log();")), + expected("console.log();")); + + testSame( + externs, + srcs( + lines( + "$jscomp$polyfill('Map', function() {}, 'es6', 'es3');", // + "console.log(new Map());"))); + } +} diff --git a/test/com/google/javascript/jscomp/TestExternsBuilder.java b/test/com/google/javascript/jscomp/TestExternsBuilder.java index c2bb45e4047..31a3685d3cc 100644 --- a/test/com/google/javascript/jscomp/TestExternsBuilder.java +++ b/test/com/google/javascript/jscomp/TestExternsBuilder.java @@ -18,6 +18,7 @@ import com.google.common.base.Joiner; import java.util.ArrayList; +import java.util.Collections; import java.util.List; /** @@ -430,6 +431,7 @@ public class TestExternsBuilder { private boolean includeArgumentsExterns = false; private boolean includeConsoleExterns = false; private boolean includePromiseExterns = false; + private final List extraExterns = new ArrayList<>(); public TestExternsBuilder addIterable() { includeIterableExterns = true; @@ -478,6 +480,11 @@ public TestExternsBuilder addConsole() { return this; } + public TestExternsBuilder addExtra(String... lines) { + Collections.addAll(extraExterns, lines); + return this; + } + public String build() { List externSections = new ArrayList<>(); if (includeIterableExterns) { @@ -504,6 +511,7 @@ public String build() { if (includePromiseExterns) { externSections.add(PROMISE_EXTERNS); } + externSections.addAll(extraExterns); return LINE_JOINER.join(externSections); }