Skip to content

Commit 21b4a00

Browse files
Hans Halversonfacebook-github-bot
Hans Halverson
authored and
facebook-github-bot
committed
Intern comments in JSX spread children
Summary: Interns comments in JSX spread children. JSX spread children appear as JSX children, so that cannot have comments directly preceding or suceeding them. However they can have comments between the opening brace and the ellipsis. We can attach these as the "leading" comments for JSX spread children. The comments directly preceding and succeeding the expression that is spread will be attached to the expression itself: ``` <div>{/* L spread */... /* L expr */ expr /* T expr */}</div> ``` Note that this required creating a new record type in the ast for JSX spread children so that we can store both the expression and comments, as no record type previously existed. Reviewed By: pieterv Differential Revision: D20317867 fbshipit-source-id: 567880ce9695c52f26bf4c4eb013f92c4abb5fa1
1 parent 06b68e8 commit 21b4a00

File tree

12 files changed

+172
-15
lines changed

12 files changed

+172
-15
lines changed

‎packages/flow-parser/test/custom_ast_types.js‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,16 @@ def("BigIntLiteralTypeAnnotation")
285285
def("SymbolTypeAnnotation")
286286
.bases("Type");
287287

288+
def("JSXElement")
289+
.field("children", [or(
290+
def("JSXElement"),
291+
def("JSXExpressionContainer"),
292+
def("JSXFragment"),
293+
def("JSXText"),
294+
def("Literal"),
295+
def("JSXSpreadChild")
296+
)], defaults.emptyArray);
297+
288298
def("JSXFragment")
289299
.bases("Expression")
290300
.build("openingFragment", "closingFragment", "children")
@@ -295,7 +305,8 @@ def("JSXFragment")
295305
def("JSXExpressionContainer"),
296306
def("JSXFragment"),
297307
def("JSXText"),
298-
def("Literal")
308+
def("Literal"),
309+
def("JSXSpreadChild")
299310
)], defaults.emptyArray)
300311

301312
def("JSXOpeningFragment")
@@ -306,6 +317,11 @@ def("JSXClosingFragment")
306317
.bases("Node")
307318
.build();
308319

320+
def("JSXSpreadChild")
321+
.bases("Expression")
322+
.build("expression")
323+
.field("expression", def("Expression"));
324+
309325
// Enums
310326
def("EnumDeclaration")
311327
.bases("Declaration")

