Simplify parsing and printing attributes of function and arguments

CHANGELOG.md

@@ -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.
 
 #### :rocket: New Feature
 

src/res_core.ml

@@ -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
@@ -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;
@@ -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
@@ -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)
+            ([], 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) ))
         | _ ->
           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

src/res_parsetree_viewer.ml

@@ -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 =
@@ -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)
     | 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

src/res_printer.ml

@@ -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
@@ -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 ": ";

tests/parsing/grammar/expressions/expected/arrow.res.txt

@@ -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

tests/ppx/react/expected/fileLevelConfig.res.txt

@@ -5,7 +5,8 @@ module V3 = {
 
   @react.component
   let make =
-    (@warning("-16") ~msg) => {
+    @warning("-16")
+    (~msg) => {
       ReactDOMRe.createDOMElementVariadic("div", [{msg->React.string}])
     }
   let make = {

tests/ppx/react/expected/forwardRef.res.txt

@@ -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},

tests/ppx/react/expected/innerModule.res.txt

@@ -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"], ())
@@ -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"], ())

tests/ppx/react/expected/topLevel.res.txt

@@ -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"], ())

tests/printer/expr/expected/asyncAwait.res.txt

@@ -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
@@ -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