Skip to content

Commit

Permalink
Correctly parse multi-element optional chains directly following a call
Browse files Browse the repository at this point in the history 
Summary:
The Hermes parser currently appears to have a bug when parsing optional chains with multiple elements that directly follow a call. If an optional member expression directly follows a call expression it will be correctly parsed as an `OptionalMemberExpression`, however if that member expression is followed by a second member expression or call expression, that second member/call expression will be parsed as a `MemberExpression` or `CallExpression` instead of an `OptionalMemberExpression` or `OptionalCallExpression` with `optional = false`.

Both the Babel and Flow parser parse expressions like these correctly ([AST Explorer](https://astexplorer.net/#/gist/045126c9ad90d6535d8d00d2cfe81184/1fbfb97a32549d3e9e8eeef51038205a3e0df16b)).

This can be observed with the following ASTs produced by Hermes:

```
// Note that the outer expression is a MemberExpression instead of an OptionalMemberExpression
a()?.b.c;
// {
//   "type": "MemberExpression",
//   "object": {
//     "type": "OptionalMemberExpression",
//     "object": {
//       "type": "CallExpression",
//       "callee": "a"
//     },
//     "property": "b",
//     "optional": true
//   },
//   "property": "c"
// }

// But when wrapped in parentheses so that the optional member does not directly follow the call, the outer expression is correctly an OptionalMemberExpression
(a())?.b.c;
// {
//   "type": "OptionalMemberExpression",
//   "object": {
//     "type": "OptionalMemberExpression",
//     "object": {
//       "type": "CallExpression",
//       "callee": "a"
//     },
//     "property": "b",
//     "optional": true
//   },
//   "property": "c"
//   "optional": false
// }

// Optional chains not following a call do not have this problem and are parsed correctly
a?.b.c;
// {
//   "type": "OptionalMemberExpression",
//   "object": {
//     "type": "OptionalMemberExpression",
//     "object": "a",
//     "property": "b",
//     "optional": true
//   },
//   "property": "c"
//   "optional": false
// }
```

The same applies to call expressions like in `a()?.b()`, where the last call is parsed as a `CallExpression` instead of an `OptionalCallExpression` with `optional = false`.

This simply arises from not updating whether we have seen an optional chain in a case within `parseCallExpression`. We should instead be marking that we have seen an optional chain when a `?.` token is encountered in `parseCallExpression`.

Reviewed By: avp

Differential Revision: D24092103

fbshipit-source-id: e2ecd8d0743de9485ff4e954571e9f2cb0bf322c
  • Loading branch information
@facebook-github-bot
Hans Halverson authored and facebook-github-bot committed Oct 5, 2020
1 parent ba154cc commit b61f164
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 0 deletions.
4 changes: 4 additions & 0 deletions lib/Parser/JSParserImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3212,6 +3212,10 @@ Optional<ESTree::Node *> JSParserImpl::parseCallExpression(
TokenKind::l_square,
TokenKind::period,
TokenKind::questiondot)) {
if (check(TokenKind::questiondot)) {
seenOptionalChain = true;
}

SMLoc nextStartLoc = tok_->getStartLoc();
auto msel = parseMemberSelect(startLoc, expr, seenOptionalChain);
if (!msel)
Expand Down
60 changes: 60 additions & 0 deletions test/Parser/optional-chaining.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,66 @@ new a().b?.c;
// CHECK-NEXT: "directive": null
// CHECK-NEXT: },

a()?.b();
// CHECK-NEXT: {
// CHECK-NEXT: "type": "ExpressionStatement",
// CHECK-NEXT: "expression": {
// CHECK-NEXT: "type": "OptionalCallExpression",
// CHECK-NEXT: "callee": {
// CHECK-NEXT: "type": "OptionalMemberExpression",
// CHECK-NEXT: "object": {
// CHECK-NEXT: "type": "CallExpression",
// CHECK-NEXT: "callee": {
// CHECK-NEXT: "type": "Identifier",
// CHECK-NEXT: "name": "a"
// CHECK-NEXT: },
// CHECK-NEXT: "arguments": []
// CHECK-NEXT: },
// CHECK-NEXT: "property": {
// CHECK-NEXT: "type": "Identifier",
// CHECK-NEXT: "name": "b"
// CHECK-NEXT: },
// CHECK-NEXT: "computed": false,
// CHECK-NEXT: "optional": true
// CHECK-NEXT: },
// CHECK-NEXT: "arguments": [],
// CHECK-NEXT: "optional": false
// CHECK-NEXT: },
// CHECK-NEXT: "directive": null
// CHECK-NEXT: },

a()?.b.c;
// CHECK-NEXT: {
// CHECK-NEXT: "type": "ExpressionStatement",
// CHECK-NEXT: "expression": {
// CHECK-NEXT: "type": "OptionalMemberExpression",
// CHECK-NEXT: "object": {
// CHECK-NEXT: "type": "OptionalMemberExpression",
// CHECK-NEXT: "object": {
// CHECK-NEXT: "type": "CallExpression",
// CHECK-NEXT: "callee": {
// CHECK-NEXT: "type": "Identifier",
// CHECK-NEXT: "name": "a"
// CHECK-NEXT: },
// CHECK-NEXT: "arguments": []
// CHECK-NEXT: },
// CHECK-NEXT: "property": {
// CHECK-NEXT: "type": "Identifier",
// CHECK-NEXT: "name": "b"
// CHECK-NEXT: },
// CHECK-NEXT: "computed": false,
// CHECK-NEXT: "optional": true
// CHECK-NEXT: },
// CHECK-NEXT: "property": {
// CHECK-NEXT: "type": "Identifier",
// CHECK-NEXT: "name": "c"
// CHECK-NEXT: },
// CHECK-NEXT: "computed": false,
// CHECK-NEXT: "optional": false
// CHECK-NEXT: },
// CHECK-NEXT: "directive": null
// CHECK-NEXT: },

a()?.b;
// CHECK-NEXT: {
// CHECK-NEXT: "type": "ExpressionStatement",
Expand Down

0 comments on commit b61f164

Please sign in to comment.