Skip to content

Commit

Permalink
Fix bug parsing second optional call expression with type arguments i…
Browse files Browse the repository at this point in the history
…n chain

Summary:
Hermes currently has a bug when parsing optional chains that contain two optional calls that both have type arguments.

This is because `parseCallExpression` maintains a `typeArgs` variable when looping through parts of an optional chain that is filled with type args when a call expression with type arguments is encountered. However this is not being reset to null for optional call expressions, meaning the next optional call expression that is encountered will notice that `typeArgs` is non-null and then enter a bad state when it tries to parse its own type arguments. This results in the second optional call with type arguments actually being parsed as an optional member that is part of two comparisons (due to the `<` and `>`).

The fix for this is simply that we need to clear the `typeArgs` variable after optional call expressions are parsed.

Reviewed By: avp

Differential Revision: D24550737

fbshipit-source-id: 0e6dc65e0ae4ac9ac20a78d39085b6933189bb5d
  • Loading branch information
Hans Halverson authored and facebook-github-bot committed Oct 28, 2020
1 parent 63588a7 commit 09c8e5f
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 3 deletions.
7 changes: 4 additions & 3 deletions lib/Parser/JSParserImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3253,10 +3253,11 @@ Optional<ESTree::Node *> JSParserImpl::parseCallExpression(
debugLoc,
new (context_)
ESTree::CallExpressionNode(expr, typeArgs, std::move(argList)));
// typeArgs have been used, discard them so the next item in the call
// chain can populate them if necessary.
typeArgs = nullptr;
}

// typeArgs have been used, discard them so the next item in the call
// chain can populate them if necessary.
typeArgs = nullptr;
} else if (checkN(
TokenKind::l_square,
TokenKind::period,
Expand Down
101 changes: 101 additions & 0 deletions test/Parser/flow/optional-chaining.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

// RUN: %hermes -dump-ast -parse-flow --pretty-json %s | %FileCheck %s --match-full-lines

// CHECK: {
// CHECK-NEXT: "type": "Program",
// CHECK-NEXT: "body": [

a?.b<string>();
// 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": "Identifier",
// CHECK-NEXT: "name": "a"
// 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: "typeArguments": {
// CHECK-NEXT: "type": "TypeParameterInstantiation",
// CHECK-NEXT: "params": [
// CHECK-NEXT: {
// CHECK-NEXT: "type": "StringTypeAnnotation"
// CHECK-NEXT: }
// CHECK-NEXT: ]
// CHECK-NEXT: },
// CHECK-NEXT: "arguments": [],
// CHECK-NEXT: "optional": false
// CHECK-NEXT: },
// CHECK-NEXT: "directive": null
// CHECK-NEXT: },

a?.b<string>()?.c<string>();
// 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": "OptionalCallExpression",
// CHECK-NEXT: "callee": {
// CHECK-NEXT: "type": "OptionalMemberExpression",
// CHECK-NEXT: "object": {
// CHECK-NEXT: "type": "Identifier",
// CHECK-NEXT: "name": "a"
// 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: "typeArguments": {
// CHECK-NEXT: "type": "TypeParameterInstantiation",
// CHECK-NEXT: "params": [
// CHECK-NEXT: {
// CHECK-NEXT: "type": "StringTypeAnnotation"
// CHECK-NEXT: }
// CHECK-NEXT: ]
// CHECK-NEXT: },
// CHECK-NEXT: "arguments": [],
// CHECK-NEXT: "optional": false
// CHECK-NEXT: },
// CHECK-NEXT: "property": {
// CHECK-NEXT: "type": "Identifier",
// CHECK-NEXT: "name": "c"
// CHECK-NEXT: },
// CHECK-NEXT: "computed": false,
// CHECK-NEXT: "optional": true
// CHECK-NEXT: },
// CHECK-NEXT: "typeArguments": {
// CHECK-NEXT: "type": "TypeParameterInstantiation",
// CHECK-NEXT: "params": [
// CHECK-NEXT: {
// CHECK-NEXT: "type": "StringTypeAnnotation"
// CHECK-NEXT: }
// CHECK-NEXT: ]
// CHECK-NEXT: },
// CHECK-NEXT: "arguments": [],
// CHECK-NEXT: "optional": false
// CHECK-NEXT: },
// CHECK-NEXT: "directive": null
// CHECK-NEXT: }

// CHECK-NEXT: ]
// CHECK-NEXT: }

0 comments on commit 09c8e5f

Please sign in to comment.