Skip to content

Commit

Permalink
Add default parameter compatibility to remove unused vars pass
Browse files Browse the repository at this point in the history
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164661798
  • Loading branch information
simran-arora authored and blickly committed Aug 9, 2017
1 parent 25fad63 commit 74711c5
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 15 deletions.
10 changes: 5 additions & 5 deletions src/com/google/javascript/jscomp/NodeUtil.java
Expand Up @@ -3784,15 +3784,15 @@ private static void getLhsNodesHelper(Node n, List<Node> lhsNodes) {
} }
} }


/** /** Retrieves lhs nodes declared in the current declaration. */
* Retrieves lhs nodes declared in the current declaration. static List<Node> getLhsNodesOfDeclaration(Node declNode) {
*/
static Iterable<Node> getLhsNodesOfDeclaration(Node declNode) {
checkArgument( checkArgument(
isNameDeclaration(declNode) isNameDeclaration(declNode)
|| declNode.isParamList() || declNode.isParamList()
|| declNode.isCatch() || declNode.isCatch()
|| declNode.isDestructuringLhs(), declNode); || declNode.isDestructuringLhs()
|| declNode.isDefaultValue(),
declNode);
ArrayList<Node> lhsNodes = new ArrayList<>(); ArrayList<Node> lhsNodes = new ArrayList<>();
getLhsNodesHelper(declNode, lhsNodes); getLhsNodesHelper(declNode, lhsNodes);
return lhsNodes; return lhsNodes;
Expand Down
100 changes: 90 additions & 10 deletions src/com/google/javascript/jscomp/RemoveUnusedVars.java
Expand Up @@ -20,6 +20,7 @@
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;


import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Multimap; import com.google.common.collect.Multimap;
import com.google.javascript.jscomp.CodingConvention.SubclassRelationship; import com.google.javascript.jscomp.CodingConvention.SubclassRelationship;
import com.google.javascript.jscomp.DefinitionsRemover.Definition; import com.google.javascript.jscomp.DefinitionsRemover.Definition;
Expand Down Expand Up @@ -496,19 +497,91 @@ private void removeUnreferencedFunctionArgs(Scope fparamScope) {
boolean modifyCallers = modifyCallSites boolean modifyCallers = modifyCallSites
&& callSiteOptimizer.canModifyCallers(function); && callSiteOptimizer.canModifyCallers(function);
if (!modifyCallers) { if (!modifyCallers) {
// Strip unreferenced args off the end of the function declaration.
Node lastArg; // Remove any unused names from destructuring patterns as long as there are no side effects
while ((lastArg = argList.getLastChild()) != null) { removeUnusedDestructuringNames(argList, fparamScope);
Var var = fparamScope.getVar(lastArg.getString());
if (!referenced.contains(var)) { // Strip as many unreferenced args off the end of the function declaration as possible.
compiler.reportChangeToEnclosingScope(lastArg); maybeRemoveUnusedTrailingParameters(argList, fparamScope);
argList.removeChild(lastArg); } else {
callSiteOptimizer.optimize(fparamScope, referenced);
}
}

/**
* Iterate through the parameters of the function and if they are destructuring parameters, remove
* any unreferenced variables from inside the destructuring pattern.
*/
private void removeUnusedDestructuringNames(Node argList, Scope fparamScope) {
List<Node> destructuringDeclarations = NodeUtil.getLhsNodesOfDeclaration(argList);
for (Node patternElt : Lists.reverse(destructuringDeclarations)) {
Node toRemove = patternElt;
if (patternElt.getParent().isDefaultValue()) {
Node defaultValueRhs = patternElt.getNext();
if (NodeUtil.mayHaveSideEffects(defaultValueRhs)) {
// Protects in the case where function f({a:b = alert('bar')} = alert('foo')){};
continue;
}
toRemove = patternElt.getParent();
}

if (toRemove.getParent().isParamList()) {
continue;
}

// Go through all elements of the object pattern and determine whether they should be
// removed
Var var = fparamScope.getVar(patternElt.getString());
if (!referenced.contains(var)) {
if (toRemove.getParent().isStringKey()) {
toRemove = toRemove.getParent();
}
NodeUtil.deleteNode(toRemove, compiler);
}
}
}

/**
* Strip as many unreferenced args off the end of the function declaration as possible. We start
* from the end of the function declaration because removing parameters from the middle of the
* param list could mess up the interpretation of parameters being sent over by any function
* calls.
*
* @param argList list of function's arguments
* @param fparamScope
*/
private void maybeRemoveUnusedTrailingParameters(Node argList, Scope fparamScope) {
Node lastArg;
while ((lastArg = argList.getLastChild()) != null) {
Node toRemove = lastArg;
if (lastArg.isDefaultValue()) {
toRemove = lastArg.getFirstChild();
Node defaultValueSecondChild = toRemove.getNext();
if (NodeUtil.mayHaveSideEffects(defaultValueSecondChild)) {
break;
}
}

if (toRemove.isDestructuringPattern()) {
if (!toRemove.hasChildren()) {
// Remove empty destructuring patterns and their associated object literal assignment
// if it exists and if the right hand side does not have side effects. Note, a
// destructuring pattern with a "leftover" property key as in {a:{}} is not considered
// empty in this case!
NodeUtil.deleteNode(lastArg, compiler);
continue;
} else { } else {
break; break;
} }
} }
} else {
callSiteOptimizer.optimize(fparamScope, referenced); // Remove unreferenced parameters
Var var = fparamScope.getVar(toRemove.getString());
if (!referenced.contains(var)) {
NodeUtil.deleteNode(lastArg, compiler);
} else {
break;
}
} }
} }


Expand Down Expand Up @@ -932,7 +1005,11 @@ private void removeUnreferencedVars() {
|| NodeUtil.isDestructuringDeclaration(grandParent) || NodeUtil.isDestructuringDeclaration(grandParent)
|| toRemove.isArrayPattern() // Array Pattern || toRemove.isArrayPattern() // Array Pattern
|| parent.isObjectPattern() // Object Pattern || parent.isObjectPattern() // Object Pattern
|| toRemove.isClass(), || toRemove.isClass()
|| ((toRemove.isDefaultValue()
&& NodeUtil.getEnclosingScopeRoot(toRemove).isFunction())
|| ((parent.isDefaultValue())
&& NodeUtil.getEnclosingScopeRoot(parent).isFunction())),
"We should only declare Vars and functions and function args and classes"); "We should only declare Vars and functions and function args and classes");