‎src/parser/estree_translator.ml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1516,7 +1516,7 @@ with type t = Impl.t = struct
15161516
| (loc, Element element) -> jsx_element (loc, element)
15171517
| (loc, Fragment fragment) -> jsx_fragment (loc, fragment)
15181518
| (loc, ExpressionContainer expr) -> jsx_expression_container (loc, expr)
1519-
| (loc, SpreadChild expr) -> node "JSXSpreadChild" loc [("expression", expression expr)]
1519+
| (loc, SpreadChild spread) -> jsx_spread_child (loc, spread)
15201520
| (loc, Text str) -> jsx_text (loc, str))
15211521
and jsx_name =
15221522
JSX.(
@@ -1554,6 +1554,8 @@ with type t = Impl.t = struct
15541554
node ?comments "JSXEmptyExpression" empty_loc []
15551555
in
15561556
node ?comments "JSXExpressionContainer" loc [("expression", expression)]
1557+
and jsx_spread_child (loc, { JSX.SpreadChild.expression = expr; comments }) =
1558+
node ?comments "JSXSpreadChild" loc [("expression", expression expr)]
15571559
and jsx_text (loc, { JSX.Text.value; raw }) =
15581560
node "JSXText" loc [("value", string value); ("raw", string raw)]
15591561
and jsx_member_expression (loc, { JSX.MemberExpression._object; property }) =

‎src/parser/flow_ast.ml‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1323,13 +1323,21 @@ and JSX : sig
13231323
and ('M, 'T) t' = { name: ('M, 'T) name } [@@deriving show]
13241324
end
13251325

1326+
module SpreadChild : sig
1327+
type ('M, 'T) t = {
1328+
expression: ('M, 'T) Expression.t;
1329+
comments: ('M, unit) Syntax.t option;
1330+
}
1331+
[@@deriving show]
1332+
end
1333+
13261334
type ('M, 'T) child = 'M * ('M, 'T) child'
13271335

13281336
and ('M, 'T) child' =
13291337
| Element of ('M, 'T) element
13301338
| Fragment of ('M, 'T) fragment
13311339
| ExpressionContainer of ('M, 'T) ExpressionContainer.t
1332-
| SpreadChild of ('M, 'T) Expression.t
1340+
| SpreadChild of ('M, 'T) SpreadChild.t
13331341
| Text of Text.t
13341342

13351343
and ('M, 'T) element = {

‎src/parser/jsx_parser.ml‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,14 @@ module JSX (Parse : Parser_common.PARSER) = struct
6161
let result =
6262
match Peek.token env with
6363
| T_ELLIPSIS ->
64+
let leading = Peek.comments env in
6465
Expect.token env T_ELLIPSIS;
65-
let expr = Parse.assignment env in
66-
JSX.SpreadChild expr
66+
let expression = Parse.assignment env in
67+
JSX.SpreadChild
68+
{
69+
JSX.SpreadChild.expression;
70+
comments = Flow_ast_utils.mk_comments_opt ~leading ();
71+
}
6772
| _ ->
6873
let expression = expression_container_contents env in
6974
JSX.ExpressionContainer { JSX.ExpressionContainer.expression; comments = None }

‎src/parser/test/flow/comment_interning/jsx_element.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,5 @@
1111
<div /* 6.1 L JSX id */ name /* 6.2 T JSX id */="test" />;
1212

1313
<div name=/* 7.1 L JSX expr */ {/* 7.2 L num */ 1 /* 7.3 T num */} /* 7.4 T JSX expr */ />;
14+
15+
<div>{/* 8.1 L JSX spread */ ... /* 8.2 L obj */ {} /* 8.3 T obj */}</div>;

‎src/parser/test/flow/comment_interning/jsx_element.tree.json‎

Lines changed: 92 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"type":"Program",
3-
"loc":{"source":null,"start":{"line":1,"column":24},"end":{"line":13,"column":91}},
4-
"range":[24,552],
3+
"loc":{"source":null,"start":{"line":1,"column":24},"end":{"line":15,"column":75}},
4+
"range":[24,629],
55
"body":[
66
{
77
"type":"ExpressionStatement",
@@ -490,6 +490,78 @@
490490
"children":[]
491491
},
492492
"directive":null
493+
},
494+
{
495+
"type":"ExpressionStatement",
496+
"loc":{"source":null,"start":{"line":15,"column":0},"end":{"line":15,"column":75}},
497+
"range":[554,629],
498+
"expression":{
499+
"type":"JSXElement",
500+
"loc":{"source":null,"start":{"line":15,"column":0},"end":{"line":15,"column":74}},
501+
"range":[554,628],
502+
"openingElement":{
503+
"type":"JSXOpeningElement",
504+
"loc":{"source":null,"start":{"line":15,"column":0},"end":{"line":15,"column":5}},
505+
"range":[554,559],
506+
"name":{
507+
"type":"JSXIdentifier",
508+
"loc":{"source":null,"start":{"line":15,"column":1},"end":{"line":15,"column":4}},
509+
"range":[555,558],
510+
"name":"div"
511+
},
512+
"attributes":[],
513+
"selfClosing":false
514+
},
515+
"closingElement":{
516+
"type":"JSXClosingElement",
517+
"loc":{"source":null,"start":{"line":15,"column":68},"end":{"line":15,"column":74}},
518+
"range":[622,628],
519+
"name":{
520+
"type":"JSXIdentifier",
521+
"loc":{"source":null,"start":{"line":15,"column":70},"end":{"line":15,"column":73}},
522+
"range":[624,627],
523+
"name":"div"
524+
}
525+
},
526+
"children":[
527+
{
528+
"type":"JSXSpreadChild",
529+
"leadingComments":[
530+
{
531+
"type":"Block",
532+
"loc":{"source":null,"start":{"line":15,"column":6},"end":{"line":15,"column":28}},
533+
"range":[560,582],
534+
"value":" 8.1 L JSX spread "
535+
}
536+
],
537+
"loc":{"source":null,"start":{"line":15,"column":5},"end":{"line":15,"column":68}},
538+
"range":[559,622],
539+
"expression":{
540+
"type":"ObjectExpression",
541+
"trailingComments":[
542+
{
543+
"type":"Block",
544+
"loc":{"source":null,"start":{"line":15,"column":52},"end":{"line":15,"column":67}},
545+
"range":[606,621],
546+
"value":" 8.3 T obj "
547+
}
548+
],
549+
"leadingComments":[
550+
{
551+
"type":"Block",
552+
"loc":{"source":null,"start":{"line":15,"column":33},"end":{"line":15,"column":48}},
553+
"range":[587,602],
554+
"value":" 8.2 L obj "
555+
}
556+
],
557+
"loc":{"source":null,"start":{"line":15,"column":49},"end":{"line":15,"column":51}},
558+
"range":[603,605],
559+
"properties":[]
560+
}
561+
}
562+
]
563+
},
564+
"directive":null
493565
}
494566
],
495567
"comments":[
@@ -624,6 +696,24 @@
624696
"loc":{"source":null,"start":{"line":13,"column":67},"end":{"line":13,"column":87}},
625697
"range":[528,548],
626698
"value":" 7.4 T JSX expr "
699+
},
700+
{
701+
"type":"Block",
702+
"loc":{"source":null,"start":{"line":15,"column":6},"end":{"line":15,"column":28}},
703+
"range":[560,582],
704+
"value":" 8.1 L JSX spread "
705+
},
706+
{
707+
"type":"Block",
708+
"loc":{"source":null,"start":{"line":15,"column":33},"end":{"line":15,"column":48}},
709+
"range":[587,602],
710+
"value":" 8.2 L obj "
711+
},
712+
{
713+
"type":"Block",
714+
"loc":{"source":null,"start":{"line":15,"column":52},"end":{"line":15,"column":67}},
715+
"range":[606,621],
716+
"value":" 8.3 T obj "
627717
}
628718
]
629719
}

