Skip to content

Commit fdb739a

Browse files
rishipalcopybara-github
authored andcommitted
Update PeepholeMinimizeConditions pass for optional chaining
PiperOrigin-RevId: 324252487
1 parent 4bcd116 commit fdb739a

File tree

2 files changed

+43
-9
lines changed

2 files changed

+43
-9
lines changed

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

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,10 @@ class PeepholeMinimizeConditions
4444
private final boolean late;
4545

4646
/**
47-
* @param late When late is false, this mean we are currently running before
48-
* most of the other optimizations. In this case we would avoid optimizations
49-
* that would make the code harder to analyze (such as using string splitting,
50-
* merging statements with commas, etc). When this is true, we would
51-
* do anything to minimize for size.
47+
* @param late When late is false, this mean we are currently running before most of the other
48+
* optimizations. In this case we would avoid optimizations that would make the code harder to
49+
* analyze (such as using string splitting, merging statements with commas, etc). When late is
50+
* true, we would do anything to minimize for size.
5251
*/
5352
PeepholeMinimizeConditions(boolean late) {
5453
this.late = late;
@@ -809,15 +808,22 @@ private static boolean isFoldableExpressBlock(Node n) {
809808
// http://blickly.github.io/closure-compiler-issues/#291
810809
// We try to detect this case, and not fold EXPR_RESULTs
811810
// into other expressions.
812-
if (maybeExpr.getFirstChild().isCall()) {
811+
// e.g.:
812+
// if (e.onchange) {
813+
// e.onchange({
814+
// _extendedByPrototype: Prototype.emptyFunction,
815+
// target: e
816+
// });
817+
// }
818+
if (maybeExpr.getFirstChild().isCall() || maybeExpr.getFirstChild().isOptChainCall()) {
813819
Node calledFn = maybeExpr.getFirstFirstChild();
814820

815821
// We only have to worry about methods with an implicit 'this'
816822
// param, or this doesn't happen.
817-
if (calledFn.isGetElem()) {
823+
if (calledFn.isGetElem() || calledFn.isOptChainGetElem()) {
818824
return false;
819-
} else if (calledFn.isGetProp()
820-
&& calledFn.getLastChild().getString().startsWith("on")) {
825+
} else if ((calledFn.isGetProp() || calledFn.isOptChainGetProp())
826+
&& calledFn.getLastChild().getString().startsWith("on")) {
821827
return false;
822828
}
823829
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,12 @@ public void testFoldOneChildBlocks() {
6565
late = false;
6666
fold("function f(){if(x)a();x=3}",
6767
"function f(){x&&a();x=3}");
68+
fold("function f(){if(x)a?.();x=3}", "function f(){x&&a?.();x=3}");
69+
6870
fold("function f(){if(x){a()}x=3}",
6971
"function f(){x&&a();x=3}");
72+
fold("function f(){if(x){a?.()}x=3}", "function f(){x&&a?.();x=3}");
73+
7074
fold("function f(){if(x){return 3}}",
7175
"function f(){if(x)return 3}");
7276
fold("function f(){if(x){a()}}",
@@ -84,6 +88,7 @@ public void testFoldOneChildBlocks() {
8488
fold("function f(){if(x){a.b+=1}}", "function f(){x&&(a.b+=1)}");
8589
fold("function f(){if(x){++a.b}}", "function f(){x&&++a.b}");
8690
fold("function f(){if(x){a.foo()}}", "function f(){x&&a.foo()}");
91+
fold("function f(){if(x){a?.foo()}}", "function f(){x&&a?.foo()}");
8792

8893
// Try it out with throw/catch/finally [which should not change]
8994
foldSame("function f(){try{foo()}catch(e){bar(e)}finally{baz()}}");
@@ -178,6 +183,7 @@ public void testCombineIfs2() {
178183
// Can't combine, side-effect
179184
fold("function f(){ if (x) g(); if (y) g() }",
180185
"function f(){ x&&g(); y&&g() }");
186+
fold("function f(){ if (x) g?.(); if (y) g?.() }", "function f(){ x&&g?.(); y&&g?.() }");
181187
// Can't combine, side-effect
182188
fold("function f(){ if (x) y = 0; if (y) y = 0; }",
183189
"function f(){ x&&(y = 0); y&&(y = 0); }");
@@ -269,6 +275,7 @@ public void testAndParenthesesCount() {
269275
fold("function f(){if(x||y)a.foo()}", "function f(){(x||y)&&a.foo()}");
270276
fold("function f(){if(x.a)x.a=0}",
271277
"function f(){x.a&&(x.a=0)}");
278+
fold("function f(){if(x?.a)x.a=0}", "function f(){x?.a&&(x.a=0)}");
272279
foldSame("function f(){if(x()||y()){x()||y()}}");
273280
}
274281

@@ -400,8 +407,12 @@ public void testMinimizeHook() {
400407
fold("x ? x : y", "x || y");
401408
// We assume GETPROPs don't have side effects.
402409
fold("x.y ? x.y : x.z", "x.y || x.z");
410+
fold("x?.y ? x?.y : x.z", "x?.y || x.z");
411+
fold("x?.y ? x?.y : x?.z", "x?.y || x?.z");
412+
403413
// This can be folded if x() does not have side effects.
404414
foldSame("x() ? x() : y()");
415+
foldSame("x?.() ? x?.() : y()");
405416

406417
fold("!x ? foo() : bar()",
407418
"x ? bar() : foo()");
@@ -752,13 +763,21 @@ public void testNestedIfCombine() {
752763
fold("if(x)if(y){if(z){while(1){}}}", "if(x&&(y&&z)){while(1){}}");
753764
}
754765

766+
// See: http://blickly.github.io/closure-compiler-issues/#291
755767
@Test
756768
public void testIssue291() {
757769
fold("if (true) { f.onchange(); }", "if (1) f.onchange();");
758770
foldSame("if (f) { f.onchange(); }");
759771
foldSame("if (f) { f.bar(); } else { f.onchange(); }");
760772
fold("if (f) { f.bonchange(); }", "f && f.bonchange();");
761773
foldSame("if (f) { f['x'](); }");
774+
775+
// optional versions
776+
fold("if (true) { f?.onchange(); }", "if (1) f?.onchange();");
777+
foldSame("if (f) { f?.onchange(); }");
778+
foldSame("if (f) { f?.bar(); } else { f?.onchange(); }");
779+
fold("if (f) { f?.bonchange(); }", "f && f?.bonchange();");
780+
foldSame("if (f) { f?.['x'](); }");
762781
}
763782

764783
@Test
@@ -835,6 +854,15 @@ public void testIssue925() {
835854
"}",
836855
"a = (x[--y]) ? 0 : 1;");
837856

857+
test(
858+
lines(
859+
"if (x?.[--y]) {", //
860+
" a = 0;",
861+
"} else {",
862+
" a = 1;",
863+
"}"),
864+
"a = (x?.[--y]) ? 0 : 1;");
865+
838866
test("if (x++) { x += 2 } else { x += 3 }",
839867
"x++ ? x += 2 : x += 3");
840868

0 commit comments

Comments
 (0)