if ((toRemove.isParamList() && parent.isFunction()) if ((toRemove.isParamList() && parent.isFunction())
Expand All @@ -948,6 +1025,9 @@ private void removeUnreferencedVars() {
fnNameNode.setString(""); fnNameNode.setString("");
} }
// Don't remove bleeding functions. // Don't remove bleeding functions.
} else if (toRemove.isArrayPattern() && grandParent.isParamList()) {
compiler.reportChangeToEnclosingScope(toRemove);
NodeUtil.removeChild(toRemove, nameNode);
} else if (parent.isForIn()) { } else if (parent.isForIn()) {
// foreach iterations have 3 children. Leave them alone. // foreach iterations have 3 children. Leave them alone.
} else if (parent.isDestructuringPattern()) { } else if (parent.isDestructuringPattern()) {
Expand Down
93 changes: 93 additions & 0 deletions test/com/google/javascript/jscomp/RemoveUnusedVarsTest.java
Expand Up @@ -219,6 +219,99 @@ public void testComputedPropertyDestructuring() {
testSame("var a = {['foo' + 1]:1, ['foo' + 2]:2}; alert(a.foo1);"); testSame("var a = {['foo' + 1]:1, ['foo' + 2]:2}; alert(a.foo1);");
} }


public void testFunctionArgRemoval_defaultValue1() {
test(
"function f(unusedParam = undefined) {}; f();",
"function f( ) {}; f();");
}

