diff --git a/src/com/google/javascript/jscomp/RemoveUnusedVars.java b/src/com/google/javascript/jscomp/RemoveUnusedVars.java index dfa621a4bcd..e933a1c5e82 100644 --- a/src/com/google/javascript/jscomp/RemoveUnusedVars.java +++ b/src/com/google/javascript/jscomp/RemoveUnusedVars.java @@ -384,13 +384,22 @@ private void traverseRest(Node restNode, Scope scope) { } private void traverseObjectLiteral(Node objectLiteral, Scope scope) { + checkArgument(objectLiteral.isObjectLit(), objectLiteral); + // Is this an object literal that is assigned directly to a 'prototype' property? + if (isAssignmentToPrototype(objectLiteral.getParent())) { + traversePrototypeLiteral(objectLiteral, scope); + } else { + traverseNonPrototypeObjectLiteral(objectLiteral, scope); + } + } + + private void traverseNonPrototypeObjectLiteral(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 { @@ -399,6 +408,31 @@ private void traverseObjectLiteral(Node objectLiteral, Scope scope) { } } + private void traversePrototypeLiteral(Node objectLiteral, Scope scope) { + for (Node propertyNode = objectLiteral.getFirstChild(); + propertyNode != null; + propertyNode = propertyNode.getNext()) { + if (propertyNode.isComputedProp() || propertyNode.isQuotedString()) { + traverseChildren(propertyNode, scope); + } else { + // If we've come this far, we already know we're keeping the prototype literal itself, + // but we may be able to remove unreferenced properties in it. + considerForIndependentRemoval( + new RemovableBuilder() + .addContinuation(new Continuation(propertyNode.getOnlyChild(), scope)) + .buildClassOrPrototypeNamedProperty(propertyNode)); + } + } + } + + private boolean isAssignmentToPrototype(Node n) { + return n.isAssign() && isPrototypeGetProp(n.getFirstChild()); + } + + private boolean isPrototypeGetProp(Node n) { + return n.isGetProp() && n.getLastChild().getString().equals("prototype"); + } + private void traverseCatch(Node catchNode, Scope scope) { Node exceptionNameNode = catchNode.getFirstChild(); Node block = exceptionNameNode.getNext(); @@ -804,7 +838,7 @@ private void traverseClassMembers(Node node, Scope scope) { considerForIndependentRemoval( new RemovableBuilder() .addContinuation(new Continuation(member, scope)) - .buildMethodDefinition(member)); + .buildClassOrPrototypeNamedProperty(member)); } else { checkState(member.isComputedProp()); traverseChildren(member, scope); @@ -1231,12 +1265,12 @@ boolean isPrototypeObjectNamedPropertyAssignment() { return isPrototypeObjectPropertyAssignment && isNamedPropertyAssignment(); } - boolean isMethodDeclaration() { + boolean isClassOrPrototypeNamedProperty() { return false; } boolean isIndependentlyRemovableNamedProperty() { - return isPrototypeObjectNamedPropertyAssignment() || isMethodDeclaration(); + return isPrototypeObjectNamedPropertyAssignment() || isClassOrPrototypeNamedProperty(); } } @@ -1275,10 +1309,14 @@ NamedClassExpression buildNamedClassExpression(Node classNode) { return new NamedClassExpression(this, classNode); } - MethodDefinition buildMethodDefinition(Node methodNode) { - checkArgument(methodNode.isMemberFunctionDef() || NodeUtil.isGetOrSetKey(methodNode)); - this.propertyName = methodNode.getString(); - return new MethodDefinition(this, methodNode); + ClassOrPrototypeNamedProperty buildClassOrPrototypeNamedProperty(Node propertyNode) { + checkArgument( + propertyNode.isMemberFunctionDef() + || NodeUtil.isGetOrSetKey(propertyNode) + || (propertyNode.isStringKey() && !propertyNode.isQuotedString()), + propertyNode); + this.propertyName = propertyNode.getString(); + return new ClassOrPrototypeNamedProperty(this, propertyNode); } FunctionDeclaration buildFunctionDeclaration(Node functionNode) { @@ -1443,22 +1481,22 @@ public void removeInternal(AbstractCompiler compiler) { } } - private class MethodDefinition extends Removable { - final Node methodNode; + private class ClassOrPrototypeNamedProperty extends Removable { + final Node propertyNode; - MethodDefinition(RemovableBuilder builder, Node methodNode) { + ClassOrPrototypeNamedProperty(RemovableBuilder builder, Node propertyNode) { super(builder); - this.methodNode = methodNode; + this.propertyNode = propertyNode; } @Override - boolean isMethodDeclaration() { + boolean isClassOrPrototypeNamedProperty() { return true; } @Override void removeInternal(AbstractCompiler compiler) { - NodeUtil.deleteNode(methodNode, compiler); + NodeUtil.deleteNode(propertyNode, compiler); } } diff --git a/test/com/google/javascript/jscomp/RemoveUnusedVarsPrototypePropertiesTest.java b/test/com/google/javascript/jscomp/RemoveUnusedVarsPrototypePropertiesTest.java index 34611c19eb3..4d3d5d53e37 100644 --- a/test/com/google/javascript/jscomp/RemoveUnusedVarsPrototypePropertiesTest.java +++ b/test/com/google/javascript/jscomp/RemoveUnusedVarsPrototypePropertiesTest.java @@ -84,8 +84,7 @@ public void testAnalyzePrototypeProperties() { "var x = new e; x.a()"); } - public void disabledTestObjectLiteralPrototype() { - // TODO(bradfordcsmith): handle properties in object literal assigned to prototype + public void testObjectLiteralPrototype() { test("function e(){}" + "e.prototype = {a: function(){}, b: function(){}};" + "var x=new e; x.a()", @@ -463,8 +462,7 @@ public void testGlobalFunctionsInGraph8() { ""); } - public void disabledTestGetterBaseline() { - // TODO(bradfordcsmith): remove unused getters + public void testGetterBaseline() { keepGlobals = true; test( "function Foo() {}" + @@ -481,37 +479,41 @@ public void disabledTestGetterBaseline() { "function x() { (new Foo).methodA(); }"); } - public void disabledTestGetter1() { - // TODO(bradfordcsmith): implement getters and setters + public void testGetter1() { test( - "function Foo() {}" + - "Foo.prototype = { " + - " get methodA() {}," + - " get methodB() { x(); }" + - "};" + - "function x() { (new Foo).methodA; }", - - "function Foo() {}" + - "Foo.prototype = {};"); + lines( + "function Foo() {}", + "Foo.prototype = { ", + " get methodA() {},", + " get methodB() { x(); }", + "};", + "function x() { (new Foo).methodA; }", + "new Foo();"), + lines( + "function Foo() {}", + // x() and all methods of Foo removed. + "Foo.prototype = {};", + "new Foo();")); keepGlobals = true; test( - "function Foo() {}" + - "Foo.prototype = { " + - " get methodA() {}," + - " get methodB() { x(); }" + - "};" + - "function x() { (new Foo).methodA; }", - - "function Foo() {}" + - "Foo.prototype = { " + - " get methodA() {}" + - "};" + - "function x() { (new Foo).methodA; }"); + lines( + "function Foo() {}", + "Foo.prototype = { ", + " get methodA() {},", + " get methodB() { x(); }", + "};", + "function x() { (new Foo).methodA; }"), + lines( + "function Foo() {}", + "Foo.prototype = { ", + " get methodA() {}", + "};", + // x() keeps methodA alive + "function x() { (new Foo).methodA; }")); } - public void disabledTestGetter2() { - // TODO(bradfordcsmith): remove unused getters + public void testGetter2() { keepGlobals = true; test( "function Foo() {}" +