Skip to content

Commit 4948397

Browse files
rishipalcopybara-github
authored andcommitted
Add optional chaining support in PeepholeFoldConstants pass
PiperOrigin-RevId: 325926033
1 parent abfd8b6 commit 4948397

File tree

5 files changed

+227
-48
lines changed

5 files changed

+227
-48
lines changed

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import com.google.javascript.rhino.jstype.JSType;
5151
import com.google.javascript.rhino.jstype.TernaryValue;
5252
import java.math.BigInteger;
53+
import java.util.ArrayDeque;
5354
import java.util.ArrayList;
5455
import java.util.Collection;
5556
import java.util.EnumSet;
@@ -2027,6 +2028,45 @@ static boolean isEndOfOptChainSegment(Node n) {
20272028
}
20282029
}
20292030

2031+
/**
2032+
* Given the end of an optional chain segment changes all nodes from the end down to the start
2033+
* into non-optional nodes. e.g `({a})?.a.b.c.d()?.x.y.z` gets converted to
2034+
* `({a}).a.b.c.d()?.x.y.z` when the passed in endOfOptChainSegment is `({a})?.a.b.c.d()`.
2035+
*/
2036+
static void convertToNonOptionalChainSegment(Node endOfOptChainSegment) {
2037+
checkArgument(isEndOfOptChainSegment(endOfOptChainSegment), endOfOptChainSegment);
2038+
// Since part of changing the nodes removes the isOptionalChainStart() marker we look for to
2039+
// know we're done, this logic is easier to read if we just find all the nodes first, then
2040+
// change them.
2041+
ArrayDeque<Node> segmentNodes = new ArrayDeque<>();
2042+
Node segmentNode = endOfOptChainSegment;
2043+
while (true) {
2044+
checkState(NodeUtil.isOptChainNode(segmentNode), segmentNode);
2045+
segmentNodes.add(segmentNode);
2046+
if (segmentNode.isOptionalChainStart()) {
2047+
break;
2048+
}
2049+
segmentNode = segmentNode.getFirstChild();
2050+
}
2051+
for (Node n : segmentNodes) {
2052+
n.setIsOptionalChainStart(false);
2053+
n.setToken(getNonOptChainToken(n.getToken()));
2054+
}
2055+
}
2056+
2057+
private static Token getNonOptChainToken(Token optChainToken) {
2058+
switch (optChainToken) {
2059+
case OPTCHAIN_CALL:
2060+
return Token.CALL;
2061+
case OPTCHAIN_GETELEM:
2062+
return Token.GETELEM;
2063+
case OPTCHAIN_GETPROP:
2064+
return Token.GETPROP;
2065+
default:
2066+
throw new IllegalStateException("Should be an OPTCHAIN token: " + optChainToken);
2067+
}
2068+
}
2069+
20302070
/**
20312071
* Is this node the name of a block-scoped declaration? Checks for let, const, class, or
20322072
* block-scoped function declarations.
@@ -4025,6 +4065,9 @@ public static List<Node> findLhsNodesInNode(Node assigningParent) {
40254065

40264066
/** Returns {@code true} if the node is a definition with Object.defineProperties */
40274067
public static boolean isObjectDefinePropertiesDefinition(Node n) {
4068+
// We intentionally don't check optional Object.defineProperties?.() because we expect it to be
4069+
// defined or polyfill-ed, and to avoid changing the current invariant that this method returns
4070+
// true only for non-optional `Object.defineProperties()`.
40284071
if (!n.isCall() || !n.hasXChildren(3)) {
40294072
return false;
40304073
}

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

Lines changed: 3 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature;
2323
import com.google.javascript.rhino.IR;
2424
import com.google.javascript.rhino.Node;
25-
import com.google.javascript.rhino.Token;
2625
import java.util.ArrayDeque;
2726

2827
/**
@@ -190,7 +189,7 @@ private Node rewriteInitialSegment(final Node fullChainStart, final Node fullCha
190189
checkArgument(!NodeUtil.isOptChainNode(receiverNode), receiverNode);
191190

192191
// change the initial chain's nodes to be non-optional
193-
convertToNonOptionalChainSegment(initialChainEnd);
192+
NodeUtil.convertToNonOptionalChainSegment(initialChainEnd);
194193

195194
final Node placeholder = IR.empty();
196195
fullChainEnd.replaceWith(placeholder);
@@ -268,41 +267,7 @@ String declareTempVarName(Node valueNode) {
268267
return tempVarName;
269268
}
270269

271-
/**
272-
* Given the end of an optional chain segment. Change all nodes from the end down to the start
273-
* into non-optional nodes.
274-
*/
275-
private static void convertToNonOptionalChainSegment(Node endOfOptChainSegment) {
276-
// Since part of changing the nodes removes the isOptionalChainStart() marker we look for to
277-
// know we're done, this logic is easier to read if we just find all the nodes first, then
278-
// change them.
279-
final ArrayDeque<Node> segmentNodes = new ArrayDeque<>();
280-
Node segmentNode = endOfOptChainSegment;
281-
while (true) {
282-
checkState(NodeUtil.isOptChainNode(segmentNode), segmentNode);
283-
segmentNodes.add(segmentNode);
284-
if (segmentNode.isOptionalChainStart()) {
285-
break;
286-
} else {
287-
segmentNode = segmentNode.getFirstChild();
288-
}
289-
}
290-
for (Node n : segmentNodes) {
291-
n.setIsOptionalChainStart(false);
292-
n.setToken(getNonOptChainToken(n.getToken()));
293-
}
294-
}
295270

296-
private static Token getNonOptChainToken(Token optChainToken) {
297-
switch (optChainToken) {
298-
case OPTCHAIN_CALL:
299-
return Token.CALL;
300-
case OPTCHAIN_GETELEM:
301-
return Token.GETELEM;
302-
case OPTCHAIN_GETPROP:
303-
return Token.GETPROP;
304-
default:
305-
throw new IllegalStateException("Should be an OPTCHAIN token: " + optChainToken);
306-
}
307-
}
271+
272+
308273
}

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

