Skip to content
This repository has been archived by the owner on Jun 15, 2023. It is now read-only.

Commit

Permalink
Simplify parsing attributes of function and args (#722)
Browse files Browse the repository at this point in the history
  • Loading branch information
mununki authored Dec 14, 2022
1 parent 51e4e16 commit c1b478e
Show file tree
Hide file tree
Showing 11 changed files with 117 additions and 96 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#### :boom: Breaking Change

- Emit an error when a `@string` or `@int` attribute is used in a V4 component https://github.com/rescript-lang/rescript-compiler/issues/5724
- Parse the attributes of labelled argument to the pattern attributes of argument instead of function. https://github.com/rescript-lang/syntax/pull/722

#### :rocket: New Feature

Expand Down
35 changes: 20 additions & 15 deletions 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,30 @@ 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)
([], Asttypes.Labelled lblName, pat)
| 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,
( [],
Asttypes.Labelled lblName,
Ast_helper.Pat.var ~loc (Location.mkloc lblName loc) ))
Ast_helper.Pat.var ~attrs:(propLocAttr :: attrs) ~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
44 changes: 12 additions & 32 deletions 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,28 @@ 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
(* If a fun has an attribute, then it stops here and makes currying.
i.e attributes outside of (...), uncurried `(.)` and `async` make currying *)
| {pexp_desc = Pexp_fun _} -> (attrsBefore, List.rev acc, expr)
| 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 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
17 changes: 9 additions & 8 deletions tests/parsing/grammar/expressions/expected/arrow.res.txt
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 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 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 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 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
4 changes: 4 additions & 0 deletions tests/printer/expr/asyncAwait.res
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,7 @@ let b1 = await (3+4)
let b2 = await (3**4)
let b3 = await (foo->bar(~arg))
let b4 = await (foo.bar.baz)

let c1 = @foo x => @bar y => x + y
let c2 = (. x) => y => x+y
let c3 = (. x) => @foo y => x+y
10 changes: 7 additions & 3 deletions 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 All @@ -135,3 +135,7 @@ let b1 = await (3 + 4)
let b2 = await (3 ** 4)
let b3 = await foo->bar(~arg)
let b4 = await foo.bar.baz

let c1 = @foo x => @bar y => x + y
let c2 = (. x, y) => x + y
let c3 = (. x) => @foo y => x + y

0 comments on commit c1b478e

Please sign in to comment.