Skip to content

Commit dfb5b41

Browse files
Hans Halversonfacebook-github-bot
Hans Halverson
authored and
facebook-github-bot
committed
Intern comments in JSX spread attributes
Summary: Interns comments around JSX spread attributes. Comments that directly precede and succeed the braces of the JSX spread attribute will be attached as trailing and leading comments, respectively. ``` <div attribute=/* L JSX spread */ {.../* L expr */ expr /* T expr */} /* T JSX spread */ /> ``` Note that this does not attach comments that appear between the opening brace the the ellipsis. If we want to support these, they can be added later as internal comments. Also note that there is still ambiguity about whether leading and trailing comments for JSX attributes should be attached to the previous or next node. This will be fixed in the future once we have an API for eating portions of comments, as this same problem appears for the trailing comments on statements. Reviewed By: pieterv Differential Revision: D20319152 fbshipit-source-id: 75c64b0989f1a2aeadbd21478d916abdf73dd091
1 parent 21b4a00 commit dfb5b41

10 files changed

Lines changed: 144 additions & 20 deletions

File tree

‎src/parser/estree_translator.ml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1536,8 +1536,8 @@ with type t = Impl.t = struct
15361536
function
15371537
| Literal (loc, value) -> literal (loc, value)
15381538
| ExpressionContainer (loc, expr) -> jsx_expression_container (loc, expr))
1539-
and jsx_spread_attribute (loc, { JSX.SpreadAttribute.argument }) =
1540-
node "JSXSpreadAttribute" loc [("argument", expression argument)]
1539+
and jsx_spread_attribute (loc, { JSX.SpreadAttribute.argument; comments }) =
1540+
node ?comments "JSXSpreadAttribute" loc [("argument", expression argument)]
15411541
and jsx_expression_container (loc, { JSX.ExpressionContainer.expression = expr; comments }) =
15421542
let expression =
15431543
match expr with

‎src/parser/flow_ast.ml‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1279,7 +1279,11 @@ and JSX : sig
12791279
module SpreadAttribute : sig
12801280
type ('M, 'T) t = 'M * ('M, 'T) t'
12811281

1282-
and ('M, 'T) t' = { argument: ('M, 'T) Expression.t } [@@deriving show]
1282+
and ('M, 'T) t' = {
1283+
argument: ('M, 'T) Expression.t;
1284+
comments: ('M, unit) Syntax.t option;
1285+
}
1286+
[@@deriving show]
12831287
end
12841288

12851289
module MemberExpression : sig

‎src/parser/jsx_parser.ml‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,16 @@ module JSX (Parse : Parser_common.PARSER) = struct
1717
let attr =
1818
with_loc
1919
(fun env ->
20+
let leading = Peek.comments env in
2021
Expect.token env T_LCURLY;
2122
Expect.token env T_ELLIPSIS;
2223
let argument = Parse.assignment env in
2324
Expect.token env T_RCURLY;
24-
{ JSX.SpreadAttribute.argument })
25+
let trailing = Peek.comments env in
26+
{
27+
JSX.SpreadAttribute.argument;
28+
comments = Flow_ast_utils.mk_comments_opt ~leading ~trailing ();
29+
})
2530
env
2631
in
2732
Eat.pop_lex_mode env;

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,5 @@
1313
<div name=/* 7.1 L JSX expr */ {/* 7.2 L num */ 1 /* 7.3 T num */} /* 7.4 T JSX expr */ />;
1414

1515
<div>{/* 8.1 L JSX spread */ ... /* 8.2 L obj */ {} /* 8.3 T obj */}</div>;
16+
17+
<div /* 9.1 L JSX spread */{... /* 9.2 L obj */ {} /* 9.3 T obj */} /* 9.4 T JSX spread */ />;

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

