Skip to content

Commit

Permalink
Don't modify parameter lists of class methods that reference "argumen…
Browse files Browse the repository at this point in the history
…ts".

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=249832742
  • Loading branch information
concavelenz authored and tjgq committed May 25, 2019
1 parent 952f86e commit 71660df
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 5 deletions.
10 changes: 6 additions & 4 deletions src/com/google/javascript/jscomp/OptimizeParameters.java
Expand Up @@ -384,7 +384,7 @@ private boolean isCandidate(String name, ArrayList<Node> refs) {
return false; return false;
} }


boolean seenCandidateDefiniton = false; boolean seenCandidateDefinition = false;
boolean seenCandidateUse = false; boolean seenCandidateUse = false;
for (Node n : refs) { for (Node n : refs) {
// TODO(johnlenz): Determine what to do about ".constructor" references. // TODO(johnlenz): Determine what to do about ".constructor" references.
Expand All @@ -398,7 +398,7 @@ private boolean isCandidate(String name, ArrayList<Node> refs) {
// TODO(johnlenz): filter .apply when we support it // TODO(johnlenz): filter .apply when we support it
seenCandidateUse = true; seenCandidateUse = true;
} else if (isCandidateDefinition(n)) { } else if (isCandidateDefinition(n)) {
seenCandidateDefiniton = true; seenCandidateDefinition = true;
} else { } else {
// If this isn't an non-aliasing reference (typeof, instanceof, etc) // If this isn't an non-aliasing reference (typeof, instanceof, etc)
// then there is nothing that can be done. // then there is nothing that can be done.
Expand All @@ -409,7 +409,7 @@ private boolean isCandidate(String name, ArrayList<Node> refs) {
} }
} }


return seenCandidateDefiniton && seenCandidateUse; return seenCandidateDefinition && seenCandidateUse;
} }


private boolean isCandidateDefinition(Node n) { private boolean isCandidateDefinition(Node n) {
Expand All @@ -425,7 +425,9 @@ private boolean isCandidateDefinition(Node n) {
return true; return true;
} }
} else if (isClassMemberDefinition(n)) { } else if (isClassMemberDefinition(n)) {
return true; if (!NodeUtil.doesFunctionReferenceOwnArgumentsObject(n.getFirstChild())) {
return true;
}
} }


return false; return false;
Expand Down
20 changes: 19 additions & 1 deletion test/com/google/javascript/jscomp/OptimizeParametersTest.java
Expand Up @@ -732,12 +732,30 @@ public void testApplyIsIgnore() {
} }


@Test @Test
public void testFunctionWithReferenceToArgumentsShouldNotBeOptimize() { public void testFunctionWithReferenceToArgumentsShouldNotBeOptimized() {
testSame("function foo(a,b,c) { return arguments.size; }; foo(1);"); testSame("function foo(a,b,c) { return arguments.size; }; foo(1);");
testSame("var foo = function(a,b,c) { return arguments.size }; foo(1);"); testSame("var foo = function(a,b,c) { return arguments.size }; foo(1);");
testSame("var foo = function bar(a,b,c) { return arguments.size }; foo(2); bar(2);"); testSame("var foo = function bar(a,b,c) { return arguments.size }; foo(2); bar(2);");
} }


@Test
public void testClassMemberWithReferenceToArgumentsShouldNotBeOptimize() {
testSame(
lines(
"class C {", //
" constructor() {",
" }",
" setValue(value) {",
" if (!arguments.length) {",
" return 0;",
" }",
" return value;",
" }",
"}",
"var c = new C();",
"alert(c.setValue(42));"));
}

@Test @Test
public void testFunctionWithTwoNames() { public void testFunctionWithTwoNames() {
testSame("var foo = function bar(a,b) {};"); testSame("var foo = function bar(a,b) {};");
Expand Down

0 comments on commit 71660df

Please sign in to comment.