Skip to content

Commit a27cc06

Browse files
committed
Fix setters; add more tests
1 parent 745b880 commit a27cc06

File tree

2 files changed

+26
-8
lines changed

2 files changed

+26
-8
lines changed

src/com/google/javascript/jscomp/OptimizeArgumentsArray.java

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,8 @@ public void visit(NodeTraversal traversal, Node node, Node parent) {
150150
*/
151151
private void tryReplaceArguments(Node scopeRoot) {
152152
Node scopeRootParent = scopeRoot.getParent();
153-
// Nothing to do for getters and setters:
154-
// - Getters cannot have any params.
155-
// - Setters can only have one param, and it is required to be present, so we cannot synthesize
156-
// any new params.
157-
if (scopeRootParent.isGetterDef() || scopeRootParent.isSetterDef()) {
153+
// Getters cannot have any params, so there are none to replace or synthesize.
154+
if (scopeRootParent.isGetterDef()) {
158155
return;
159156
}
160157

@@ -170,8 +167,14 @@ private void tryReplaceArguments(Node scopeRoot) {
170167
return;
171168
}
172169

173-
ImmutableSortedMap<Integer, String> argNames =
174-
assembleParamNames(parametersList, highestIndex + 1);
170+
int maxCount = highestIndex + 1;
171+
// Setters can only have one param and it is required to be present, so we cannot synthesize any
172+
// new params, but we can still replace references to the first param.
173+
if (scopeRootParent.isSetterDef()) {
174+
maxCount = 1;
175+
}
176+
177+
ImmutableSortedMap<Integer, String> argNames = assembleParamNames(parametersList, maxCount);
175178
changeMethodSignature(argNames, parametersList);
176179
changeBody(argNames);
177180
}

test/com/google/javascript/jscomp/OptimizeArgumentsArrayTest.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,15 +355,30 @@ public void testGlobalArgumentsReferences() {
355355

356356
@Test
357357
public void testGettersCannotHaveAnyParams() {
358-
// Getters cannot have any parameters; synthesizing one would be an error.
358+
// Getters cannot have any parameters; synthesizing them would be an error.
359359
testSame("class Foo { get prop() { arguments[0] } }");
360360
testSame("const a = { get prop() { arguments[0] } }");
361+
362+
// Ensure references in nested functions are still eligible.
363+
test(
364+
"class Foo { get prop() { function f( ) { arguments[0] } } }", //
365+
"class Foo { get prop() { function f(p0) { p0 } } }");
361366
}
362367

363368
@Test
364369
public void testSettersCanOnlyHaveOneParam() {
365370
// Setters can only have one parameter; synthesizing any more would be an error.
366371
testSame("class Foo { set prop(x) { arguments[1] } }");
367372
testSame("const a = { set prop(x) { arguments[1] } }");
373+
374+
// We can still replace references to the first param.
375+
test(
376+
"class Foo { set prop(x) { arguments[0]; arguments[1] } }", //
377+
"class Foo { set prop(x) { x; arguments[1] } }");
378+
379+
// Ensure references in nested functions are still eligible.
380+
test(
381+
"class Foo { set prop(x) { function f( ) { arguments[0] } } }", //
382+
"class Foo { set prop(x) { function f(p0) { p0 } } }");
368383
}
369384
}

0 commit comments

Comments
 (0)