Lines changed: 55 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,10 @@ class PeepholeFoldConstants extends AbstractPeepholeOptimization {
6363
@Override
6464
Node optimizeSubtree(Node subtree) {
6565
switch (subtree.getToken()) {
66+
case OPTCHAIN_CALL:
6667
case CALL:
6768
return tryFoldCall(subtree);
69+
6870
case NEW:
6971
return tryFoldCtorCall(subtree);
7072

@@ -106,10 +108,12 @@ private Node tryFoldBinaryOperator(Node subtree) {
106108

107109
// If we've reached here, node is truly a binary operator.
108110
switch (subtree.getToken()) {
111+
case OPTCHAIN_GETPROP:
109112
case GETPROP:
110113
return tryFoldGetProp(subtree, left, right);
111114

112115
case GETELEM:
116+
case OPTCHAIN_GETELEM:
113117
return tryFoldGetElem(subtree, left, right);
114118

115119
case INSTANCEOF:
@@ -1284,7 +1288,7 @@ private Node tryFoldCtorCall(Node n) {
12841288
* Object.defineProperties(o, {}) -> o
12851289
*/
12861290
private Node tryFoldCall(Node n) {
1287-
checkArgument(n.isCall());
1291+
checkArgument(n.isCall() || n.isOptChainCall());
12881292

12891293
if (NodeUtil.isObjectDefinePropertiesDefinition(n)) {
12901294
Node srcObj = n.getLastChild();
@@ -1344,10 +1348,11 @@ private Node tryFoldInForcedStringContext(Node n) {
13441348
}
13451349

13461350
/**
1347-
* Try to fold array-element. e.g [1, 2, 3][10];
1351+
* For element access using GETLEM/OPTCHAIN_GETELEM on object literals, arrays or strings, tries
1352+
* to fold the prop access. e.g. folds array-element [1, 2, 3][1];
13481353
*/
13491354
private Node tryFoldGetElem(Node n, Node left, Node right) {
1350-
checkArgument(n.isGetElem());
1355+
checkArgument(n.isGetElem() || n.isOptChainGetElem());
13511356

13521357
if (left.isObjectLit()) {
13531358
return tryFoldObjectPropAccess(n, left, right);
@@ -1364,10 +1369,12 @@ private Node tryFoldGetElem(Node n, Node left, Node right) {
13641369
}
13651370

13661371
/**
1367-
* Try to fold array-length. e.g [1, 2, 3].length ==> 3, [x, y].length ==> 2
1372+
* For prop access using GETPROP/OPTCHAIN_GETPROP on object literals, tries to fold their property
1373+
* access. For prop access on arrays, only tries to fold array-length. e.g [1, 2, 3].length ==> 3,
1374+
* [x, y].length ==> 2
13681375
*/
13691376
private Node tryFoldGetProp(Node n, Node left, Node right) {
1370-
checkArgument(n.isGetProp());
1377+
checkArgument(n.isGetProp() || n.isOptChainGetProp());
13711378

13721379
if (left.isObjectLit()) {
13731380
return tryFoldObjectPropAccess(n, left, right);
@@ -1420,6 +1427,7 @@ private Node tryFoldArrayAccess(Node n, Node left, Node right) {
14201427
double index = right.getDouble();
14211428
int intIndex = (int) index;
14221429
if (intIndex != index) {
1430+
// Ideally this should be caught in the check passes.
14231431
report(INVALID_GETELEM_INDEX_ERROR, right);
14241432
return n;
14251433
}
@@ -1535,8 +1543,25 @@ private Node tryFoldStringArrayAccess(Node n, Node left, Node right) {
15351543
return elem;
15361544
}
15371545

1546+
/**
1547+
* Tries to fold the node `n` that's accessing an object literal's property. Bails out (skips
1548+
* folding and returns the same `n` node) with certainty when any of the following holds true:
1549+
*
1550+
* <ul>
1551+
* <li>the access `n` is L-value
1552+
* <li>the property references super
1553+
* <li>the object has side-effects other than those preserved by folding the property
1554+
* <li>property accessed does not exist on the object (might exist on prototype)
1555+
* </ul>
1556+
*
1557+
* <ul>
1558+
* Examples of folding:
1559+
* <li>`({a() { return 1; }})?.a` ---> `(function() { return 1; })`
1560+
* <li>`({a() { return 1; }}).a()` ---> `(function() { return 1; }())`
1561+
* </ul>
1562+
*/
15381563
private Node tryFoldObjectPropAccess(Node n, Node left, Node right) {
1539-
checkArgument(NodeUtil.isNormalGet(n));
1564+
checkArgument(NodeUtil.isNormalOrOptChainGet(n));
15401565

15411566
if (!left.isObjectLit() || !right.isString()) {
15421567
return n;
@@ -1546,6 +1571,7 @@ private Node tryFoldObjectPropAccess(Node n, Node left, Node right) {
15461571
// If GETPROP/GETELEM is used as assignment target the object literal is
15471572
// acting as a temporary we can't fold it here:
15481573
// "{a:x}.a += 1" is not "x += 1"
1574+
checkState(!NodeUtil.isOptChainNode(n)); // optional chains can not be targets of assign
15491575
return n;
15501576
}
15511577

@@ -1590,7 +1616,7 @@ private Node tryFoldObjectPropAccess(Node n, Node left, Node right) {
15901616
}
15911617

15921618
// Didn't find a definition of the name in the object literal, it might
1593-
// be coming from the Object prototype
1619+
// be coming from the Object prototype.
15941620
if (value == null) {
15951621
return n;
15961622
}
@@ -1635,6 +1661,28 @@ private Node tryFoldObjectPropAccess(Node n, Node left, Node right) {
16351661

16361662
value.detach();
16371663

1664+
Node parent = n.getParent();
1665+
1666+
if (NodeUtil.isOptChainNode(parent)) {
1667+
1668+
/**
1669+
* If the chain continues after `n`, simply doing `n.replaceWith(value)` below would leave the
1670+
* subsequent nodes in the current chain segment optional, with their start `n` replaced.
1671+
*
1672+
* <p>So, we must ensure that all nodes in the chain's current segment are made non-optional.
1673+
*
1674+
* <p>This can happen for e.g.
1675+
*
1676+
* <ul>
1677+
* <li>`({a() { return 1; }})?.a()` ---> `(function() { return 1; })()`. Here, parent
1678+
* OPTCHAIN_CALL node must be converted to CALL.
1679+
* <li>`({a() { return 1; }})?.a().b.c?.d` ---> `(function() { return 1; })().b.c?.d`. Here,
1680+
* all nodes upto `({a() { return 1; }})?.a().b.c` must become non-optional
1681+
* </ul>
1682+
*/
1683+
Node endOfCurrentChain = NodeUtil.getEndOfOptChainSegment(parent);
1684+
NodeUtil.convertToNonOptionalChainSegment(endOfCurrentChain);
1685+
}
16381686
if (keyIsGetter) {
16391687
value = IR.call(value);
16401688
value.putBooleanProp(Node.FREE_CALL, true);

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3721,6 +3721,48 @@ public void testIsBundledGoogModule_onlyIfInScript() {
37213721
}
37223722
}
37233723

3724+
@RunWith(JUnit4.class)
3725+
public static class ConvertToNonOptChainTests {
3726+
@Test
3727+
public void simpleChain() {
3728+
// `expr?.prop`
3729+
Node origChain = IR.startOptChainGetprop(IR.name("expr"), IR.string("prop"));
3730+
NodeUtil.convertToNonOptionalChainSegment(origChain);
3731+
assertThat(isChainConverted(origChain)).isTrue();
3732+
}
3733+
3734+
@Test
3735+
public void continuedChain() {
3736+
// `expr?.prop1.prop2`
3737+
Node innerGetProp = IR.startOptChainGetprop(IR.name("expr"), IR.string("pro1"));
3738+
Node outterGetProp = IR.continueOptChainGetprop(innerGetProp, IR.string("prop2"));
3739+
NodeUtil.convertToNonOptionalChainSegment(outterGetProp);
3740+
assertThat(isChainConverted(outterGetProp)).isTrue();
3741+
}
3742+
3743+
@Test
3744+
public void nestedChain() {
3745+
// `expr2[expr1 ?.prop1]?.prop2`
3746+
Node innerGetProp = IR.startOptChainGetprop(IR.name("expr1"), IR.string("pro1"));
3747+
Node getElem = IR.getelem(IR.name("expr2"), innerGetProp);
3748+
Node outterGetProp = IR.startOptChainGetprop(getElem, IR.string("prop2"));
3749+
3750+
NodeUtil.convertToNonOptionalChainSegment(outterGetProp);
3751+
assertThat(isChainConverted(outterGetProp)).isTrue();
3752+
3753+
// nested (inner) optional chain is unchanged
3754+
assertThat(outterGetProp.getFirstChild().getSecondChild().isOptChainGetProp()).isTrue();
3755+
}
3756+
3757+
// All nodes till the current chain's start are converted to non-optional.
3758+
private boolean isChainConverted(Node node) {
3759+
if (node == null) {
3760+
return true;
3761+
}
3762+
return !NodeUtil.isOptChainNode(node) && isChainConverted(node.getFirstChild());
3763+
}
3764+
}
3765+
37243766
@RunWith(JUnit4.class)
37253767
public static class GetStartOfOptChainTests {
37263768

0 commit comments

Comments
 (0)