Lines changed: 104 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":15,"column":75}},
4-
"range":[24,629],
3+
"loc":{"source":null,"start":{"line":1,"column":24},"end":{"line":17,"column":94}},
4+
"range":[24,725],
55
"body":[
66
{
77
"type":"ExpressionStatement",
@@ -562,6 +562,84 @@
562562
]
563563
},
564564
"directive":null
565+
},
566+
{
567+
"type":"ExpressionStatement",
568+
"loc":{"source":null,"start":{"line":17,"column":0},"end":{"line":17,"column":94}},
569+
"range":[631,725],
570+
"expression":{
571+
"type":"JSXElement",
572+
"loc":{"source":null,"start":{"line":17,"column":0},"end":{"line":17,"column":93}},
573+
"range":[631,724],
574+
"openingElement":{
575+
"type":"JSXOpeningElement",
576+
"loc":{"source":null,"start":{"line":17,"column":0},"end":{"line":17,"column":93}},
577+
"range":[631,724],
578+
"name":{
579+
"type":"JSXIdentifier",
580+
"trailingComments":[
581+
{
582+
"type":"Block",
583+
"loc":{"source":null,"start":{"line":17,"column":5},"end":{"line":17,"column":27}},
584+
"range":[636,658],
585+
"value":" 9.1 L JSX spread "
586+
}
587+
],
588+
"loc":{"source":null,"start":{"line":17,"column":1},"end":{"line":17,"column":4}},
589+
"range":[632,635],
590+
"name":"div"
591+
},
592+
"attributes":[
593+
{
594+
"type":"JSXSpreadAttribute",
595+
"trailingComments":[
596+
{
597+
"type":"Block",
598+
"loc":{"source":null,"start":{"line":17,"column":68},"end":{"line":17,"column":90}},
599+
"range":[699,721],
600+
"value":" 9.4 T JSX spread "
601+
}
602+
],
603+
"leadingComments":[
604+
{
605+
"type":"Block",
606+
"loc":{"source":null,"start":{"line":17,"column":5},"end":{"line":17,"column":27}},
607+
"range":[636,658],
608+
"value":" 9.1 L JSX spread "
609+
}
610+
],
611+
"loc":{"source":null,"start":{"line":17,"column":27},"end":{"line":17,"column":67}},
612+
"range":[658,698],
613+
"argument":{
614+
"type":"ObjectExpression",
615+
"trailingComments":[
616+
{
617+
"type":"Block",
618+
"loc":{"source":null,"start":{"line":17,"column":51},"end":{"line":17,"column":66}},
619+
"range":[682,697],
620+
"value":" 9.3 T obj "
621+
}
622+
],
623+
"leadingComments":[
624+
{
625+
"type":"Block",
626+
"loc":{"source":null,"start":{"line":17,"column":32},"end":{"line":17,"column":47}},
627+
"range":[663,678],
628+
"value":" 9.2 L obj "
629+
}
630+
],
631+
"loc":{"source":null,"start":{"line":17,"column":48},"end":{"line":17,"column":50}},
632+
"range":[679,681],
633+
"properties":[]
634+
}
635+
}
636+
],
637+
"selfClosing":true
638+
},
639+
"closingElement":null,
640+
"children":[]
641+
},
642+
"directive":null
565643
}
566644
],
567645
"comments":[
@@ -714,6 +792,30 @@
714792
"loc":{"source":null,"start":{"line":15,"column":52},"end":{"line":15,"column":67}},
715793
"range":[606,621],
716794
"value":" 8.3 T obj "
795+
},
796+
{
797+
"type":"Block",
798+
"loc":{"source":null,"start":{"line":17,"column":5},"end":{"line":17,"column":27}},
799+
"range":[636,658],
800+
"value":" 9.1 L JSX spread "
801+
},
802+
{
803+
"type":"Block",
804+
"loc":{"source":null,"start":{"line":17,"column":32},"end":{"line":17,"column":47}},
805+
"range":[663,678],
806+
"value":" 9.2 L obj "
807+
},
808+
{
809+
"type":"Block",
810+
"loc":{"source":null,"start":{"line":17,"column":51},"end":{"line":17,"column":66}},
811+
"range":[682,697],
812+
"value":" 9.3 T obj "
813+
},
814+
{
815+
"type":"Block",
816+
"loc":{"source":null,"start":{"line":17,"column":68},"end":{"line":17,"column":90}},
817+
"range":[699,721],
818+
"value":" 9.4 T JSX spread "
717819
}
718820
]
719821
}

‎src/parser_utils/flow_ast_differ.ml‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,16 +1189,19 @@ let program
11891189
let open Ast.JSX.Opening in
11901190
match (jsx_attr1, jsx_attr2) with
11911191
| (Attribute attr1, Attribute attr2) -> diff_if_changed_ret_opt jsx_attribute attr1 attr2
1192-
| (SpreadAttribute attr1, SpreadAttribute attr2) ->
1193-
diff_if_changed jsx_spread_attribute attr1 attr2 |> Base.Option.return
1192+
| (SpreadAttribute ((loc, _) as attr1), SpreadAttribute attr2) ->
1193+
diff_if_changed_ret_opt (jsx_spread_attribute loc) attr1 attr2
11941194
| _ -> None
11951195
and jsx_spread_attribute
1196+
(loc : Loc.t)
11961197
(attr1 : (Loc.t, Loc.t) Ast.JSX.SpreadAttribute.t)
1197-
(attr2 : (Loc.t, Loc.t) Ast.JSX.SpreadAttribute.t) : node change list =
1198+
(attr2 : (Loc.t, Loc.t) Ast.JSX.SpreadAttribute.t) : node change list option =
11981199
let open Flow_ast.JSX.SpreadAttribute in
1199-
let (_, { argument = arg1 }) = attr1 in
1200-
let (_, { argument = arg2 }) = attr2 in
1201-
diff_if_changed expression arg1 arg2
1200+
let (_, { argument = arg1; comments = comments1 }) = attr1 in
1201+
let (_, { argument = arg2; comments = comments2 }) = attr2 in
1202+
let argument_diff = Some (diff_if_changed expression arg1 arg2) in
1203+
let comments_diff = syntax_opt loc comments1 comments2 in
1204+
join_diff_list [argument_diff; comments_diff]
12021205
and jsx_attribute
12031206
(attr1 : (Loc.t, Loc.t) Ast.JSX.Attribute.t) (attr2 : (Loc.t, Loc.t) Ast.JSX.Attribute.t) :
12041207
node change list option =

