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

Simplify parsing and printing attributes of function and arguments #5783

Merged
merged 7 commits into from
Nov 7, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
30 changes: 17 additions & 13 deletions res_syntax/src/res_core.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1458,6 +1458,12 @@ and parseTernaryExpr leftOperand p =
and parseEs6ArrowExpression ?context ?parameters p =
let startPos = p.Parser.startPos in
Parser.leaveBreadcrumb p Grammar.Es6ArrowExpr;
(* Parsing function parameters and attributes:
1. Basically, attributes outside of `(...)` are added to the function, except
the uncurried attribute `(.)` is added to the function. e.g. async, uncurried

2. Attributes inside `(...)` are added to the arguments regardless of whether
labeled, optional or nolabeled *)
let parameters =
match parameters with
| Some params -> params
Expand Down Expand Up @@ -1531,12 +1537,6 @@ and parseParameter p =
then
let startPos = p.Parser.startPos in
let uncurried = Parser.optional p Token.Dot in
(* two scenarios:
* attrs ~lbl ...
* attrs pattern
* Attributes before a labelled arg, indicate that it's on the whole arrow expr
* Otherwise it's part of the pattern
* *)
let attrs = parseAttributes p in
if p.Parser.token = Typ then (
Parser.next p;
Expand All @@ -1554,9 +1554,9 @@ and parseParameter p =
match p.Parser.token with
| Comma | Equal | Rparen ->
let loc = mkLoc startPos p.prevEndPos in
( attrs,
( [],
Asttypes.Labelled lblName,
Ast_helper.Pat.var ~attrs:[propLocAttr] ~loc
Ast_helper.Pat.var ~attrs:(propLocAttr :: attrs) ~loc
(Location.mkloc lblName loc) )
| Colon ->
let lblEnd = p.prevEndPos in
Expand All @@ -1566,25 +1566,29 @@ and parseParameter p =
let pat =
let pat = Ast_helper.Pat.var ~loc (Location.mkloc lblName loc) in
let loc = mkLoc startPos p.prevEndPos in
Ast_helper.Pat.constraint_ ~attrs:[propLocAttr] ~loc pat typ
Ast_helper.Pat.constraint_ ~attrs:(propLocAttr :: attrs) ~loc pat
typ
in
(attrs, Asttypes.Labelled lblName, pat)
cristianoc marked this conversation as resolved.
Show resolved Hide resolved
| As ->
Parser.next p;
let pat =
let pat = parseConstrainedPattern p in
{pat with ppat_attributes = propLocAttr :: pat.ppat_attributes}
{
pat with
ppat_attributes = (propLocAttr :: attrs) @ pat.ppat_attributes;
}
in
(attrs, Asttypes.Labelled lblName, pat)
([], Asttypes.Labelled lblName, pat)
| t ->
Parser.err p (Diagnostics.unexpected t p.breadcrumbs);
let loc = mkLoc startPos p.prevEndPos in
( attrs,
( [],
cristianoc marked this conversation as resolved.
Show resolved Hide resolved
Asttypes.Labelled lblName,
Ast_helper.Pat.var ~loc (Location.mkloc lblName loc) ))
| _ ->
let pattern = parseConstrainedPattern p in
let attrs = List.concat [attrs; pattern.ppat_attributes] in
let attrs = List.concat [pattern.ppat_attributes; attrs] in
([], Asttypes.Nolabel, {pattern with ppat_attributes = attrs})
in
match p.Parser.token with
Expand Down
42 changes: 10 additions & 32 deletions res_syntax/src/res_parsetree_viewer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ let funExpr expr =
collectNewTypes (stringLoc :: acc) returnExpr
| returnExpr -> (List.rev acc, returnExpr)
in
let rec collect n attrsBefore acc expr =
let rec collect attrsBefore acc expr =
match expr with
| {
pexp_desc =
Expand All @@ -147,48 +147,26 @@ let funExpr expr =
{pexp_desc = Pexp_apply _} );
} ->
(attrsBefore, List.rev acc, rewriteUnderscoreApply expr)
| {
pexp_desc = Pexp_fun (lbl, defaultExpr, pattern, returnExpr);
pexp_attributes = [];
} ->
let parameter = Parameter {attrs = []; lbl; defaultExpr; pat = pattern} in
collect (n + 1) attrsBefore (parameter :: acc) returnExpr
| {pexp_desc = Pexp_newtype (stringLoc, rest); pexp_attributes = attrs} ->
let stringLocs, returnExpr = collectNewTypes [stringLoc] rest in
let param = NewTypes {attrs; locs = stringLocs} in
collect (n + 1) attrsBefore (param :: acc) returnExpr
| {pexp_desc = Pexp_fun _; pexp_attributes}
when pexp_attributes
|> List.exists (fun ({Location.txt}, _) ->
txt = "bs" || txt = "res.async")
&& n > 0 ->
(* stop here, the uncurried or async attribute always indicates the beginning of an arrow function
* e.g. `(. a) => (. b)` instead of `(. a, . b)` *)
(attrsBefore, List.rev acc, expr)
collect attrsBefore (param :: acc) returnExpr
| {
pexp_desc =
Pexp_fun
(((Labelled _ | Optional _) as lbl), defaultExpr, pattern, returnExpr);
pexp_attributes = attrs;
pexp_desc = Pexp_fun (lbl, defaultExpr, pattern, returnExpr);
pexp_attributes = [];
} ->
(* Normally attributes are attached to the labelled argument, e.g. (@foo ~x) => ...
In the case of `@res.async`, pass the attribute to the outside *)
let attrs_async, attrs_other =
attrs |> List.partition (fun ({Location.txt}, _) -> txt = "res.async")
in
let parameter =
Parameter {attrs = attrs_other; lbl; defaultExpr; pat = pattern}
in
collect (n + 1) (attrs_async @ attrsBefore) (parameter :: acc) returnExpr
let parameter = Parameter {attrs = []; lbl; defaultExpr; pat = pattern} in
collect attrsBefore (parameter :: acc) returnExpr
| {pexp_desc = Pexp_fun _} -> (attrsBefore, List.rev acc, expr)
cristianoc marked this conversation as resolved.
Show resolved Hide resolved
| expr -> (attrsBefore, List.rev acc, expr)
in
match expr with
| {
pexp_desc = Pexp_fun (Nolabel, _defaultExpr, _pattern, _returnExpr);
pexp_desc = Pexp_fun (_, _defaultExpr, _pattern, _returnExpr);
pexp_attributes = attrs;
} as expr ->
collect 0 attrs [] {expr with pexp_attributes = []}
| expr -> collect 0 [] [] expr
collect attrs [] {expr with pexp_attributes = []}
| expr -> collect [] [] expr

let processBracesAttr expr =
match expr.pexp_attributes with
Expand Down
29 changes: 21 additions & 8 deletions res_syntax/src/res_printer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -4835,13 +4835,23 @@ and printExprFunParameters ~customLayout ~inCallback ~async ~uncurried
attrs = [];
lbl = Asttypes.Nolabel;
defaultExpr = None;
pat = {Parsetree.ppat_desc = Ppat_var stringLoc};
pat =
{
Parsetree.ppat_desc = Ppat_var stringLoc;
Parsetree.ppat_attributes = attrs;
};
};
]
when not uncurried ->
let txtDoc =
let var = printIdentLike stringLoc.txt in
let var = if hasConstraint then addParens var else var in
let var =
match attrs with
| [] -> if hasConstraint then addParens var else var
| attrs ->
let attrs = printAttributes ~customLayout attrs cmtTbl in
addParens (Doc.concat [attrs; var])
in
if async then addAsync var else var
in
printComments txtDoc cmtTbl stringLoc.loc
Expand Down Expand Up @@ -4932,22 +4942,25 @@ and printExpFunParameter ~customLayout parameter cmtTbl =
match (lbl, pattern) with
| Asttypes.Nolabel, pattern -> printPattern ~customLayout pattern cmtTbl
| ( (Asttypes.Labelled lbl | Optional lbl),
{
ppat_desc = Ppat_var stringLoc;
ppat_attributes = [] | [({Location.txt = "ns.namedArgLoc"}, _)];
} )
{ppat_desc = Ppat_var stringLoc; ppat_attributes} )
when lbl = stringLoc.txt ->
(* ~d *)
Doc.concat [Doc.text "~"; printIdentLike lbl]
Doc.concat
[
printAttributes ~customLayout ppat_attributes cmtTbl;
Doc.text "~";
printIdentLike lbl;
]
| ( (Asttypes.Labelled lbl | Optional lbl),
{
ppat_desc = Ppat_constraint ({ppat_desc = Ppat_var {txt}}, typ);
ppat_attributes = [] | [({Location.txt = "ns.namedArgLoc"}, _)];
ppat_attributes;
} )
when lbl = txt ->
(* ~d: e *)
Doc.concat
[
printAttributes ~customLayout ppat_attributes cmtTbl;
Doc.text "~";
printIdentLike lbl;
Doc.text ": ";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,17 @@ let f = ((fun a -> fun b -> fun c -> ())[@bs ])
let f = ((fun a -> fun b -> ((fun c -> fun d -> ())[@bs ]))[@bs ])
let f = ((fun a -> ((fun b -> ((fun c -> ())[@bs ]))[@bs ]))[@bs ])
let f =
((fun ~a:((a)[@ns.namedArgLoc ]) ->
fun b -> ((fun ~c:((c)[@ns.namedArgLoc ]) -> fun d -> ())
[@bs ][@attr ]))
[@bs ][@attr ])
((fun ~a:((a)[@ns.namedArgLoc ][@attr ]) ->
fun b -> ((fun ~c:((c)[@ns.namedArgLoc ][@attr ]) -> fun d -> ())
[@bs ]))
[@bs ])
let f =
((fun ~a:((a)[@ns.namedArgLoc ]) ->
((fun ~a:((a)[@ns.namedArgLoc ][@attr ]) ->
fun ((b)[@attrOnB ]) ->
((fun ~c:((c)[@ns.namedArgLoc ]) -> fun ((d)[@attrOnD ]) -> ())
[@bs ][@attr ]))
[@bs ][@attr ])
((fun ~c:((c)[@ns.namedArgLoc ][@attr ]) ->
fun ((d)[@attrOnD ]) -> ())
[@bs ]))
[@bs ])
let f list = list ()
;;match colour with
| Red when
Expand Down
3 changes: 2 additions & 1 deletion res_syntax/tests/ppx/react/expected/fileLevelConfig.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ module V3 = {

@react.component
let make =
(@warning("-16") ~msg) => {
@warning("-16")
(~msg) => {
ReactDOMRe.createDOMElementVariadic("div", [{msg->React.string}])
}
let make = {
Expand Down
37 changes: 20 additions & 17 deletions res_syntax/tests/ppx/react/expected/forwardRef.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,28 @@ module V3 = {

@react.component
let make =
(@warning("-16") ~className=?, @warning("-16") ~children) =>
@warning("-16")
(~className=?) =>
@warning("-16")
ref =>
ReactDOMRe.createDOMElementVariadic(
"div",
[
ReactDOMRe.createDOMElementVariadic(
"input",
~props=ReactDOMRe.domProps(
~type_="text",
~className?,
~ref=?{Js.Nullable.toOption(ref)->Belt.Option.map(ReactDOM.Ref.domRef)},
(),
(~children) =>
@warning("-16")
ref =>
ReactDOMRe.createDOMElementVariadic(
"div",
[
ReactDOMRe.createDOMElementVariadic(
"input",
~props=ReactDOMRe.domProps(
~type_="text",
~className?,
~ref=?{Js.Nullable.toOption(ref)->Belt.Option.map(ReactDOM.Ref.domRef)},
(),
),
[],
),
[],
),
children,
],
)
children,
],
)
let make = React.forwardRef({
let \"ForwardRef$V3$FancyInput" = (
\"Props": {"className": option<'className>, "children": 'children},
Expand Down
22 changes: 14 additions & 8 deletions res_syntax/tests/ppx/react/expected/innerModule.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@ module Bar = {

@react.component
let make =
(@warning("-16") ~a, @warning("-16") ~b, _) => {
Js.log("This function should be named `InnerModule.react$Bar`")
ReactDOMRe.createDOMElementVariadic("div", [])
}
@warning("-16")
(~a) =>
@warning("-16")
(~b, _) => {
Js.log("This function should be named `InnerModule.react$Bar`")
ReactDOMRe.createDOMElementVariadic("div", [])
}
let make = {
let \"InnerModule$Bar" = (\"Props": {"a": 'a, "b": 'b}) =>
make(~b=\"Props"["b"], ~a=\"Props"["a"], ())
Expand All @@ -17,10 +20,13 @@ module Bar = {

@react.component
let component =
(@warning("-16") ~a, @warning("-16") ~b, _) => {
Js.log("This function should be named `InnerModule.react$Bar$component`")
ReactDOMRe.createDOMElementVariadic("div", [])
}
@warning("-16")
(~a) =>
@warning("-16")
(~b, _) => {
Js.log("This function should be named `InnerModule.react$Bar$component`")
ReactDOMRe.createDOMElementVariadic("div", [])
}
let component = {
let \"InnerModule$Bar$component" = (\"Props": {"a": 'a, "b": 'b}) =>
component(~b=\"Props"["b"], ~a=\"Props"["a"], ())
Expand Down
11 changes: 7 additions & 4 deletions res_syntax/tests/ppx/react/expected/topLevel.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ module V3 = {

@react.component
let make =
(@warning("-16") ~a, @warning("-16") ~b, _) => {
Js.log("This function should be named 'TopLevel.react'")
ReactDOMRe.createDOMElementVariadic("div", [])
}
@warning("-16")
(~a) =>
@warning("-16")
(~b, _) => {
Js.log("This function should be named 'TopLevel.react'")
ReactDOMRe.createDOMElementVariadic("div", [])
}
let make = {
let \"TopLevel$V3" = (\"Props": {"a": 'a, "b": 'b}) =>
make(~b=\"Props"["b"], ~a=\"Props"["a"], ())
Expand Down
6 changes: 3 additions & 3 deletions res_syntax/tests/printer/expr/expected/asyncAwait.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ let _ = await {

let f1 = async (~x, ~y) => x + y
let f2 = async (@foo ~x, @bar ~y) => x + y
let f3 = async (@bar @foo ~x as @zz z, ~y) => x + y
let f3 = @foo async (~x as @bar @zz z, ~y) => x + y
let f4 = async x => x
let f5 = async x => async y => 3
let f6 = async (~x1, ~x2) => async y => 3
Expand All @@ -116,8 +116,8 @@ let f9 = x => async (~y) => 3
let f10 = x => async y => 3
let f11 = (. ~x) => (. ~y) => 3

let f12 = @a x => 3
let f13 = (@a @b ~x) => 3
let f12 = @a (@b x) => 3
let f13 = @a @b (~x) => 3

let aw = (await server->start)->foo
let aw = @foo (server->start)->foo
Expand Down