‎src/parser_utils/flow_ast_differ.ml‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,8 +1239,8 @@ let program
12391239
diff_if_changed_ret_opt (jsx_fragment old_loc) frag1 frag2
12401240
| (ExpressionContainer expr1, ExpressionContainer expr2) ->
12411241
diff_if_changed_ret_opt (jsx_expression old_loc) expr1 expr2
1242-
| (SpreadChild expr1, SpreadChild expr2) ->
1243-
diff_if_changed expression expr1 expr2 |> Base.Option.return
1242+
| (SpreadChild spread1, SpreadChild spread2) ->
1243+
diff_if_changed_ret_opt (jsx_spread_child old_loc) spread1 spread2
12441244
| (Text _, Text _) -> None
12451245
| _ -> None
12461246
in
@@ -1261,6 +1261,16 @@ let program
12611261
| _ -> None
12621262
in
12631263
join_diff_list [expression_diff; comments_diff]
1264+
and jsx_spread_child
1265+
(loc : Loc.t)
1266+
(jsx_spread_child1 : (Loc.t, Loc.t) Ast.JSX.SpreadChild.t)
1267+
(jsx_spread_child2 : (Loc.t, Loc.t) Ast.JSX.SpreadChild.t) : node change list option =
1268+
let open Ast.JSX.SpreadChild in
1269+
let { expression = expr1; comments = comments1 } = jsx_spread_child1 in
1270+
let { expression = expr2; comments = comments2 } = jsx_spread_child2 in
1271+
let expression_diff = Some (diff_if_changed expression expr1 expr2) in
1272+
let comments_diff = syntax_opt loc comments1 comments2 in
1273+
join_diff_list [expression_diff; comments_diff]
12641274
and assignment
12651275
(loc : Loc.t)
12661276
(assn1 : (Loc.t, Loc.t) Ast.Expression.Assignment.t)