‎src/parser_utils/flow_ast_mapper.ml‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,8 +1253,13 @@ class ['loc] mapper =
12531253

12541254
method jsx_spread_attribute _loc (attr : ('loc, 'loc) Ast.JSX.SpreadAttribute.t') =
12551255
let open Ast.JSX.SpreadAttribute in
1256-
let { argument } = attr in
1257-
id this#expression argument attr (fun argument -> { argument })
1256+
let { argument; comments } = attr in
1257+
let argument' = this#expression argument in
1258+
let comments' = this#syntax_opt comments in
1259+
if argument == argument' && comments == comments' then
1260+
attr
1261+
else
1262+
{ argument = argument'; comments = comments' }
12581263

12591264
method jsx_attribute (attr : ('loc, 'loc) Ast.JSX.Attribute.t) =
12601265
let open Ast.JSX.Attribute in

‎src/parser_utils/flow_polymorphic_ast_mapper.ml‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,8 +1031,10 @@ class virtual ['M, 'T, 'N, 'U] mapper =
10311031
method jsx_spread_attribute (attr : ('M, 'T) Ast.JSX.SpreadAttribute.t')
10321032
: ('N, 'U) Ast.JSX.SpreadAttribute.t' =
10331033
let open Ast.JSX.SpreadAttribute in
1034-
let { argument } = attr in
1035-
{ argument = this#expression argument }
1034+
let { argument; comments } = attr in
1035+
let argument' = this#expression argument in
1036+
let comments' = Base.Option.map ~f:this#syntax comments in
1037+
{ argument = argument'; comments = comments' }
10361038

10371039
method jsx_attribute (attr : ('M, 'T) Ast.JSX.Attribute.t) : ('N, 'U) Ast.JSX.Attribute.t =
10381040
let open Ast.JSX.Attribute in

‎src/parser_utils/output/js_layout_generator.ml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,8 +2032,9 @@ and jsx_attribute (loc, { Ast.JSX.Attribute.name; value }) =
20322032
end;
20332033
] )
20342034

2035-
and jsx_spread_attribute (loc, { Ast.JSX.SpreadAttribute.argument }) =
2036-
source_location_with_comments (loc, fuse [Atom "{"; Atom "..."; expression argument; Atom "}"])
2035+
and jsx_spread_attribute (loc, { Ast.JSX.SpreadAttribute.argument; comments }) =
2036+
layout_node_with_comments_opt loc comments
2037+
@@ fuse [Atom "{"; Atom "..."; expression argument; Atom "}"]
20372038

20382039
and jsx_element_name = function
20392040
| Ast.JSX.Identifier ident -> jsx_identifier ident

‎src/typing/statement.ml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5994,10 +5994,10 @@ and jsx_mk_props cx reason c name attributes children =
59945994
(* TODO: attributes with namespaced names *)
59955995
(acc, atts)
59965996
(* <element {...spread} /> *)
5997-
| Opening.SpreadAttribute (spread_loc, { SpreadAttribute.argument }) ->
5997+
| Opening.SpreadAttribute (spread_loc, { SpreadAttribute.argument; comments }) ->
59985998
let (((_, spread), _) as argument) = expression cx argument in
59995999
let acc = ObjectExpressionAcc.add_spread spread acc in
6000-
let att = Opening.SpreadAttribute (spread_loc, { SpreadAttribute.argument }) in
6000+
let att = Opening.SpreadAttribute (spread_loc, { SpreadAttribute.argument; comments }) in
60016001
(acc, att :: atts))
60026002
(ObjectExpressionAcc.empty ~allow_sealed:true, [])
60036003
attributes

0 commit comments

Comments
 (0)