Skip to content

Commit

Permalink
RemoveUnusedVars: remove unused properties in prototype literals
Browse files Browse the repository at this point in the history 
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=178190913
  • Loading branch information
@brad4d @blickly
brad4d authored and blickly committed Dec 7, 2017
1 parent 34c125f commit e8f7a77
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 43 deletions.
66 changes: 52 additions & 14 deletions src/com/google/javascript/jscomp/RemoveUnusedVars.java
View file
Expand Up @@ -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 {
Expand All @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1231,12 +1265,12 @@ boolean isPrototypeObjectNamedPropertyAssignment() {
return isPrototypeObjectPropertyAssignment && isNamedPropertyAssignment();
}

boolean isMethodDeclaration() {
boolean isClassOrPrototypeNamedProperty() {
return false;
}

boolean isIndependentlyRemovableNamedProperty() {
return isPrototypeObjectNamedPropertyAssignment() || isMethodDeclaration();
return isPrototypeObjectNamedPropertyAssignment() || isClassOrPrototypeNamedProperty();
}
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
}

Expand Down
60 changes: 31 additions & 29 deletions test/com/google/javascript/jscomp/RemoveUnusedVarsPrototypePropertiesTest.java
View file
Expand Up @@ -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()",
Expand Down Expand Up @@ -463,8 +462,7 @@ public void testGlobalFunctionsInGraph8() {
"");
}

public void disabledTestGetterBaseline() {
// TODO(bradfordcsmith): remove unused getters
public void testGetterBaseline() {
keepGlobals = true;
test(
"function Foo() {}" +
Expand All @@ -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() {}" +
Expand Down

0 comments on commit e8f7a77

Please sign in to comment.