Skip to content

Commit

Permalink
Optional chaining parser fixes
Browse files Browse the repository at this point in the history
Summary:
D7029173 implemented basic lexer/parser support for optional chaining, based on the [Babylon implementation](babel/babylon#545) linked from the [proposal](https://github.com/tc39/proposal-optional-chaining#related-issues). However, that code is out of sync with the current specification. Babel has since [updated](babel/babel#7256) its code to fix those issues. This diff parallels those changes, which should make their way into [ESTree](estree/estree#146).

Notes:
* We never prohibited `delete`-ing an optional chain, so there's no need to add this back in.
* D7029173 already prohibits combining optional chains with `super`, `new` expressions, and template literals.
* Rather than annotating `Member` and `Call` AST nodes with an `optional` property, we have new `OptionalMember` and `OptionalCall` nodes. This makes detecting and tracking optional chains easier, and we can leverage this to track where [parentheses have curtailed short-circuiting](https://github.com/tc39/proposal-optional-chaining#edge-case-grouping).

Reviewed By: mroch

Differential Revision: D7380919

fbshipit-source-id: bbfaa620936bd14c7f8e06a85ae22d81b144a6d4
  • Loading branch information
Mayank Patke authored and facebook-github-bot committed Apr 19, 2018
1 parent 24cfe90 commit 344a7c5
Show file tree
Hide file tree
Showing 38 changed files with 768 additions and 270 deletions.
10 changes: 10 additions & 0 deletions packages/flow-parser/test/custom_ast_types.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,3 +232,13 @@ def('Import')

def('CallExpression')
.field('callee', or(def('Expression'), def('Import')));

def('OptionalMemberExpression')
.bases("MemberExpression")
.build("optional")
.field("optional", Boolean)

def('OptionalCallExpression')
.bases("CallExpression")
.build("optional")
.field("optional", Boolean)
7 changes: 0 additions & 7 deletions packages/flow-parser/test/esprima_test_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -260,13 +260,6 @@ function handleSpecialObjectCompare(esprima, flow, env) {
delete param.typeAnnotation;
delete param.optional;
}
/* Esprima doesn't implement optional chaining, so ignore the `optional`
* fields on `Member` and `Call` Flow AST nodes.
*/
case 'MemberExpression':
case 'CallExpression':
delete flow.optional;
break;
}

switch (esprima.type) {
Expand Down
18 changes: 14 additions & 4 deletions src/common/reason.ml
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,7 @@ end)
* In the first example we need to wrap 1 + 2 to correctly print the property
* access. However, we don't need to wrap o in o.p. In o[1 + 2] we don't need to
* wrap 1 + 2 since it is already wrapped in a sense. *)
let rec code_desc_of_expression ~wrap (_, x) =
let rec code_desc_of_expression ~wrap (loc, x) =
let do_wrap = if wrap then (fun s -> "(" ^ s ^ ")") else (fun s -> s) in
Ast.Expression.(match x with
| Array { Array.elements = []; _ } -> "[]"
Expand Down Expand Up @@ -815,9 +815,9 @@ Ast.Expression.(match x with
do_wrap (left ^ " " ^ operator ^ " " ^ right)
| Binary { Binary.operator; left; right } ->
do_wrap (code_desc_of_operation left (`Binary operator) right)
| Call { Call.callee; arguments = []; optional = _ } ->
| Call { Call.callee; arguments = [] } ->
(code_desc_of_expression ~wrap:true callee) ^ "()"
| Call { Call.callee; arguments = _; optional = _ } ->
| Call { Call.callee; arguments = _ } ->
(code_desc_of_expression ~wrap:true callee) ^ "(...)"
| Class _ -> "class { ... }"
| Conditional { Conditional.test; consequent; alternate } ->
Expand All @@ -835,7 +835,7 @@ Ast.Expression.(match x with
| Ast.Expression.Literal x -> code_desc_of_literal x
| Logical { Logical.operator; left; right } ->
do_wrap (code_desc_of_operation left (`Logical operator) right)
| Member { Member._object; property; computed = _; optional = _ } -> Member.(
| Member { Member._object; property; computed = _ } -> Member.(
let o = code_desc_of_expression ~wrap:true _object in
o ^ (match property with
| PropertyIdentifier (_, x) -> "." ^ x
Expand All @@ -848,6 +848,16 @@ Ast.Expression.(match x with
| New { New.callee; arguments = _ } ->
"new " ^ (code_desc_of_expression ~wrap:true callee) ^ "(...)"
| Object _ -> "{...}"
| OptionalCall { OptionalCall.call; optional } ->
let call_desc = code_desc_of_expression ~wrap:false (loc, Call call) in
if optional then "?." ^ call_desc else call_desc
| OptionalMember { OptionalMember.member; optional } ->
let member_desc = code_desc_of_expression ~wrap:false (loc, Member member) in
if optional
then Member.(match member.property with
| PropertyExpression _ -> "?." ^ member_desc
| PropertyIdentifier _ | PropertyPrivateName _ -> "?" ^ member_desc
) else member_desc
| Sequence { Sequence.expressions } ->
code_desc_of_expression ~wrap (List.hd (List.rev expressions))
| Super -> "super"
Expand Down
12 changes: 12 additions & 0 deletions src/parser/ast.ml
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,11 @@ and Expression : sig
type 'M t = {
callee: 'M Expression.t;
arguments: 'M expression_or_spread list;
}
end
module OptionalCall : sig
type 'M t = {
call: 'M Call.t;
optional: bool;
}
end
Expand All @@ -735,6 +740,11 @@ and Expression : sig
_object: 'M Expression.t;
property: 'M property;
computed: bool;
}
end
module OptionalMember : sig
type 'M t = {
member: 'M Member.t;
optional: bool;
}
end
Expand Down Expand Up @@ -799,6 +809,8 @@ and Expression : sig
| MetaProperty of 'M MetaProperty.t
| New of 'M New.t
| Object of 'M Object.t
| OptionalCall of 'M OptionalCall.t
| OptionalMember of 'M OptionalMember.t
| Sequence of 'M Sequence.t
| Super
| TaggedTemplate of 'M TaggedTemplate.t
Expand Down
48 changes: 30 additions & 18 deletions src/parser/estree_translator.ml
Original file line number Diff line number Diff line change
Expand Up @@ -470,25 +470,19 @@ end with type t = Impl.t) = struct
"arguments", array_of_list expression_or_spread _new.arguments;
]
)
| loc, Call call -> Call.(
node "CallExpression" loc [
"callee", expression call.callee;
"arguments", array_of_list expression_or_spread call.arguments;
"optional", bool call.optional;
]
| loc, Call call ->
node "CallExpression" loc (call_node_properties call)
| loc, OptionalCall opt_call -> OptionalCall.(
node "OptionalCallExpression" loc (call_node_properties opt_call.call @ [
"optional", bool opt_call.optional;
])
)
| loc, Member member -> Member.(
let property = match member.property with
| PropertyIdentifier id -> identifier id
| PropertyPrivateName name -> private_name name
| PropertyExpression expr -> expression expr
in
node "MemberExpression" loc [
"object", expression member._object;
"property", property;
"computed", bool member.computed;
"optional", bool member.optional;
]
| loc, Member member ->
node "MemberExpression" loc (member_node_properties member)
| loc, OptionalMember opt_member -> OptionalMember.(
node "OptionalMemberExpression" loc (member_node_properties opt_member.member @ [
"optional", bool opt_member.optional;
])
)
| loc, Yield yield -> Yield.(
node "YieldExpression" loc [
Expand Down Expand Up @@ -1447,4 +1441,22 @@ end with type t = Impl.t) = struct
in
node _type loc value
)

and call_node_properties call = Expression.Call.([
"callee", expression call.callee;
"arguments", array_of_list expression_or_spread call.arguments;
])

and member_node_properties member = Expression.Member.(
let property = match member.property with
| PropertyIdentifier id -> identifier id
| PropertyPrivateName name -> private_name name
| PropertyExpression expr -> expression expr
in
[
"object", expression member._object;
"property", property;
"computed", bool member.computed;
]
)
end
59 changes: 42 additions & 17 deletions src/parser/expression_parser.ml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ module Expression
| _, Literal _
| _, Logical _
| _, New _
| _, OptionalCall _
| _, OptionalMember _
| _, Sequence _
| _, Super
| _, TaggedTemplate _
Expand Down Expand Up @@ -212,6 +214,8 @@ module Expression
| _, Logical _
| _, New _
| _, Object _
| _, OptionalCall _
| _, OptionalMember _
| _, Sequence _
| _, Super
| _, TaggedTemplate _
Expand Down Expand Up @@ -532,12 +536,19 @@ module Expression
| T_LPAREN when not (no_call env) ->
let args_loc, arguments = arguments env in
let loc = Loc.btwn start_loc args_loc in
call_cover ~allow_optional_chain ~in_optional_chain env start_loc
(Cover_expr (loc, Expression.(Call { Call.
callee = as_expression env left;
arguments;
let call = { Expression.Call.
callee = as_expression env left;
arguments;
} in
let call = if in_optional_chain
then Expression.(OptionalCall { OptionalCall.
call;
optional;
})))
})
else Expression.Call call
in
call_cover ~allow_optional_chain ~in_optional_chain env start_loc
(Cover_expr (loc, call))
| _ -> left

and call ?(allow_optional_chain=true) env start_loc left =
Expand Down Expand Up @@ -625,13 +636,20 @@ module Expression
let last_loc = Peek.loc env in
Expect.token env T_RBRACKET;
let loc = Loc.btwn start_loc last_loc in
call_cover ~allow_optional_chain ~in_optional_chain env start_loc
(Cover_expr (loc, Expression.(Member { Member.
_object = as_expression env left;
property = Member.PropertyExpression expr;
computed = true;
let member = Expression.Member.({
_object = as_expression env left;
property = PropertyExpression expr;
computed = true;
}) in
let member = if in_optional_chain
then Expression.(OptionalMember { OptionalMember.
member;
optional;
})))
})
else Expression.Member member
in
call_cover ~allow_optional_chain ~in_optional_chain env start_loc
(Cover_expr (loc, member))
in
let static ?(allow_optional_chain=true) ?(in_optional_chain=false)
?(optional=false) env start_loc left =
Expand All @@ -646,13 +664,20 @@ module Expression
| Cover_expr (_, Ast.Expression.Super) when is_private ->
error_at env (loc, Error.SuperPrivate)
| _ -> () end;
call_cover ~allow_optional_chain ~in_optional_chain env start_loc
(Cover_expr (loc, Expression.(Member { Member.
_object = as_expression env left;
property;
computed = false;
let member = Expression.Member.({
_object = as_expression env left;
property;
computed = false;
}) in
let member = if in_optional_chain
then Expression.(OptionalMember { OptionalMember.
member;
optional;
})))
})
else Expression.Member member
in
call_cover ~allow_optional_chain ~in_optional_chain env start_loc
(Cover_expr (loc, member))
in
fun ?(allow_optional_chain=true) ?(in_optional_chain=false) env start_loc left ->
let options = parse_options env in
Expand Down
2 changes: 1 addition & 1 deletion src/parser/parse_error.ml
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,6 @@ module PP =
use the optional chaining operator (`?.`). Optional chaining is an active early-stage \
feature proposal which may change and is not enabled by default. To enable support in \
the parser, use the `esproposal_optional_chaining` option."
| OptionalChainNew -> "`new` may not be combined with an optional chain."
| OptionalChainNew -> "An optional chain may not be used in a `new` expression."
| OptionalChainTemplate -> "Template literals may not be used in an optional chain."
end
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@ new B?.C?.(a, b)
new B?.C

new B?.C()

new C?.b.d()
Loading

0 comments on commit 344a7c5

Please sign in to comment.