public void testFunctionArgRemoval_defaultValue2() {
test("function f(unusedParam = 0) {}; f();", "function f( ) {}; f();");
}

public void testFunctionArgRemoval_defaultValue3() {
// Parameters already encountered can be used by later parameters
test("function f(x, y = x + 0) {}; f();", "function f( ) {}; f();");

testSame("function f(x, y = x + 0) { y; }; f();");
}

public void testFunctionArgRemoval_defaultValue4() {
// Default value inside arrow function param list
test("var f = (unusedParam = 0) => {}; f();", "var f = ( ) => {}; f();");

testSame("var f = (usedParam = 0) => { usedParam; }; f();");

test("var f = (usedParam = 0) => {usedParam;};", "");
}

public void testDestructuringParams() {
// Default value not used
test(
"function f({a:{b:b}} = {a:{}}) { /** b is unused */ }; f();",
"function f({a:{}} = {a:{}}) {/** b is unused */}; f();");

// Default value with nested value used in default value assignment
test(
"function f({a:{b:b}} = {a:{b:1}}) { /** b is unused */ }; f();",
"function f({a:{}} = {a:{b:1}}) {/** b is unused */}; f();");

// Default value with nested value used in function body
testSame("function f({a:{b:b}} = {a:{b:1}}) { b; }; f();");

test(
"function f({a:{b:b}} = {a:{b:1}}) { a.b; }; f();",
"function f({a:{}} = {a:{b:1}}) { a.b; }; f();");

test("function f({a:{b:b}} = {a:{}}) { a; }; f();", "function f({a:{}} = {a:{}}) { a; }; f();");

// Destructuring pattern not default and parameter not used
test(
"function f({a:{b:b}}) { /** b is unused */ }; f({c:{d:d}});",
"function f({a:{}}) { /** b is unused */ }; f({c:{d:d}});");

// Destructuring pattern not default and parameter used
testSame("function f({a:{b:b}}) { b; }; f({c:{d:d}});");
}

public void testMixedParamTypes() {
// Traditional and destructuring pattern
test("function f({a:{b:b}}, c, d) { c; }; f();", "function f({a:{}}, c) { c; }; f();");

test("function f({a:{b:b = 5}}, c, d) { c; }; f();", "function f({a:{}}, c) { c; }; f()");

testSame("function f({}, c, {d:{e:e}}) { c; e; }; f();");

// Parent is the parameter list
test("function f({a}, b, {c}) {use(a)}; f({}, {});", "function f({a}) {use(a)}; f({}, {});");

// Default and traditional
testSame("function f(unusedParam = undefined, z) { z; }; f();");
}

public void testDefaultParamsWithSideEffects() {
testSame("function f(a = alert('foo')) {}; f();");

testSame("function f({} = alert('foo')){}; f()");

test("function f({a:b} = alert('foo')){}; f();", "function f({} = alert('foo')){}; f();");

testSame("function f({a:b = alert('bar')} = alert('foo')){}; f();");
}

public void testArrayDestructuringParams() {
// Default values in array pattern unused
test("function f([x,y] = [1,2]) {}; f();", "function f([,] = [1,2]) {}; f();");

test("function f([x,y] = [1,2], z) { z; }; f();", "function f([,] = [1,2], z) { z; }; f();");

// Default values in array pattern used
test("function f([x,y,z] =[1,2,3]) { y; }; f();", "function f([,y] = [1,2,3]) { y; }; f();");

// Side effects
test("function f([x] = [foo()]) {}; f();", "function f([] = [foo()]) {}; f();");
}

public void testFunctionsDeadButEscaped() { public void testFunctionsDeadButEscaped() {
testSame("function b(a) { a = 1; print(arguments[0]) }; b(6)"); testSame("function b(a) { a = 1; print(arguments[0]) }; b(6)");
testSame("function b(a) { var c = 2; a = c; print(arguments[0]) }; b(6)"); testSame("function b(a) { var c = 2; a = c; print(arguments[0]) }; b(6)");
Expand Down

0 comments on commit 74711c5

Please sign in to comment.