Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jsx ast #7286

Draft
wants to merge 70 commits into
base: master
Choose a base branch
from
Draft

Jsx ast #7286

Changes from 7 commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
db7eee6
Intial exploration of JSX ast
nojaf Feb 6, 2025
28aa287
Initial mapping from0
nojaf Feb 24, 2025
b57f2df
Format code
nojaf Feb 24, 2025
32161a3
Merge branch 'master' into jsx-ast
nojaf Mar 3, 2025
d714829
Fix jsx fragment mapping
nojaf Mar 3, 2025
e27b339
Remove fragment
nojaf Mar 3, 2025
92657ee
Update test output
nojaf Mar 3, 2025
441de6e
Introducing Pexp_jsx_unary_element & Pexp_jsx_container_element
nojaf Mar 6, 2025
bf1b411
Refactor fragment transformation.
nojaf Mar 6, 2025
5fe5f12
Initial transform of Pexp_jsx_unary_element in automatic mode
nojaf Mar 7, 2025
9251609
Make props for unary element in automatic mode.
nojaf Mar 7, 2025
1aaa686
Initial custom component unary tag
nojaf Mar 8, 2025
b374212
lowercase container element with children.
nojaf Mar 8, 2025
c91aeda
Uppercase container elements
nojaf Mar 8, 2025
85ccab4
Streamline automatic element calls
nojaf Mar 9, 2025
86c9f5f
lowercase container element in classic mode
nojaf Mar 9, 2025
0dfd5b6
Deal with uppercase tags in classic mode.
nojaf Mar 10, 2025
203ff25
Remove old code
nojaf Mar 10, 2025
a273f2e
Improve recovery of incomplete jsx tags.
nojaf Mar 11, 2025
998edbe
Correct range of incomplete jsx elements
nojaf Mar 11, 2025
fcf2d90
Update semantic tokens
nojaf Mar 11, 2025
162b7a2
Make the closing tag optional for jsx_container_element.
nojaf Mar 11, 2025
9b815a0
Add tighter pattern match for edge case in jsx props completion.
nojaf Mar 11, 2025
9408d06
Update analysis tests for jsx elements ast.
nojaf Mar 11, 2025
cc5beb4
print_jsx_unary_tag
nojaf Mar 11, 2025
02e9fc9
Merge branch 'master' into jsx-ast
nojaf Mar 11, 2025
5e060ea
Rough print_jsx_container_tag
nojaf Mar 12, 2025
e57ab30
Add ml printing
nojaf Mar 12, 2025
b6407d0
Wing the sexp thing
nojaf Mar 17, 2025
e565ca7
First step towards ast mapping
nojaf Mar 18, 2025
84e9cbe
prop punning conversion
nojaf Mar 19, 2025
bbfe816
Map prop value
nojaf Mar 19, 2025
63fc87b
Map prop spreading
nojaf Mar 19, 2025
d6c9fa8
Initial container element mapping.
nojaf Mar 19, 2025
e446ae1
Try support children spreading
nojaf Mar 19, 2025
5fef337
Only print space when there are props
nojaf Mar 19, 2025
251244b
Add space after children.
nojaf Mar 19, 2025
aec0d8e
Restore braces in props and children
nojaf Mar 20, 2025
6a3f4c9
Inline is_jsx_expression and remove old code
nojaf Mar 21, 2025
506dda3
Better indentation of children
nojaf Mar 21, 2025
96839f4
Handle unary tag
shulhi Mar 21, 2025
ff1bb65
Refactor
shulhi Mar 21, 2025
91c283e
WIP: Fix unary tag comments handling
shulhi Mar 22, 2025
2b9b5ce
Fix comments inside prop expression
shulhi Mar 22, 2025
0133f91
Formats
shulhi Mar 22, 2025
aaef8ac
Fix closing tag indentation
shulhi Mar 22, 2025
b863888
Fix closing tag indentation for cases with break vs inline
shulhi Mar 22, 2025
7f8bf01
Handle some edge cases
shulhi Mar 22, 2025
4e0951c
Refactor
shulhi Mar 22, 2025
7843795
Merge pull request #3 from shulhi/shulhi-jsx-ast
nojaf Mar 22, 2025
9e68c1a
WIP: Handle punning
shulhi Mar 22, 2025
53616a7
Fix optional printing with braces
shulhi Mar 22, 2025
25e20b8
WIP: Handle comments attachment in a different way
shulhi Mar 23, 2025
ad72bf2
Handle empty props
shulhi Mar 23, 2025
7ba4996
Handle props spread
shulhi Mar 24, 2025
f09cd70
Fix optional with punning
shulhi Mar 24, 2025
aa1d978
Merge pull request #4 from shulhi/shulhi-jsx-ast-2
nojaf Mar 24, 2025
0ee7601
Indent props if they don't fit on one line.
nojaf Mar 24, 2025
bd45146
Fix indentation of children when props are multiline.
nojaf Mar 24, 2025
2d50ff8
Refactor to Pexp_jsx_element
nojaf Mar 26, 2025
fcabcff
Merge branch 'master' into jsx-ast
nojaf Mar 26, 2025
ddcdaa2
Don't always append make when uppercase component
nojaf Mar 26, 2025
52ecf03
Use Doc.line for child spreading
nojaf Mar 26, 2025
cd9059f
Exotic prop name
nojaf Mar 26, 2025
2e9fb66
Don't create record if only spreading prop, use that expression instead.
nojaf Mar 26, 2025
37755ae
Key prop can be optional
nojaf Mar 27, 2025
3a434c9
Keep comment after opening greater than
nojaf Mar 27, 2025
6cf1e71
Print prop value with comments
nojaf Mar 27, 2025
ae9b428
Update generic jsx completion tests
nojaf Mar 27, 2025
f3b96df
Revert isJsxComponent
nojaf Mar 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 19 additions & 19 deletions compiler/syntax/src/res_comments_table.ml
Original file line number Diff line number Diff line change
@@ -1473,27 +1473,27 @@ and walk_expression expr t comments =
let xs = exprs |> List.map (fun e -> Expression e) in
walk_list xs t rest
| Pexp_jsx_unary_element
{jsx_unary_element_tag_name = tag; jsx_unary_element_props = props} ->
(* handles the comments at the tag *)
{jsx_unary_element_tag_name = tag; jsx_unary_element_props = []} ->
let _, _, trailing = partition_by_loc comments tag.loc in
let after_expr, _ = partition_adjacent_trailing tag.loc trailing in
attach t.trailing tag.loc after_expr;
(* handles the comments for the actual props *)
List.iter
(fun prop ->
match prop with
| Parsetree.JSXPropPunning (_, _) -> ()
| Parsetree.JSXPropValue (_, _, expr) ->
let _leading, inside, trailing =
partition_by_loc comments expr.pexp_loc
in
let after_expr, _ =
partition_adjacent_trailing expr.pexp_loc trailing
in
attach t.trailing expr.pexp_loc after_expr;
walk_expression expr t inside
| Parsetree.JSXPropSpreading (_loc, _expr) -> ())
props
attach t.trailing tag.loc after_expr
| Pexp_jsx_unary_element
{jsx_unary_element_tag_name = tag; jsx_unary_element_props = props} ->
let _leading, inside, trailing = partition_by_loc comments tag.loc in
walk_list
(props
|> List.filter_map (fun prop ->
match prop with
| Parsetree.JSXPropPunning (_, {loc}) ->
Some (ExprArgument {expr; loc})
| Parsetree.JSXPropValue ({loc}, _, expr) ->
let loc = {loc with loc_end = expr.pexp_loc.loc_end} in
Some (ExprArgument {expr; loc})
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shulhi I struggle a bit to grasp this part.
What should happen in the follow scenario:

<Foo bar="hey" // comment
></Foo>

Is // comment linked to "hey" or the entire bar="hey" expression?
The loc created here is combining the prop name and value but is further processed with only the value expression?
This feel off, is this by design? If so, I assume you then compensate for this in res_printer?
Would it not make more sense to either introduce a new node type or create a helper function and process the three parts (entire prop, prop name & prop value)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is // comment linked to "hey" or the entire bar="hey" expression?
This feel off, is this by design? If so, I assume you then compensate for this in res_printer?

Initially I had it attached to just the property (i.e. bar) instead of the whole expression but it doesn't work well especially for multiline expression. Attaching it to the whole thing seems to work better.

This is compensated in the printer. Technically, we can still attach it to just the property, but we still have to compensate for the comment printing or add additional logic to cater for multiline expression or other edge cases. This approach seems like a better trade off at that point.

Would it not make more sense to either introduce a new node type or create a helper function and process the three parts (entire prop, prop name & prop value)?

I think a helper function should work. A new node type would work in the future to improve the comment printing.

| Parsetree.JSXPropSpreading (loc, expr) ->
let loc = {loc with loc_end = expr.pexp_loc.loc_end} in
Some (ExprArgument {expr; loc})))
t trailing;
walk_expression expr t inside
| Pexp_jsx_container_element
{
jsx_container_element_opening_tag_end = opening_greater_than;
60 changes: 39 additions & 21 deletions compiler/syntax/src/res_printer.ml
Original file line number Diff line number Diff line change
@@ -4510,34 +4510,52 @@ and print_jsx_prop ~state prop cmt_tbl =
let open Parsetree in
match prop with
| JSXPropPunning (is_optional, name) ->
if is_optional then Doc.concat [Doc.question; Doc.text name.txt]
else Doc.text name.txt
let prop_has_trailing_comment = has_trailing_comments cmt_tbl name.loc in
let doc =
if is_optional then Doc.concat [Doc.question; Doc.text name.txt]
else Doc.text name.txt
in
let doc = print_comments doc cmt_tbl name.loc in
if prop_has_trailing_comment then
Doc.group (Doc.concat [Doc.break_parent; doc])
else doc
| JSXPropValue (name, is_optional, value) ->
let value =
{
value with
pexp_loc = {value.pexp_loc with loc_start = name.loc.loc_start};
}
in
let value_doc =
let v =
Doc.concat
[
(if is_optional then Doc.question else Doc.nil);
print_expression_with_comments ~state value cmt_tbl;
]
in
let v = print_expression ~state value cmt_tbl in
match Parens.jsx_prop_expr value with
| Parenthesized | Braced _ ->
let inner_doc = if Parens.braced_expr value then add_parens v else v in
if has_leading_line_comment cmt_tbl value.pexp_loc then
add_braces inner_doc
else Doc.concat [Doc.lbrace; inner_doc; Doc.rbrace]
Doc.concat [Doc.lbrace; inner_doc; Doc.rbrace]
| _ -> v
in
Doc.concat [Doc.text name.txt; Doc.equal; Doc.group value_doc]
| JSXPropSpreading (_, value) ->
Doc.concat
[
Doc.lbrace;
Doc.dotdotdot;
print_expression_with_comments ~state value cmt_tbl;
Doc.rbrace;
]
let doc =
Doc.concat
[
Doc.text name.txt;
Doc.equal;
(if is_optional then Doc.question else Doc.nil);
Doc.group value_doc;
]
in
print_comments doc cmt_tbl value.pexp_loc
| JSXPropSpreading (loc, value) ->
let value =
{value with pexp_loc = {value.pexp_loc with loc_start = loc.loc_start}}
in
Doc.group
(Doc.concat
[
Doc.lbrace;
Doc.dotdotdot;
print_expression_with_comments ~state value cmt_tbl;
Doc.rbrace;
])

and print_jsx_props ~state props cmt_tbl : Doc.t list =
props |> List.map (fun prop -> print_jsx_prop ~state prop cmt_tbl)