Skip to content

Commit

Permalink
Fix escaping parameters when "arguments" is referenced.
Browse files Browse the repository at this point in the history
Scope.getArgumentsVar() now returns null for global or module scope, rather than throwing an exception.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=162776652
  • Loading branch information
concavelenz authored and brad4d committed Jul 25, 2017
1 parent af9d80e commit 918812b
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 8 deletions.
23 changes: 18 additions & 5 deletions src/com/google/javascript/jscomp/RemoveUnusedVars.java
Expand Up @@ -323,13 +323,20 @@ private void traverseNode(Node n, Node parent, Scope scope) {
return;
}
} else {

if ("arguments".equals(n.getString())) {
System.out.println("hmm");
}
// If arguments is escaped, we just assume the worst and continue
// on all the parameters. Ignored if we are in block scope
if ("arguments".equals(n.getString()) && scope.isFunctionBlockScope()) {
Node lp = scope.getParentScope().getRootNode().getSecondChild();
for (Node a = lp.getFirstChild(); a != null; a = a.getNext()) {
markReferencedVar(scope.getVar(a.getString()));
if (var != null
&& "arguments".equals(n.getString())
&& var.equals(scope.getArgumentsVar())) {
Scope fnScope = var.getScope();
Node lp = fnScope.getRootNode().getSecondChild();
for (Node p = lp.getFirstChild(); p != null; p = p.getNext()) {
Var paramVar = fnScope.getOwnSlot(p.getString());
checkNotNull(paramVar);
markReferencedVar(paramVar);
}
}

Expand All @@ -349,6 +356,7 @@ private void traverseNode(Node n, Node parent, Scope scope) {
}
}
break;

default:
break;
}
Expand All @@ -363,6 +371,11 @@ private void traverseChildren(Node n, Scope scope) {
}

private boolean isRemovableVar(Var var) {
// If this is a functions "arguments" object, it isn't removable
if (var.equals(var.getScope().getArgumentsVar())) {
return false;
}

// Global variables are off-limits if the user might be using them.
if (!removeGlobals && var.isGlobal()) {
return false;
Expand Down
3 changes: 2 additions & 1 deletion src/com/google/javascript/jscomp/Scope.java
Expand Up @@ -175,8 +175,9 @@ public Var getVar(String name) {
*/
public Var getArgumentsVar() {
if (isGlobal() || isModuleScope()) {
throw new IllegalStateException("No arguments var for scope: " + this);
return null;
}

if (!isFunctionScope() || rootNode.isArrowFunction()) {
return parent.getArgumentsVar();
}
Expand Down
7 changes: 5 additions & 2 deletions test/com/google/javascript/jscomp/RemoveUnusedVarsTest.java
Expand Up @@ -373,9 +373,12 @@ public void testUnusedAssign8() {
"var a = 3; var i; for (i in {}) {i = a} alert(a);");
}

public void testUnusedAssign9a() {
testSame("function b(a) { a = 1; arguments; }; b(6)");
}

public void testUnusedAssign9() {
test("function b(a) { a = 1; arguments=1; }; b(6)",
"function b() { arguments=1; }; b(6)");
testSame("function b(a) { a = 1; arguments=1; }; b(6)");
}

public void testUnusedPropAssign1() {
Expand Down

0 comments on commit 918812b

Please sign in to comment.