‎src/parser_utils/flow_ast_mapper.ml‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1288,8 +1288,8 @@ class ['loc] mapper =
12881288
id_loc this#jsx_fragment loc frag child (fun frag -> (loc, Fragment frag))
12891289
| (loc, ExpressionContainer expr) ->
12901290
id_loc this#jsx_expression loc expr child (fun expr -> (loc, ExpressionContainer expr))
1291-
| (loc, SpreadChild expr) ->
1292-
id this#expression expr child (fun expr -> (loc, SpreadChild expr))
1291+
| (loc, SpreadChild spread) ->
1292+
id this#jsx_spread_child spread child (fun spread -> (loc, SpreadChild spread))
12931293
| (_loc, Text _) -> child
12941294

12951295
method jsx_expression _loc (jsx_expr : ('loc, 'loc) Ast.JSX.ExpressionContainer.t) =
@@ -1309,6 +1309,16 @@ class ['loc] mapper =
13091309
else
13101310
{ expression = EmptyExpression; comments = comments' }
13111311

1312+
method jsx_spread_child (jsx_spread_child : ('loc, 'loc) Ast.JSX.SpreadChild.t) =
1313+
let open Ast.JSX.SpreadChild in
1314+
let { expression; comments } = jsx_spread_child in
1315+
let expression' = this#expression expression in
1316+
let comments' = this#syntax_opt comments in
1317+
if expression == expression' && comments == comments' then
1318+
jsx_spread_child
1319+
else
1320+
{ expression = expression'; comments = comments' }
1321+
13121322
method jsx_name (name : ('loc, 'loc) Ast.JSX.name) =
13131323
let open Ast.JSX in
13141324
let name' =

‎src/parser_utils/flow_polymorphic_ast_mapper.ml‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1066,7 +1066,7 @@ class virtual ['M, 'T, 'N, 'U] mapper =
10661066
| Element elem -> Element (this#jsx_element elem)
10671067
| Fragment frag -> Fragment (this#jsx_fragment frag)
10681068
| ExpressionContainer expr -> ExpressionContainer (this#jsx_expression expr)
1069-
| SpreadChild expr -> SpreadChild (this#expression expr)
1069+
| SpreadChild spread -> SpreadChild (this#jsx_spread_child spread)
10701070
| Text _ as child' -> child' )
10711071

10721072
method jsx_expression (jsx_expr : ('M, 'T) Ast.JSX.ExpressionContainer.t)
@@ -1081,6 +1081,14 @@ class virtual ['M, 'T, 'N, 'U] mapper =
10811081
let comments' = Base.Option.map ~f:this#syntax comments in
10821082
{ expression = expression'; comments = comments' }
10831083

1084+
method jsx_spread_child (jsx_spread_child : ('M, 'T) Ast.JSX.SpreadChild.t)
1085+
: ('N, 'U) Ast.JSX.SpreadChild.t =
1086+
let open Ast.JSX.SpreadChild in
1087+
let { expression; comments } = jsx_spread_child in
1088+
let expression' = this#expression expression in
1089+
let comments' = Base.Option.map ~f:this#syntax comments in
1090+
{ expression = expression'; comments = comments' }
1091+
10841092
method jsx_name (name : ('M, 'T) Ast.JSX.name) : ('N, 'U) Ast.JSX.name =
10851093
let open Ast.JSX in
10861094
match name with

‎src/parser_utils/flow_polymorphic_ast_mapper.mli‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,8 @@ class virtual ['M, 'T, 'N, 'U] mapper :
287287
method jsx_expression :
288288
('M, 'T) Ast.JSX.ExpressionContainer.t -> ('N, 'U) Ast.JSX.ExpressionContainer.t
289289

290+
method jsx_spread_child : ('M, 'T) Ast.JSX.SpreadChild.t -> ('N, 'U) Ast.JSX.SpreadChild.t
291+
290292
method jsx_fragment : ('M, 'T) Ast.JSX.fragment -> ('N, 'U) Ast.JSX.fragment
291293

292294
method jsx_identifier : ('M, 'T) Flow_ast.JSX.Identifier.t -> ('N, 'U) Ast.JSX.Identifier.t

0 commit comments

Comments
 (0)